mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

chore: backport 15 changes from 2-M127 to 1-M129 (#44355)

Browse source 
chore: backport 18 changes from 2-M127 to 1-M129

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
This commit is contained in:
Pedro Pontes 2024-10-22 07:20:35 -07:00 committed by GitHub
parent afad838a8b
commit c6714ee45c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
20 changed files with 1469 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
patches/DirectXShaderCompiler/.patches Normal file
View file

@ -0,0 +1 @@
fix_asan_uaf_in_dxilconditionalmem2reg_6910.patch

100
patches/DirectXShaderCompiler/fix_asan_uaf_in_dxilconditionalmem2reg_6910.patch Normal file
View file

@ -0,0 +1,100 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Antonio Maiorano <amaiorano@google.com>
Date: Tue, 17 Sep 2024 18:15:03 -0400
Subject: Fix ASAN UAF in DxilConditionalMem2Reg (#6910)
ScalarizePreciseVectorAlloca would iterate over all instructions, then
for each instruction use, would iterate and potentially erase the
instruction. If the erased instruction was the immediate next
instruction after the alloca, this would invalidate the outer
instruction iterator. Fixed by collecting the allocas in a vector first.
Bug: 365254285
Change-Id: I551455295c600e49354beae665275bc80ce186e4
Reviewed-on: https://chromium-review.googlesource.com/c/external/github.com/microsoft/DirectXShaderCompiler/+/5870863
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
diff --git a/lib/Transforms/Scalar/DxilConditionalMem2Reg.cpp b/lib/Transforms/Scalar/DxilConditionalMem2Reg.cpp
index 8d44d651511ce3cdb24d4689a1fdd0215dee35a2..c3fc16c40b5cd83194b1edc820e22388f79c66c2 100644
--- a/lib/Transforms/Scalar/DxilConditionalMem2Reg.cpp
+++ b/lib/Transforms/Scalar/DxilConditionalMem2Reg.cpp
@@ -270,7 +270,7 @@ public:
static bool ScalarizePreciseVectorAlloca(Function &F) {
BasicBlock *Entry = &*F.begin();
- bool Changed = false;
+ SmallVector<AllocaInst *, 4> PreciseAllocaInsts;
for (auto it = Entry->begin(); it != Entry->end();) {
Instruction *I = &*(it++);
AllocaInst *AI = dyn_cast<AllocaInst>(I);
@@ -278,7 +278,11 @@ public:
continue;
if (!HLModule::HasPreciseAttributeWithMetadata(AI))
continue;
+ PreciseAllocaInsts.push_back(AI);
+ }
+ bool Changed = false;
+ for (auto AI : PreciseAllocaInsts) {
IRBuilder<> B(AI);
VectorType *VTy = cast<VectorType>(AI->getAllocatedType());
Type *ScalarTy = VTy->getVectorElementType();
diff --git a/tools/clang/test/DXC/Passes/DxilConditionalMem2Reg/precise-vector-alloca-followed-by-use.hlsl b/tools/clang/test/DXC/Passes/DxilConditionalMem2Reg/precise-vector-alloca-followed-by-use.hlsl
new file mode 100644
index 0000000000000000000000000000000000000000..413daca955a0c9eef1ec96310583eb9ac491108e
--- /dev/null
+++ b/tools/clang/test/DXC/Passes/DxilConditionalMem2Reg/precise-vector-alloca-followed-by-use.hlsl
@@ -0,0 +1,52 @@
+// RUN: %dxc -T vs_6_0 %s | FileCheck %s
+
+// The following HLSL resulted in an ASAN use-after-free in DxilConditionalMem2Reg
+// in ScalarizePreciseVectorAlloca. ScalarizePreciseVectorAlloca would iterate over
+// all instructions, then for each instruction use, would iterate and potentially
+// erase the instruction. If the erased instruction was the immediate next
+// instruction after the alloca, this would invalidate the outer instruction iterator.
+
+// Unfortunately, we cannot create an IR test for this because dxil-cond-mem2reg
+// executes between scalarrepl-param-hlsl and hlsl-dxil-precise, and the former
+// temporarily marks empty functions as 'precise' while the latter pass uses this
+// information, and then deletes these functions. But splitting the passes in between
+// these two fails validation because empty functions cannot have attributes on them.
+// So we use a full HLSL test for this.
+
+// The IR before dxil-cond-mem2reg for this HLSL contains a precise vector alloca
+// followed immediately by a use of the alloca (a store in this case):
+//
+// %sp.0 = alloca <4 x float>, !dx.precise !3
+// store <4 x float> zeroinitializer, <4 x float>* %sp.0, !dbg !4
+//
+// After dxil-cond-mem2reg, it should look like:
+//
+// %1 = alloca float, !dx.precise !3
+// %2 = alloca float, !dx.precise !3
+// %3 = alloca float, !dx.precise !3
+// %4 = alloca float, !dx.precise !3
+// store float 0.000000e+00, float* %1, !dbg !4
+// store float 0.000000e+00, float* %2, !dbg !4
+// store float 0.000000e+00, float* %3, !dbg !4
+// store float 0.000000e+00, float* %4, !dbg !4
+
+// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 0, float 1.000000e+00)
+// CHECK-NEXT: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 1, float 1.000000e+00)
+// CHECK-NEXT: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 2, float 1.000000e+00)
+// CHECK-NEXT: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 3, float 1.000000e+00)
+
+struct S {
+ float4 b;
+};
+
+struct SP {
+ precise float4 b : SV_Position;
+};
+
+static S s = {(1.0f).xxxx};
+
+SP main() {
+ SP sp = (SP)0;
+ sp.b = s.b;
+ return sp;
+}

5
patches/chromium/.patches
View file

@ -134,3 +134,8 @@ feat_enable_customizing_symbol_color_in_framecaptionbutton.patch
cherry-pick-99cafbf4b4b9.patch
cherry-pick-44b7fbf35b10.patch
fix_potential_draggable_region_crash_when_no_mainframeimpl.patch
m126-lts_fix_a_range_check_for_when_it_overflows.patch
m126-lts_check_string_range_in_shapesegment.patch
m126-lts_reland_fix_stringview_to_crash_when_offset_length.patch
m126-lts_protect_automation_rate_from_non-deterministic_change.patch
m126-lts_don_t_perform_pseudo-element_ident_parsing_on_non-ascii.patch

75
patches/chromium/m126-lts_check_string_range_in_shapesegment.patch Normal file
View file

@ -0,0 +1,75 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Koji Ishii <kojii@chromium.org>
Date: Thu, 12 Sep 2024 06:00:02 +0000
Subject: [M126-LTS] Check string range in `ShapeSegment`
crrev.com/c/5776342 fixed a range `CHECK` in
`CollectFallbackHintChars`, but depends on the CSS and font
configurations, it's possible that the code doesn't go to
`CollectFallbackHintChars` and the following code may hit
the same issue.
This patch adds another `CHECK` for the case.
(cherry picked from commit ef6f7b4521bb9e8d0235550c93acf885e198abdb)
Bug: 355731798, 357622693
Change-Id: Ieb4ada7699c80564e8a4b866cb6a6ffbc665ebc7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5776204
Commit-Queue: Kent Tamura <tkent@chromium.org>
Auto-Submit: Koji Ishii <kojii@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1340006}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5806849
Auto-Submit: Roger Felipe Zanoni da Silva (xWF) <rzanoni@google.com>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Reviewed-by: Fahad Mansoor <fahadmansoor@google.com>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/branch-heads/6478@{#1959}
Cr-Branched-From: e6143acc03189c5e52959545b110d6d17ecd5286-refs/heads/main@{#1300313}
diff --git a/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_shaper.cc b/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_shaper.cc
index 7731bd142f1352d0bbc67a1f9a3742de0adc11ad..be09f8302145e71c42899aa17dfc765037413a2c 100644
--- a/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_shaper.cc
+++ b/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_shaper.cc
@@ -492,6 +492,12 @@ inline void HarfBuzzShaper::CheckTextLen(unsigned start,
CHECK_LE(length, text_.length() - start);
}
+inline void HarfBuzzShaper::CheckTextEnd(unsigned start, unsigned end) const {
+ CHECK_LE(start, end);
+ CHECK_LE(start, text_.length());
+ CHECK_LE(end, text_.length());
+}
+
void HarfBuzzShaper::CommitGlyphs(RangeContext* range_data,
const SimpleFontData* current_font,
UScriptCode current_run_script,
@@ -942,12 +948,13 @@ void HarfBuzzShaper::ShapeSegment(
// Clamp the start and end offsets of the queue item to the offsets
// representing the shaping window.
- unsigned shape_start =
+ const unsigned shape_start =
std::max(range_data->start, current_queue_item.start_index_);
- unsigned shape_end =
+ const unsigned shape_end =
std::min(range_data->end, current_queue_item.start_index_ +
current_queue_item.num_characters_);
DCHECK_GT(shape_end, shape_start);
+ CheckTextEnd(shape_start, shape_end);
CaseMapIntend case_map_intend = CaseMapIntend::kKeepSameCase;
if (needs_caps_handling) {
diff --git a/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_shaper.h b/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_shaper.h
index 102b6bb08105db6f9327acf6250c961d0b322170..f97e92a26fcde1aa533869dfad9eaf20ae65dd95 100644
--- a/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_shaper.h
+++ b/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_shaper.h
@@ -173,6 +173,7 @@ class PLATFORM_EXPORT HarfBuzzShaper final {
ShapeResult*) const;
void CheckTextLen(unsigned start, unsigned length) const;
+ void CheckTextEnd(unsigned start, unsigned end) const;
const String text_;
EmojiMetricsCallback emoji_metrics_reporter_for_testing_;

50
patches/chromium/m126-lts_don_t_perform_pseudo-element_ident_parsing_on_non-ascii.patch Normal file
View file

@ -0,0 +1,50 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Gyuyoung Kim <qkim@google.com>
Date: Tue, 1 Oct 2024 02:11:48 +0000
Subject: [M126-LTS] Don't perform pseudo-element ident parsing on non-ASCII
ParsePseudoType crashes on ASAN when given non-ASCII characters,
so returning early if those are present.
(cherry picked from commit f50b84cf5edf9a3ef09ddee9a24aeae5da55c630)
Bug: 350779647
Change-Id: Ic77351a1c95437a226dce66c7826b7b8481b8d91
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5686295
Commit-Queue: Noam Rosenthal <nrosenthal@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1346768}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5872266
Owners-Override: Victor Gabriel Savu <vsavu@google.com>
Reviewed-by: Victor Gabriel Savu <vsavu@google.com>
Commit-Queue: Gyuyoung Kim (xWF) <qkim@google.com>
Cr-Commit-Position: refs/branch-heads/6478@{#1974}
Cr-Branched-From: e6143acc03189c5e52959545b110d6d17ecd5286-refs/heads/main@{#1300313}
diff --git a/third_party/blink/renderer/core/css/parser/css_selector_parser.cc b/third_party/blink/renderer/core/css/parser/css_selector_parser.cc
index a14ef80a98838036649215d47054b86132e08bc7..991fef25fb349f46ed2ccb9e02fe02d1fc75ae41 100644
--- a/third_party/blink/renderer/core/css/parser/css_selector_parser.cc
+++ b/third_party/blink/renderer/core/css/parser/css_selector_parser.cc
@@ -962,7 +962,7 @@ PseudoId CSSSelectorParser::ParsePseudoElement(const String& selector_string,
CSSParserToken selector_name_token = range.Peek(ident_start);
if (selector_name_token.GetType() == kIdentToken) {
- if (!selector_name_token.Value().Is8Bit()) {
+ if (!selector_name_token.Value().ContainsOnlyASCIIOrEmpty()) {
return kPseudoIdInvalid;
}
if (range.Peek(ident_start + 1).GetType() != kEOFToken) {
diff --git a/third_party/blink/web_tests/external/wpt/css/cssom/getComputedStyle-special-chars-crash.html b/third_party/blink/web_tests/external/wpt/css/cssom/getComputedStyle-special-chars-crash.html
new file mode 100644
index 0000000000000000000000000000000000000000..a9c1dd9976af10efb197bf9bdb0bef47516db7c3
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/css/cssom/getComputedStyle-special-chars-crash.html
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<body>
+<script>
+ window.getComputedStyle(document.body, String.fromCharCode( 92, 109, 107, 78, 80, 113, 90, 102, 76, 49));
+</script>
+This test shouldn't crash.
+</body>
\ No newline at end of file

63
patches/chromium/m126-lts_fix_a_range_check_for_when_it_overflows.patch Normal file
View file

@ -0,0 +1,63 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Koji Ishii <kojii@chromium.org>
Date: Thu, 12 Sep 2024 05:51:00 +0000
Subject: [M126-LTS] Fix a range `CHECK` for when it overflows
This patch fixes a `CHECK` for a range of a string when
`offset + length` overflows the `unsigned`.
(cherry picked from commit 59c286e8419f07143ce859342f0fe9ddea36392d)
Bug: 355731798
Change-Id: If04222f10f2b73b6dcd6b412cf4d82fa5b71bbe2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5776342
Commit-Queue: Kent Tamura <tkent@chromium.org>
Auto-Submit: Koji Ishii <kojii@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1339526}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5804713
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Fahad Mansoor <fahadmansoor@google.com>
Auto-Submit: Roger Felipe Zanoni da Silva (xWF) <rzanoni@google.com>
Cr-Commit-Position: refs/branch-heads/6478@{#1958}
Cr-Branched-From: e6143acc03189c5e52959545b110d6d17ecd5286-refs/heads/main@{#1300313}
diff --git a/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_shaper.cc b/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_shaper.cc
index 427fbb617742c7690338ad6729be720826955b1f..7731bd142f1352d0bbc67a1f9a3742de0adc11ad 100644
--- a/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_shaper.cc
+++ b/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_shaper.cc
@@ -486,6 +486,12 @@ CanvasRotationInVertical CanvasRotationForRun(
} // namespace
+inline void HarfBuzzShaper::CheckTextLen(unsigned start,
+ unsigned length) const {
+ CHECK_LE(start, text_.length());
+ CHECK_LE(length, text_.length() - start);
+}
+
void HarfBuzzShaper::CommitGlyphs(RangeContext* range_data,
const SimpleFontData* current_font,
UScriptCode current_run_script,
@@ -697,7 +703,7 @@ bool HarfBuzzShaper::CollectFallbackHintChars(
break;
}
- CHECK_LE((it->start_index_ + it->num_characters_), text_.length());
+ CheckTextLen(it->start_index_, it->num_characters_);
if (text_.Is8Bit()) {
for (unsigned i = 0; i < it->num_characters_; i++) {
const UChar hint_char = text_[it->start_index_ + i];
diff --git a/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_shaper.h b/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_shaper.h
index 6ad434d4586c3f82a11a215f27bbb2e548b5bce9..102b6bb08105db6f9327acf6250c961d0b322170 100644
--- a/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_shaper.h
+++ b/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_shaper.h
@@ -172,6 +172,8 @@ class PLATFORM_EXPORT HarfBuzzShaper final {
const BufferSlice&,
ShapeResult*) const;
+ void CheckTextLen(unsigned start, unsigned length) const;
+
const String text_;
EmojiMetricsCallback emoji_metrics_reporter_for_testing_;
};

205
patches/chromium/m126-lts_protect_automation_rate_from_non-deterministic_change.patch Normal file
View file

@ -0,0 +1,205 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Hongchan Choi <hongchan@chromium.org>
Date: Tue, 17 Sep 2024 17:04:42 +0000
Subject: [M126-LTS] Protect automation_rate_ from non-deterministic change
This CL fixes non-deterministic (racy) data change on
AudioParamHandler::automation_rate_. It also revises incorrect logic
in the DelayHandler's process function; the process function
needs to process all the channels in the delay kernel in the same
rate. However, the previous code allowed the automation rate to
change any time even in the middle of processing.
This fix is locally confirmed with the provided repro case,
and also a test was added to verify other related API surfaces.
(cherry picked from commit ec85a32bb5d736637c934088c14b2b6a42457467)
Bug: 357391257
Change-Id: I7ce953837edd818e435e3a1b917f6b3c6147d95b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5767447
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1345091}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5854511
Reviewed-by: Giovanni Pezzino <giovax@google.com>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Michael Wilson <mjwilson@chromium.org>
Commit-Queue: Roger Felipe Zanoni da Silva (xWF) <rzanoni@google.com>
Cr-Commit-Position: refs/branch-heads/6478@{#1962}
Cr-Branched-From: e6143acc03189c5e52959545b110d6d17ecd5286-refs/heads/main@{#1300313}
diff --git a/third_party/blink/renderer/modules/webaudio/audio_param_handler.h b/third_party/blink/renderer/modules/webaudio/audio_param_handler.h
index 6343c4863b5d325a360489a3f3aab3df860d870c..128f283f8d6150cf8b9062ae034d1112930892fc 100644
--- a/third_party/blink/renderer/modules/webaudio/audio_param_handler.h
+++ b/third_party/blink/renderer/modules/webaudio/audio_param_handler.h
@@ -123,8 +123,12 @@ class AudioParamHandler final : public ThreadSafeRefCounted<AudioParamHandler>,
float Value();
void SetValue(float);
- AutomationRate GetAutomationRate() const { return automation_rate_; }
+ AutomationRate GetAutomationRate() const {
+ base::AutoLock rate_locker(RateLock());
+ return automation_rate_;
+ }
void SetAutomationRate(AutomationRate automation_rate) {
+ base::AutoLock rate_locker(RateLock());
automation_rate_ = automation_rate;
}
@@ -163,6 +167,8 @@ class AudioParamHandler final : public ThreadSafeRefCounted<AudioParamHandler>,
return intrinsic_value_.load(std::memory_order_relaxed);
}
+ base::Lock& RateLock() const { return rate_lock_; }
+
private:
AudioParamHandler(BaseAudioContext&,
AudioParamType,
@@ -195,8 +201,12 @@ class AudioParamHandler final : public ThreadSafeRefCounted<AudioParamHandler>,
float default_value_;
+ // Protects `automation_rate_`.
+ mutable base::Lock rate_lock_;
+
// The automation rate of the AudioParam (k-rate or a-rate)
AutomationRate automation_rate_;
+
// `rate_mode_` determines if the user can change the automation rate to a
// different value.
const AutomationRateMode rate_mode_;
diff --git a/third_party/blink/renderer/modules/webaudio/delay_handler.cc b/third_party/blink/renderer/modules/webaudio/delay_handler.cc
index ff3a2ffac5ebeb3841bdbf21bc95981f182358d3..5548e27689acba1c3b301a19567256bf138ebb7f 100644
--- a/third_party/blink/renderer/modules/webaudio/delay_handler.cc
+++ b/third_party/blink/renderer/modules/webaudio/delay_handler.cc
@@ -59,21 +59,27 @@ void DelayHandler::Process(uint32_t frames_to_process) {
source_bus->Zero();
}
- base::AutoTryLock try_locker(process_lock_);
- if (try_locker.is_acquired()) {
+ base::AutoTryLock process_try_locker(process_lock_);
+ base::AutoTryLock rate_try_locker(delay_time_->RateLock());
+ if (process_try_locker.is_acquired() && rate_try_locker.is_acquired()) {
DCHECK_EQ(source_bus->NumberOfChannels(),
destination_bus->NumberOfChannels());
DCHECK_EQ(source_bus->NumberOfChannels(), kernels_.size());
- for (unsigned i = 0; i < kernels_.size(); ++i) {
- if (delay_time_->HasSampleAccurateValues() &&
- delay_time_->IsAudioRate()) {
+ if (delay_time_->IsAudioRate()) {
+ for (unsigned i = 0; i < kernels_.size(); ++i) {
+ // Assumes that the automation rate cannot change in the middle of
+ // the process function. (See crbug.com/357391257)
+ CHECK(delay_time_->IsAudioRate());
delay_time_->CalculateSampleAccurateValues(kernels_[i]->DelayTimes(),
frames_to_process);
kernels_[i]->ProcessARate(source_bus->Channel(i)->Data(),
destination_bus->Channel(i)->MutableData(),
frames_to_process);
- } else {
+ }
+ } else {
+ for (unsigned i = 0; i < kernels_.size(); ++i) {
+ CHECK(!delay_time_->IsAudioRate());
kernels_[i]->SetDelayTime(delay_time_->FinalValue());
kernels_[i]->ProcessKRate(source_bus->Channel(i)->Data(),
destination_bus->Channel(i)->MutableData(),
diff --git a/third_party/blink/web_tests/webaudio/AudioParam/audioparam-rate-change-357391257.html b/third_party/blink/web_tests/webaudio/AudioParam/audioparam-rate-change-357391257.html
new file mode 100644
index 0000000000000000000000000000000000000000..e2d8b9aacd25e86a9b0a5f28524777590280c305
--- /dev/null
+++ b/third_party/blink/web_tests/webaudio/AudioParam/audioparam-rate-change-357391257.html
@@ -0,0 +1,91 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <meta name="timeout" content="long"/>
+ <title>
+ AudioParam automateRate property change - crbug.com/357391257
+ </title>
+ <script src="../../resources/testharness.js"></script>
+ <script src="../../resources/testharnessreport.js"></script>
+</head>
+<body>
+ <script>
+ const t = async_test('audio-param-rate-change-357391257');
+
+ // The problematic value used in the reproduction code.
+ const testValue = 5;
+
+ // Number of iterations for triggering the issue. A high value can strain
+ // testing resources. Empirically determined: the reported repro case was
+ // aboe to crash in 3 iterations on average.
+ const maxIteration = 3;
+
+ // The original repro only has setValueAtTime() but the fix/test covers
+ // all methods.
+ const subtestTypes = [
+ 'setValueAtTime',
+ 'linearRampToValueAtTime',
+ 'exponentialRampToValueAtTime',
+ 'linearRampToValueAtTime',
+ 'setTargetAtTime',
+ 'setValueCurveAtTime'
+ ];
+
+ let subtestsCompleted = 0;
+
+ const runTest = (iteration, subtestType) => {
+ const context = new AudioContext({sampleRate: 768000});
+ const scriptNode = context.createScriptProcessor();
+ const delayNode = context.createDelay(1);
+
+ scriptNode.onaudioprocess = () => {
+ delayNode.delayTime.automationRate = 'k-rate';
+ delayNode.delayTime.automationRate = 'a-rate';
+ };
+ delayNode.delayTime.linearRampToValueAtTime(1, 2);
+ scriptNode.connect(delayNode).connect(context.destination);
+
+ switch (subtestTypes[subtestType]) {
+ case 'setValueAtTime':
+ delayNode.delayTime.setValueAtTime(testValue, context.currentTime);
+ break;
+ case 'linearRampToValueAtTime':
+ delayNode.delayTime.linearRampToValueAtTime(
+ testValue, context.currentTime);
+ break;
+ case 'exponentialRampToValueAtTime':
+ delayNode.delayTime.exponentialRampToValueAtTime(
+ testValue, context.currentTime);
+ break;
+ case 'setTargetAtTime':
+ delayNode.delayTime.setTargetAtTime(
+ testValue, context.currentTime, 0);
+ break;
+ case 'setValueCurveAtTime':
+ const curve = new Float32Array(2);
+ curve[0] = testValue;
+ curve[1] = 0;
+ // To avoid the schedule overlap with setValueAtTime() above, start
+ // the automation at 2.5s.
+ delayNode.delayTime.setValueCurveAtTime(curve, 2.5, 1);
+ break;
+ defaut:
+ assert_unreached('invalid method test type');
+ }
+
+ if (iteration < maxIteration) {
+ setTimeout(() => runTest(iteration + 1, subtestType), 100);
+ } else {
+ if (++subtestsCompleted === subtestTypes.length) {
+ context.close();
+ t.done();
+ }
+ }
+ };
+
+ window.addEventListener('load', t.step_func(() => {
+ subtestTypes.forEach((_, subtestType) => runTest(0, subtestType));
+ }));
+ </script>
+</body>
+</html>

90
patches/chromium/m126-lts_reland_fix_stringview_to_crash_when_offset_length.patch Normal file
View file

@ -0,0 +1,90 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Koji Ishii <kojii@chromium.org>
Date: Thu, 12 Sep 2024 06:17:42 +0000
Subject: [M126-LTS] Reland "Fix `StringView` to crash when `offset + length`
overflows"
This is a reland of commit ba40b993a6b700a2ad0fd092e141783fb1f60e70
The original change failed on mac11-arm64-rel and reverted at
crrev.com/c/5776005. This is because the unit tests assumed
that the `SECURITY_DCHECK` is always enabled, but it's
actually enabled only for DCHECK-enabled builds.
This patch fixes it by wrapping the unit tests by `#if`.
Original change's description:
> Fix `StringView` to crash when `offset + length` overflows
>
> This patch fixes `SECURITY_DCHECK` in `StringView` for when
> `offset + length` overflows the `unsigned`.
>
> Bug: 357622693, 355731798
> Change-Id: I5a7a7979192fe132496661b1272c5902cdbdb09a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5775486
> Auto-Submit: Koji Ishii <kojii@chromium.org>
> Commit-Queue: Kent Tamura <tkent@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1340005}
(cherry picked from commit 5fe8d13101707cfe668bab004fe705241a12b11d)
Bug: 357622693, 355731798
Change-Id: I5402234a5fe54bf8dec2c986ab0ab388e1bc783d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5782718
Commit-Queue: Koji Ishii <kojii@chromium.org>
Auto-Submit: Koji Ishii <kojii@chromium.org>
Commit-Queue: Kentaro Hara <haraken@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1340817}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5807454
Auto-Submit: Roger Felipe Zanoni da Silva (xWF) <rzanoni@google.com>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Fahad Mansoor <fahadmansoor@google.com>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/branch-heads/6478@{#1960}
Cr-Branched-From: e6143acc03189c5e52959545b110d6d17ecd5286-refs/heads/main@{#1300313}
diff --git a/third_party/blink/renderer/platform/wtf/text/string_view.h b/third_party/blink/renderer/platform/wtf/text/string_view.h
index ba3ea1a57f44a51cc3d7b23efc40880e3c421175..96ff7f092c12bceaa8f603fdca10582f988e56d5 100644
--- a/third_party/blink/renderer/platform/wtf/text/string_view.h
+++ b/third_party/blink/renderer/platform/wtf/text/string_view.h
@@ -284,7 +284,8 @@ inline StringView::StringView(const StringView& view,
unsigned offset,
unsigned length)
: impl_(view.impl_), length_(length) {
- SECURITY_DCHECK(offset + length <= view.length());
+ SECURITY_DCHECK(offset <= view.length());
+ SECURITY_DCHECK(length <= view.length() - offset);
if (Is8Bit())
bytes_ = view.Characters8() + offset;
else
@@ -330,7 +331,8 @@ inline void StringView::Clear() {
inline void StringView::Set(const StringImpl& impl,
unsigned offset,
unsigned length) {
- SECURITY_DCHECK(offset + length <= impl.length());
+ SECURITY_DCHECK(offset <= impl.length());
+ SECURITY_DCHECK(length <= impl.length() - offset);
length_ = length;
impl_ = const_cast<StringImpl*>(&impl);
if (impl.Is8Bit())
diff --git a/third_party/blink/renderer/platform/wtf/text/string_view_test.cc b/third_party/blink/renderer/platform/wtf/text/string_view_test.cc
index d8eb2afaa2ecb8a1fd0fbcc04c4c9b8b3b6cb821..efadac6e98a05ede6270d576dfcdb78dab196431 100644
--- a/third_party/blink/renderer/platform/wtf/text/string_view_test.cc
+++ b/third_party/blink/renderer/platform/wtf/text/string_view_test.cc
@@ -374,6 +374,16 @@ TEST(StringViewTest, ConstructionLiteral16) {
EXPECT_EQ(String("12"), StringView(kChars16, 2u));
}
+#if ENABLE_SECURITY_ASSERT
+TEST(StringViewTest, OverflowInConstructor) {
+ EXPECT_DEATH_IF_SUPPORTED(StringView(StringView("12"), 2, -1), "");
+}
+
+TEST(StringViewTest, OverflowInSet) {
+ EXPECT_DEATH_IF_SUPPORTED(StringView(String("12"), 2, -1), "");
+}
+#endif // ENABLE_SECURITY_ASSERT
+
TEST(StringViewTest, IsEmpty) {
EXPECT_FALSE(StringView(kChars).empty());
EXPECT_TRUE(StringView(kChars, 0).empty());

3
patches/config.json
View file

@ -12,5 +12,6 @@
{ "patch_dir": "src/electron/patches/ReactiveObjC", "repo": "src/third_party/squirrel.mac/vendor/ReactiveObjC" },
{ "patch_dir": "src/electron/patches/webrtc", "repo": "src/third_party/webrtc" },
{ "patch_dir": "src/electron/patches/reclient-configs", "repo": "src/third_party/engflow-reclient-configs" },
{ "patch_dir": "src/electron/patches/skia", "repo": "src/third_party/skia" }
{ "patch_dir": "src/electron/patches/skia", "repo": "src/third_party/skia" },
{ "patch_dir": "src/electron/patches/DirectXShaderCompiler", "repo": "src/third_party/dawn/third_party/dxc" }
]

5
patches/skia/.patches
View file

@ -1 +1,6 @@
disallow_sksl_when_deserializing_drawables_in_custom_typefaces.patch
m126-lts_sksl_rp_prevent_overflow_when_computing_slot_allocation.patch
m126-lts_ganesh_avoid_int_overflow_when_combining_regionops.patch
m126-lts_ganesh_fix_meshop_index_combination_logic.patch
ganesh_avoid_int_overflow_in_patternhelper.patch
m126-lts_ganesh_avoid_int_overflow_in_drawatlasopimpl.patch

36
patches/skia/disallow_sksl_when_deserializing_drawables_in_custom_typefaces.patch Normal file
View file

@ -0,0 +1,36 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Brian Osman <brianosman@google.com>
Date: Tue, 6 Aug 2024 14:19:00 -0400
Subject: Disallow SkSL when deserializing drawables in custom typefaces
Bug: 355465305
Change-Id: Ifb87db5e8d0d0c29449e6a3e82254189e3f2d33b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/886696
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
(cherry picked from commit 05097fb7293043906fd8aa118c6adc3012c5b074)
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/888577
diff --git a/src/utils/SkCustomTypeface.cpp b/src/utils/SkCustomTypeface.cpp
index f7c57ecc98587728be75a24c279abc7cc276aae4..61d17f3e5321a70adcd1ae2d82343170ff7e80ef 100644
--- a/src/utils/SkCustomTypeface.cpp
+++ b/src/utils/SkCustomTypeface.cpp
@@ -23,6 +23,7 @@
#include "include/core/SkRect.h"
#include "include/core/SkRefCnt.h"
#include "include/core/SkScalar.h"
+#include "include/core/SkSerialProcs.h"
#include "include/core/SkStream.h"
#include "include/core/SkString.h"
#include "include/core/SkTypeface.h"
@@ -482,7 +483,9 @@ sk_sp<SkTypeface> SkCustomTypefaceBuilder::Deserialize(SkStream* stream) {
switch (gtype) {
case GlyphType::kDrawable: {
- auto drawable = SkDrawable::Deserialize(data->data(), data->size());
+ SkDeserialProcs procs;
+ procs.fAllowSkSL = false;
+ auto drawable = SkDrawable::Deserialize(data->data(), data->size(), &procs);
if (!drawable) {
return nullptr;
}

158
patches/skia/ganesh_avoid_int_overflow_in_patternhelper.patch Normal file
View file

@ -0,0 +1,158 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: James Godfrey-Kittle <jamesgk@google.com>
Date: Wed, 28 Aug 2024 10:48:21 -0400
Subject: [ganesh] Avoid int overflow in PatternHelper
The callers of PatternHelper which are not updated here pass in a TArray
size as repeatCount, which already prevents overflow:
https://crsrc.org/c/third_party/skia/include/private/base/SkTArray.h?q=kMaxCapacity
Bug: b/361461526
Change-Id: I86c494cb00223f0bb8d68540d33d7230b60c9486
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/893916
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: James Godfrey-Kittle <jamesgk@google.com>
(cherry picked from commit 07fcb9a00233cace0b6cc19ed4bcec6770e0315f)
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/896478
diff --git a/src/gpu/ganesh/ops/DashOp.cpp b/src/gpu/ganesh/ops/DashOp.cpp
index 6f12cc38aa36fb779375b187ea72138397adcf43..56303cd9e87d56973b00dd0c81b803e48a7d8479 100644
--- a/src/gpu/ganesh/ops/DashOp.cpp
+++ b/src/gpu/ganesh/ops/DashOp.cpp
@@ -8,6 +8,7 @@
#include "src/gpu/ganesh/ops/DashOp.h"
#include "include/gpu/GrRecordingContext.h"
+#include "src/base/SkSafeMath.h"
#include "src/core/SkMatrixPriv.h"
#include "src/core/SkPointPriv.h"
#include "src/gpu/BufferWriter.h"
@@ -354,6 +355,7 @@ private:
STArray<kNumStackDashes, SkRect, true> rects;
STArray<kNumStackDashes, DashDraw, true> draws;
+ SkSafeMath safeMath;
int totalRectCount = 0;
int rectOffset = 0;
rects.push_back_n(3 * instanceCount);
@@ -520,9 +522,9 @@ private:
devIntervals[0] = lineLength;
}
- totalRectCount += !lineDone ? 1 : 0;
- totalRectCount += hasStartRect ? 1 : 0;
- totalRectCount += hasEndRect ? 1 : 0;
+ totalRectCount = safeMath.addInt(totalRectCount, !lineDone ? 1 : 0);
+ totalRectCount = safeMath.addInt(totalRectCount, hasStartRect ? 1 : 0);
+ totalRectCount = safeMath.addInt(totalRectCount, hasEndRect ? 1 : 0);
if (SkPaint::kRound_Cap == cap && 0 != args.fSrcStrokeWidth) {
// need to adjust this for round caps to correctly set the dashPos attrib on
@@ -562,7 +564,7 @@ private:
draw.fHasEndRect = hasEndRect;
}
- if (!totalRectCount) {
+ if (!totalRectCount || !safeMath) {
return;
}
diff --git a/src/gpu/ganesh/ops/DrawAtlasOp.cpp b/src/gpu/ganesh/ops/DrawAtlasOp.cpp
index 065011699f755b3c87f6cf9a9b19e4d5d42e91df..a3d7e4ddabb1a29ec50b5e1aab88cabcf1445104 100644
--- a/src/gpu/ganesh/ops/DrawAtlasOp.cpp
+++ b/src/gpu/ganesh/ops/DrawAtlasOp.cpp
@@ -10,6 +10,7 @@
#include "include/core/SkRSXform.h"
#include "include/gpu/GrRecordingContext.h"
#include "src/base/SkRandom.h"
+#include "src/base/SkSafeMath.h"
#include "src/core/SkMatrixPriv.h"
#include "src/core/SkRectPriv.h"
#include "src/gpu/ganesh/GrCaps.h"
@@ -280,8 +281,14 @@ GrOp::CombineResult DrawAtlasOpImpl::onCombineIfPossible(GrOp* t,
return CombineResult::kCannotCombine;
}
+ SkSafeMath safeMath;
+ int newQuadCount = safeMath.addInt(fQuadCount, that->quadCount());
+ if (!safeMath) {
+ return CombineResult::kCannotCombine;
+ }
+
fGeoData.push_back_n(that->fGeoData.size(), that->fGeoData.begin());
- fQuadCount += that->quadCount();
+ fQuadCount = newQuadCount;
return CombineResult::kMerged;
}
diff --git a/src/gpu/ganesh/ops/GrMeshDrawOp.cpp b/src/gpu/ganesh/ops/GrMeshDrawOp.cpp
index 4fdf90b0381996609773e952674aad149b29b98d..5b885f0fd1cae6602d7f0982b91cc2a0af7a8b3b 100644
--- a/src/gpu/ganesh/ops/GrMeshDrawOp.cpp
+++ b/src/gpu/ganesh/ops/GrMeshDrawOp.cpp
@@ -7,6 +7,7 @@
#include "src/gpu/ganesh/ops/GrMeshDrawOp.h"
+#include "include/private/base/SkMath.h"
#include "src/gpu/ganesh/GrOpFlushState.h"
#include "src/gpu/ganesh/GrOpsRenderPass.h"
#include "src/gpu/ganesh/GrRecordingContextPriv.h"
@@ -81,6 +82,12 @@ void GrMeshDrawOp::PatternHelper::init(GrMeshDrawTarget* target, GrPrimitiveType
if (!indexBuffer) {
return;
}
+
+ // Bail out when we get overflow from really large draws.
+ if (repeatCount < 0 || repeatCount > SK_MaxS32 / verticesPerRepetition) {
+ return;
+ }
+
sk_sp<const GrBuffer> vertexBuffer;
int firstVertex;
int vertexCount = verticesPerRepetition * repeatCount;
diff --git a/src/gpu/ganesh/ops/LatticeOp.cpp b/src/gpu/ganesh/ops/LatticeOp.cpp
index d1ae7ad1fa8e06f5fd862867118bd7688ccf209b..ce9d08ac89ee801177fbf7d8fb1466dd68713915 100644
--- a/src/gpu/ganesh/ops/LatticeOp.cpp
+++ b/src/gpu/ganesh/ops/LatticeOp.cpp
@@ -9,6 +9,7 @@
#include "include/core/SkBitmap.h"
#include "include/core/SkRect.h"
+#include "src/base/SkSafeMath.h"
#include "src/core/SkLatticeIter.h"
#include "src/core/SkMatrixPriv.h"
#include "src/gpu/BufferWriter.h"
@@ -240,11 +241,13 @@ private:
int patchCnt = fPatches.size();
int numRects = 0;
+
+ SkSafeMath safeMath;
for (int i = 0; i < patchCnt; i++) {
- numRects += fPatches[i].fIter->numRectsToDraw();
+ numRects = safeMath.addInt(numRects, fPatches[i].fIter->numRectsToDraw());
}
- if (!numRects) {
+ if (!numRects || !safeMath) {
return;
}
diff --git a/src/gpu/ganesh/ops/RegionOp.cpp b/src/gpu/ganesh/ops/RegionOp.cpp
index 58383775bc8bdd0e98125df01c538e03ef1ebc69..2ff9d648db32ad9b115c39c7321d3491fb13ae04 100644
--- a/src/gpu/ganesh/ops/RegionOp.cpp
+++ b/src/gpu/ganesh/ops/RegionOp.cpp
@@ -122,12 +122,8 @@ private:
for (int i = 0; i < numRegions; i++) {
numRects = safeMath.addInt(numRects, fRegions[i].fRegion.computeRegionComplexity());
}
- if (!safeMath) {
- // This is a nonsensical draw, so we can just drop it.
- return;
- }
- if (!numRects) {
+ if (!numRects || !safeMath) {
return;
}

44
patches/skia/m126-lts_ganesh_avoid_int_overflow_when_combining_regionops.patch Normal file
View file

@ -0,0 +1,44 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: James Godfrey-Kittle <jamesgk@google.com>
Date: Tue, 20 Aug 2024 14:35:00 -0400
Subject: [M126-LTS][ganesh] Avoid int overflow when combining RegionOps
M126 merge issues:
Conflicting includes
Bug: b/360758697
Change-Id: I46eb92ac6ed71646fb05a910f8d577ec851e3b3f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/891636
Commit-Queue: James Godfrey-Kittle <jamesgk@google.com>
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/894463
Commit-Queue: Roger Felipe Zanoni da Silva (xWF) <rzanoni@google.com>
Reviewed-by: James Godfrey-Kittle <jamesgk@google.com>
diff --git a/src/gpu/ganesh/ops/RegionOp.cpp b/src/gpu/ganesh/ops/RegionOp.cpp
index eb4975052aeef3ca1f1f9e2e078b44b44d951e9d..58383775bc8bdd0e98125df01c538e03ef1ebc69 100644
--- a/src/gpu/ganesh/ops/RegionOp.cpp
+++ b/src/gpu/ganesh/ops/RegionOp.cpp
@@ -8,6 +8,7 @@
#include "src/gpu/ganesh/ops/RegionOp.h"
#include "include/core/SkRegion.h"
+#include "src/base/SkSafeMath.h"
#include "src/core/SkMatrixPriv.h"
#include "src/gpu/BufferWriter.h"
#include "src/gpu/ganesh/GrCaps.h"
@@ -116,8 +117,14 @@ private:
int numRegions = fRegions.size();
int numRects = 0;
+
+ SkSafeMath safeMath;
for (int i = 0; i < numRegions; i++) {
- numRects += fRegions[i].fRegion.computeRegionComplexity();
+ numRects = safeMath.addInt(numRects, fRegions[i].fRegion.computeRegionComplexity());
+ }
+ if (!safeMath) {
+ // This is a nonsensical draw, so we can just drop it.
+ return;
}
if (!numRects) {

33
patches/skia/m126-lts_ganesh_fix_meshop_index_combination_logic.patch Normal file
View file

@ -0,0 +1,33 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Michael Ludwig <michaelludwig@google.com>
Date: Mon, 19 Aug 2024 10:12:20 -0400
Subject: [M126-LTS][ganesh] Fix MeshOp index combination logic
Check total index count in onCombineIfPossible.
Bug: b/360265320
Change-Id: I02f04593b60dcd2470580110d0a555ed4bf47280
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/890322
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
(cherry picked from commit fdc8c2d593f7dc95b3a98216ec6a4ffa23489516)
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/894464
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Roger Felipe Zanoni da Silva (xWF) <rzanoni@google.com>
diff --git a/src/gpu/ganesh/ops/DrawMeshOp.cpp b/src/gpu/ganesh/ops/DrawMeshOp.cpp
index ff5fb0aeffdd447eb581ba7199ee699a81527c07..5061feffd5102a8e72760d0f41b7510038bcac50 100644
--- a/src/gpu/ganesh/ops/DrawMeshOp.cpp
+++ b/src/gpu/ganesh/ops/DrawMeshOp.cpp
@@ -1179,7 +1179,11 @@ GrOp::CombineResult MeshOp::onCombineIfPossible(GrOp* t, SkArenaAlloc*, const Gr
if (SkToBool(fIndexCount) != SkToBool(that->fIndexCount)) {
return CombineResult::kCannotCombine;
}
- if (SkToBool(fIndexCount) && fVertexCount > SkToInt(UINT16_MAX) - that->fVertexCount) {
+ if (SkToBool(fIndexCount) &&
+ // Index count would overflow
+ (fIndexCount > INT32_MAX - that->fIndexCount ||
+ // *or* combined vertex count would not be referenceable by uint16 indices
+ fVertexCount > SkToInt(UINT16_MAX) - that->fVertexCount)) {
return CombineResult::kCannotCombine;
}

202
patches/skia/m126-lts_sksl_rp_prevent_overflow_when_computing_slot_allocation.patch Normal file
View file

@ -0,0 +1,202 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Brian Osman <brianosman@google.com>
Date: Fri, 9 Aug 2024 14:50:21 -0400
Subject: [M126-LTS][SkSL:RP] Prevent overflow when computing slot allocation
size
Bug: 355465305
Change-Id: Ife25289f7b3489701c67b7dc5d30e473019a1193
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/888376
Commit-Queue: Brian Osman <brianosman@google.com>
(cherry picked from commit d1b243ba90f0698ced6fadc460adb9d66c248946)
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/896658
Commit-Queue: Roger Felipe Zanoni da Silva (xWF) <rzanoni@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
diff --git a/src/sksl/codegen/SkSLRasterPipelineBuilder.cpp b/src/sksl/codegen/SkSLRasterPipelineBuilder.cpp
index e8c5c56fa27b633a2be95d1e878a0a2976ecafd8..fab86abd2889bc7397e09b7ef006a8a5c2485f5c 100644
--- a/src/sksl/codegen/SkSLRasterPipelineBuilder.cpp
+++ b/src/sksl/codegen/SkSLRasterPipelineBuilder.cpp
@@ -6,11 +6,15 @@
*/
#include "src/sksl/codegen/SkSLRasterPipelineBuilder.h"
+#include <cstdint>
+#include <optional>
#include "include/core/SkStream.h"
#include "include/private/base/SkMalloc.h"
+#include "include/private/base/SkTFitsIn.h"
#include "include/private/base/SkTo.h"
#include "src/base/SkArenaAlloc.h"
+#include "src/base/SkSafeMath.h"
#include "src/core/SkOpts.h"
#include "src/core/SkRasterPipelineContextUtils.h"
#include "src/core/SkRasterPipelineOpContexts.h"
@@ -1631,13 +1635,17 @@ static void* context_bit_pun(intptr_t val) {
return sk_bit_cast<void*>(val);
}
-Program::SlotData Program::allocateSlotData(SkArenaAlloc* alloc) const {
+std::optional<Program::SlotData> Program::allocateSlotData(SkArenaAlloc* alloc) const {
// Allocate a contiguous slab of slot data for immutables, values, and stack entries.
const int N = SkOpts::raster_pipeline_highp_stride;
const int scalarWidth = 1 * sizeof(float);
const int vectorWidth = N * sizeof(float);
- const int allocSize = vectorWidth * (fNumValueSlots + fNumTempStackSlots) +
- scalarWidth * fNumImmutableSlots;
+ SkSafeMath safe;
+ size_t allocSize = safe.add(safe.mul(vectorWidth, safe.add(fNumValueSlots, fNumTempStackSlots)),
+ safe.mul(scalarWidth, fNumImmutableSlots));
+ if (!safe || !SkTFitsIn<int>(allocSize)) {
+ return std::nullopt;
+ }
float* slotPtr = static_cast<float*>(alloc->makeBytesAlignedTo(allocSize, vectorWidth));
sk_bzero(slotPtr, allocSize);
@@ -1658,8 +1666,11 @@ bool Program::appendStages(SkRasterPipeline* pipeline,
#else
// Convert our Instruction list to an array of ProgramOps.
TArray<Stage> stages;
- SlotData slotData = this->allocateSlotData(alloc);
- this->makeStages(&stages, alloc, uniforms, slotData);
+ std::optional<SlotData> slotData = this->allocateSlotData(alloc);
+ if (!slotData) {
+ return false;
+ }
+ this->makeStages(&stages, alloc, uniforms, *slotData);
// Allocate buffers for branch targets and labels; these are needed to convert labels into
// actual offsets into the pipeline and fix up branches.
@@ -1673,7 +1684,7 @@ bool Program::appendStages(SkRasterPipeline* pipeline,
auto resetBasePointer = [&]() {
// Whenever we hand off control to another shader, we have to assume that it might overwrite
// the base pointer (if it uses SkSL, it will!), so we reset it on return.
- pipeline->append(SkRasterPipelineOp::set_base_pointer, slotData.values.data());
+ pipeline->append(SkRasterPipelineOp::set_base_pointer, (*slotData).values.data());
};
resetBasePointer();
@@ -2844,7 +2855,7 @@ void Program::Dumper::dump(SkWStream* out, bool writeInstructionCount) {
// executed. The program requires pointer ranges for managing its data, and ASAN will report
// errors if those pointers are pointing at unallocated memory.
SkArenaAlloc alloc(/*firstHeapAllocation=*/1000);
- fSlots = fProgram.allocateSlotData(&alloc);
+ fSlots = fProgram.allocateSlotData(&alloc).value();
float* uniformPtr = alloc.makeArray<float>(fProgram.fNumUniformSlots);
fUniforms = SkSpan(uniformPtr, fProgram.fNumUniformSlots);
diff --git a/src/sksl/codegen/SkSLRasterPipelineBuilder.h b/src/sksl/codegen/SkSLRasterPipelineBuilder.h
index e73543b777d078fa629864a5aad97b75fb829220..acb35c6cc35d0e33e045010b8d571a8c973bbea3 100644
--- a/src/sksl/codegen/SkSLRasterPipelineBuilder.h
+++ b/src/sksl/codegen/SkSLRasterPipelineBuilder.h
@@ -19,6 +19,7 @@
#include <cstddef>
#include <cstdint>
#include <memory>
+#include <optional>
class SkArenaAlloc;
class SkRasterPipeline;
@@ -176,7 +177,7 @@ private:
SkSpan<float> stack;
SkSpan<float> immutable;
};
- SlotData allocateSlotData(SkArenaAlloc* alloc) const;
+ std::optional<SlotData> allocateSlotData(SkArenaAlloc* alloc) const;
struct Stage {
ProgramOp op;
diff --git a/tests/RasterPipelineCodeGeneratorTest.cpp b/tests/RasterPipelineCodeGeneratorTest.cpp
index 9da6e61a36fa59fc6ecf067d5643cc839d0e254f..24903c787414558864b4b9e1ef2c90c24f95f436 100644
--- a/tests/RasterPipelineCodeGeneratorTest.cpp
+++ b/tests/RasterPipelineCodeGeneratorTest.cpp
@@ -22,6 +22,7 @@
#include <memory>
#include <optional>
+#include <sstream>
#include <string>
//#define DUMP_PROGRAMS 1
@@ -250,3 +251,80 @@ DEF_TEST(SkSLRasterPipelineCodeGeneratorComparisonIntrinsicTest, r) {
/*startingColor=*/SkColor4f{0.0, 0.0, 0.0, 0.0},
/*expectedResult=*/SkColor4f{0.0, 1.0, 0.0, 1.0});
}
+
+DEF_TEST(SkSLRasterPipelineSlotOverflow_355465305, r) {
+ constexpr int kStructMembers1 = 6200;
+ constexpr int kStructMembers2 = 433;
+ std::stringstream str;
+ str << "struct M { float4x4 m";
+ for (int i = 1; i < kStructMembers1; ++i) {
+ str << ",m" << i;
+ }
+ str << ";};";
+ str << "struct M2 { float4x4 m";
+ for (int i = 1; i < kStructMembers2; ++i) {
+ str << ",m" << i;
+ }
+ str << ";};";
+ str << "M f() { M m; return m; }";
+ constexpr int kConstMembers = 40;
+ str << "struct T { float4x4 m0";
+ for (int i = 1; i < kConstMembers; ++i) {
+ str << ",m" << i;
+ }
+ str << ";};";
+ str << "const T K = T(";
+ for (int i = 0; i < kConstMembers; ++i) {
+ if (i > 0) {
+ str << ",";
+ }
+ str << "mat4x4(1337)";
+ }
+ str << ");";
+ str << "half4 main(half4 color) {";
+ str << "float4x4 a = M2(";
+ for (int j = 0; j < kStructMembers2; ++j) {
+ if (j > 0) {
+ str << ",";
+ }
+ const int numAddOps = (j == kStructMembers1 - 1) ? 23 : 25;
+ for (int i = 0; i < numAddOps; ++i) {
+ if (i > 0) {
+ str << "+";
+ }
+ str << "f().m";
+ }
+ }
+ str << ").m;";
+ str << "return half4(a[0]+(K.m0+K.m1+K.m2+K.m3)[0]);";
+ str << "}";
+ std::string src = str.str();
+
+ SkSL::Compiler compiler;
+ std::unique_ptr<SkSL::Program> program =
+ compiler.convertProgram(SkSL::ProgramKind::kRuntimeColorFilter, src, {});
+ if (!program) {
+ ERRORF(r, "Unexpected error compiling %s\n%s", src.c_str(), compiler.errorText().c_str());
+ return;
+ }
+ const SkSL::FunctionDeclaration* main = program->getFunction("main");
+ if (!main) {
+ ERRORF(r, "Program must have a 'main' function");
+ return;
+ }
+ SkArenaAlloc alloc(1000);
+ SkRasterPipeline pipeline(&alloc);
+ pipeline.appendConstantColor(&alloc, SkColors::kWhite);
+ std::unique_ptr<SkSL::RP::Program> rasterProg =
+ SkSL::MakeRasterPipelineProgram(*program, *main->definition());
+ // Ideally, this program would fail in the front-end, because of the number of slots needed
+ // for expression evaluation. For now, it succeeds (but then fails in appendStages).
+ if (!rasterProg) {
+ ERRORF(r, "MakeRasterPipelineProgram failed");
+ return;
+ }
+
+ // Append the SkSL program to the raster pipeline.
+ bool success = rasterProg->appendStages(&pipeline, &alloc, /*callbacks=*/nullptr, {});
+ REPORTER_ASSERT(r, !success, "appendStages should fail for very large program");
+}

4
patches/v8/.patches
View file

@ -6,5 +6,9 @@ spill_all_loop_inputs_before_entering_loop.patch
cherry-pick-9542895cdd3d.patch
cherry-pick-81155a8f3b20.patch
cherry-pick-f612d9a40b19.patch
m126-lts_compiler_clear_stale_data_for_zeroextendsword32toword64.patch
merged_turbofan_handle_type_none_in_samevalue.patch
m126-lts_wasm_don_t_catch_uncatchable_exceptions_in_the_jspi.patch
merged_heap_sandbox_update_ept_s_evacuation_entries_in_scavenger.patch
merged_don_t_assume_all_turbofan_frames_are_javascript.patch
merged_wasm_do_not_inline_wrappers_with_ref_extern_parameter.patch

58
patches/v8/m126-lts_compiler_clear_stale_data_for_zeroextendsword32toword64.patch Normal file
View file

@ -0,0 +1,58 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Seth Brenith <seth.brenith@microsoft.com>
Date: Tue, 6 Aug 2024 23:08:34 -0700
Subject: [M126-LTS][compiler] Clear stale data for ZeroExtendsWord32ToWord64
The first call to ZeroExtendsWord32ToWord64 produces a correct result,
but leaves some incorrect values in phi_states_. To avoid incorrect
behavior, we should clear those values when starting anew.
I think that the performance impact of this change on compilation time
should be small, because calls to ZeroExtendsWord32ToWord64 are
infrequent. Here is a histogram showing, per function compiled in
Octane, how often this new code is run:
0: 74.7%
1: 13.1%
2: 6.3%
3: 2.5%
4 or 5: 1.7%
6 to 9: 0.9%
11 to 33: 0.8%
(cherry picked from commit 780d5608bb8ab63a3cd4b5c4846a3ec41e21c1a8)
Bug: 356196918
Change-Id: I00a9e74652025bf8a32cb083a6e01c0273e44043
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5766478
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Cr-Original-Commit-Position: refs/heads/main@{#95528}
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5807474
Auto-Submit: Roger Felipe Zanoni da Silva (xWF) <rzanoni@google.com>
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Reviewed-by: Seth Brenith <seth.brenith@microsoft.com>
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/branch-heads/12.6@{#60}
Cr-Branched-From: 3c9fa12db3183a6f4ea53d2675adb66ea1194529-refs/heads/12.6.228@{#2}
Cr-Branched-From: 981bb15ba4dbf9e2381dfc94ec2c4af0b9c6a0b6-refs/heads/main@{#93835}
diff --git a/src/compiler/backend/instruction-selector.cc b/src/compiler/backend/instruction-selector.cc
index 053e8a449ec1ad7ecf2ebf13f548978d7cfafeed..c7a0a4d8cd37ca98f7dc7a8f6dc910e19d515603 100644
--- a/src/compiler/backend/instruction-selector.cc
+++ b/src/compiler/backend/instruction-selector.cc
@@ -5633,6 +5633,14 @@ bool InstructionSelectorT<Adapter>::ZeroExtendsWord32ToWord64(
const int kMaxRecursionDepth = 100;
if (this->IsPhi(node)) {
+ // Intermediate results from previous calls are not necessarily correct.
+ if (recursion_depth == 0) {
+ static_assert(sizeof(Upper32BitsState) == 1);
+ memset(phi_states_.data(),
+ static_cast<int>(Upper32BitsState::kNotYetChecked),
+ phi_states_.size());
+ }
+
Upper32BitsState current = phi_states_[this->id(node)];
if (current != Upper32BitsState::kNotYetChecked) {
return current == Upper32BitsState::kUpperBitsGuaranteedZero;

82
patches/v8/m126-lts_wasm_don_t_catch_uncatchable_exceptions_in_the_jspi.patch Normal file
View file

@ -0,0 +1,82 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Thibaud Michaud <thibaudm@chromium.org>
Date: Tue, 3 Sep 2024 11:50:45 +0200
Subject: [M126-LTS][wasm] Don't catch uncatchable exceptions in the JSPI
wrapper
M126 merge issues:
The HandleStackSwitch function doesn't exist in the LTS branch.
... And forward the exception to the parent stack instead.
R=jkummerow@chromium.org
(cherry picked from commit 9495e79f82f60da191211669e9de1b210d2af1c9)
Fixed: 361717714
Change-Id: I7c6a75b53bc7732546ec6a7a1425ac50b9b1756b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5817264
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#95847}
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5831578
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Roger Felipe Zanoni da Silva (xWF) <rzanoni@google.com>
Cr-Commit-Position: refs/branch-heads/12.6@{#62}
Cr-Branched-From: 3c9fa12db3183a6f4ea53d2675adb66ea1194529-refs/heads/12.6.228@{#2}
Cr-Branched-From: 981bb15ba4dbf9e2381dfc94ec2c4af0b9c6a0b6-refs/heads/main@{#93835}
diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc
index c3db834a8b8a9f28d32860336347df169b808043..bf4d6b90626a6e8eb98913fb2e524c9e87dd6e3c 100644
--- a/src/execution/isolate.cc
+++ b/src/execution/isolate.cc
@@ -2049,6 +2049,14 @@ Tagged<Object> Isolate::UnwindAndFindHandler() {
return exception;
};
+#if V8_ENABLE_WEBASSEMBLY
+ Tagged<Object> maybe_continuation = root(RootIndex::kActiveContinuation);
+ Tagged<WasmContinuationObject> continuation;
+ if (!IsUndefined(maybe_continuation)) {
+ continuation = WasmContinuationObject::cast(maybe_continuation);
+ }
+#endif
+
// Special handling of termination exceptions, uncatchable by JavaScript and
// Wasm code, we unwind the handlers until the top ENTRY handler is found.
bool catchable_by_js = is_catchable_by_javascript(exception);
@@ -2067,15 +2075,25 @@ Tagged<Object> Isolate::UnwindAndFindHandler() {
for (StackFrameIterator iter(this);; iter.Advance(), visited_frames++) {
#if V8_ENABLE_WEBASSEMBLY
if (iter.frame()->type() == StackFrame::STACK_SWITCH) {
- Tagged<Code> code =
- builtins()->code(Builtin::kWasmReturnPromiseOnSuspendAsm);
- HandlerTable table(code);
- Address instruction_start =
- code->InstructionStart(this, iter.frame()->pc());
- int handler_offset = table.LookupReturn(0);
- return FoundHandler(Context(), instruction_start, handler_offset,
- kNullAddress, iter.frame()->sp(), iter.frame()->fp(),
- visited_frames);
+ if (catchable_by_js) {
+ Tagged<Code> code =
+ builtins()->code(Builtin::kWasmReturnPromiseOnSuspendAsm);
+ HandlerTable table(code);
+ Address instruction_start =
+ code->InstructionStart(this, iter.frame()->pc());
+ int handler_offset = table.LookupReturn(0);
+ return FoundHandler(Context(), instruction_start, handler_offset,
+ kNullAddress, iter.frame()->sp(),
+ iter.frame()->fp(), visited_frames);
+ } else {
+ // We reached the base of the wasm stack. Follow the chain of
+ // continuations to find the parent stack and reset the iterator.
+ DCHECK(!continuation.is_null());
+ continuation = WasmContinuationObject::cast(continuation->parent());
+ wasm::StackMemory* stack =
+ Managed<wasm::StackMemory>::cast(continuation->stack())->raw();
+ iter.Reset(thread_local_top(), stack);
+ }
}
#endif
// Handler must exist.

226
patches/v8/merged_heap_sandbox_update_ept_s_evacuation_entries_in_scavenger.patch Normal file
View file

@ -0,0 +1,226 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Anton Bikineev <bikineev@chromium.org>
Date: Tue, 27 Aug 2024 11:24:48 +0200
Subject: Merged: heap,sandbox: Update EPT's evacuation entries in Scavenger.
If Scavenger interleaves MarkCompact that performs compaction on EPT,
there may be some evacuation entries allocated in the young EPT that
would back-point to the Scavenger's from-space. Add a new phase that
updates all the evacuation entries in the young EPT up until
`start_of_evacation_area`.
Bug: 358485426
(cherry picked from commit 1a2b08edbec1a8ebcf3d4adc91da4f2569fb744a)
Change-Id: Iadabe3ded39b32d8908e5d4e8fbff593b977940c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5827960
Auto-Submit: Deepti Gandluri <gdeepti@chromium.org>
Reviewed-by: Matthias Liedtke <mliedtke@chromium.org>
Commit-Queue: Deepti Gandluri <gdeepti@chromium.org>
Cr-Commit-Position: refs/branch-heads/12.8@{#50}
Cr-Branched-From: 70cbb397b153166027e34c75adf8e7993858222e-refs/heads/12.8.374@{#1}
Cr-Branched-From: 451b63ed4251c2b21c56144d8428f8be3331539b-refs/heads/main@{#95151}
diff --git a/src/heap/incremental-marking.cc b/src/heap/incremental-marking.cc
index 909817c5afb8deb8c09bee4c11a3459082c75818..94bfd80c085fe21a9fab2ad8887f23cd95748515 100644
--- a/src/heap/incremental-marking.cc
+++ b/src/heap/incremental-marking.cc
@@ -532,6 +532,68 @@ void IncrementalMarking::UpdateMarkingWorklistAfterScavenge() {
weak_objects_->UpdateAfterScavenge();
}
+void IncrementalMarking::UpdateExternalPointerTableAfterScavenge() {
+#ifdef V8_COMPRESS_POINTERS
+ if (!IsMajorMarking()) return;
+ DCHECK(!v8_flags.separate_gc_phases);
+
+ heap_->isolate()->external_pointer_table().UpdateAllEvacuationEntries(
+ heap_->young_external_pointer_space(), [](Address old_handle_location) {
+ // 1) Resolve object start from the marking bitmap. Note that it's safe
+ // since there is no black allocation for the young space (and hence
+ // no range or page marking).
+ // 2) Get a relocated object from the forwaring reference stored in the
+ // map.
+ // 3) Compute offset from the original object start to the handle
+ // location.
+ // 4) Compute and return the new handle location.
+ //
+ // Please note that instead of updating the evacuation entries, we
+ // could simply clobber them all, which would still work, but limit
+ // compaction to some extent. We can reconsider this in the future, if
+ // relying on the marking bitmap becomes an issue (e.g. with inlined
+ // mark-bits).
+ const MemoryChunk* chunk =
+ MemoryChunk::FromAddress(old_handle_location);
+ if (!chunk->InYoungGeneration()) {
+ return old_handle_location;
+ }
+ // TODO(358485426): Check that the page is not black.
+
+ Address base = MarkingBitmap::FindPreviousValidObject(
+ static_cast<const PageMetadata*>(chunk->Metadata()),
+ old_handle_location);
+ Tagged<HeapObject> object(HeapObject::FromAddress(base));
+
+ MapWord map_word = object->map_word(kRelaxedLoad);
+ if (!map_word.IsForwardingAddress()) {
+ // There may be objects in the EPT that do not exist anymore. If these
+ // objects are dead at scavenging time, their marking deque entries will
+ // not point to forwarding addresses. Hence, we can discard them.
+#if DEBUG
+ // Check that the handle did reside inside the original dead object.
+ const int object_size = object->Size();
+ // Map slots can never contain external pointers.
+ DCHECK_LT(object.address(), old_handle_location);
+ DCHECK_LT(old_handle_location, object.address() + object_size);
+#endif // DEBUG
+ return kNullAddress;
+ }
+
+ Tagged<HeapObject> moved_object = map_word.ToForwardingAddress(object);
+#if DEBUG
+ const int object_size = moved_object->Size();
+ // Map slots can never contain external pointers.
+ DCHECK_LT(object.address(), old_handle_location);
+ DCHECK_LT(old_handle_location, object.address() + object_size);
+#endif // DEBUG
+
+ const ptrdiff_t handle_offset = old_handle_location - base;
+ return moved_object.address() + handle_offset;
+ });
+#endif // V8_COMPRESS_POINTERS
+}
+
void IncrementalMarking::UpdateMarkedBytesAfterScavenge(
size_t dead_bytes_in_new_space) {
if (!IsMajorMarking()) return;
diff --git a/src/heap/incremental-marking.h b/src/heap/incremental-marking.h
index d61a43d782f8e565d6823ff87ccd50aa384201ba..b32596db5f9dffb047ac1ce089e4adef6453925c 100644
--- a/src/heap/incremental-marking.h
+++ b/src/heap/incremental-marking.h
@@ -99,6 +99,7 @@ class V8_EXPORT_PRIVATE IncrementalMarking final {
bool Stop();
void UpdateMarkingWorklistAfterScavenge();
+ void UpdateExternalPointerTableAfterScavenge();
void UpdateMarkedBytesAfterScavenge(size_t dead_bytes_in_new_space);
// Performs incremental marking step and finalizes marking if complete.
diff --git a/src/heap/scavenger.cc b/src/heap/scavenger.cc
index e335c8c067aa884e94d30230cafddff82ece280f..892b5880c98a30bb94d16c97292f2b86774974e0 100644
--- a/src/heap/scavenger.cc
+++ b/src/heap/scavenger.cc
@@ -496,6 +496,7 @@ void ScavengerCollector::CollectGarbage() {
&Heap::UpdateYoungReferenceInExternalStringTableEntry);
heap_->incremental_marking()->UpdateMarkingWorklistAfterScavenge();
+ heap_->incremental_marking()->UpdateExternalPointerTableAfterScavenge();
if (V8_UNLIKELY(v8_flags.track_retaining_path)) {
heap_->UpdateRetainersAfterScavenge();
diff --git a/src/sandbox/compactible-external-entity-table.h b/src/sandbox/compactible-external-entity-table.h
index b90abf0277381f430646cf2f1759ecab9ef32905..5fc3de392e7eb1a98598ab69b1074b7f69aac8b9 100644
--- a/src/sandbox/compactible-external-entity-table.h
+++ b/src/sandbox/compactible-external-entity-table.h
@@ -42,7 +42,7 @@ enum class ExternalEntityTableCompactionOutcome {
* compacted. This decision is mostly based on the absolute and relative
* size of the freelist.
* - If compaction is needed, this algorithm determines by how many segments
- * it would like to shrink the space (N). It will then attempts to move all
+ * it would like to shrink the space (N). It will then attempt to move all
* live entries out of these segments so that they can be deallocated
* afterwards during sweeping.
* - The algorithm then simply selects the last N segments for evacuation, and
diff --git a/src/sandbox/external-pointer-table.cc b/src/sandbox/external-pointer-table.cc
index 21298edd3b6c03fb8ffa51359642c1f1da64466a..c5e90dc31addc1a25e20020ad202d04db332d9ea 100644
--- a/src/sandbox/external-pointer-table.cc
+++ b/src/sandbox/external-pointer-table.cc
@@ -42,7 +42,7 @@ class SegmentsIterator {
using const_iterator = typename std::set<Segment>::const_reverse_iterator;
public:
- SegmentsIterator() {}
+ SegmentsIterator() = default;
void AddSegments(const std::set<Segment>& segments, Data data) {
streams_.emplace_back(segments.rbegin(), segments.rend(), data);
@@ -126,7 +126,7 @@ uint32_t ExternalPointerTable::EvacuateAndSweepAndCompact(Space* space,
segments_iter.AddSegments(from_space_segments, from_space_compaction);
FreelistHead empty_freelist;
- from_space->freelist_head_.store(empty_freelist, std::memory_order_release);
+ from_space->freelist_head_.store(empty_freelist, std::memory_order_relaxed);
for (Address field : from_space->invalidated_fields_)
space->invalidated_fields_.push_back(field);
@@ -176,6 +176,13 @@ uint32_t ExternalPointerTable::EvacuateAndSweepAndCompact(Space* space,
Address handle_location =
payload.ExtractEvacuationEntryHandleLocation();
+ // The evacuation entry may be invalidated by the Scavenger that has
+ // freed the object.
+ if (handle_location == kNullAddress) {
+ AddToFreelist(i);
+ continue;
+ }
+
// The external pointer field may have been invalidated in the meantime
// (for example if the host object has been in-place converted to a
// different type of object). In that case, the field no longer
@@ -295,6 +302,40 @@ void ExternalPointerTable::ResolveEvacuationEntryDuringSweeping(
}
}
+void ExternalPointerTable::UpdateAllEvacuationEntries(
+ Space* space, std::function<Address(Address)> function) {
+ DCHECK(space->BelongsTo(this));
+ DCHECK(!space->is_internal_read_only_space());
+
+ if (!space->IsCompacting()) return;
+
+ // Lock the space. Technically this is not necessary since no other thread can
+ // allocate entries at this point, but some of the methods we call on the
+ // space assert that the lock is held.
+ base::MutexGuard guard(&space->mutex_);
+ // Same for the invalidated fields mutex.
+ base::MutexGuard invalidated_fields_guard(&space->invalidated_fields_mutex_);
+
+ const uint32_t start_of_evacuation_area =
+ space->start_of_evacuation_area_.load(std::memory_order_relaxed);
+
+ // Iterate until the start of evacuation area.
+ for (auto& segment : space->segments_) {
+ if (segment.first_entry() == start_of_evacuation_area) return;
+ for (uint32_t i = segment.first_entry(); i < segment.last_entry() + 1;
+ ++i) {
+ ExternalPointerTableEntry& entry = at(i);
+ ExternalPointerTableEntry::Payload payload = entry.GetRawPayload();
+ if (!payload.ContainsEvacuationEntry()) {
+ continue;
+ }
+ Address new_location =
+ function(payload.ExtractEvacuationEntryHandleLocation());
+ entry.MakeEvacuationEntry(new_location);
+ }
+ }
+}
+
} // namespace internal
} // namespace v8
diff --git a/src/sandbox/external-pointer-table.h b/src/sandbox/external-pointer-table.h
index 0527057b6afeccf8f70a7d88510502b40919a974..4ed4195c7d5d7699978a6d6aee67cf2322d2101f 100644
--- a/src/sandbox/external-pointer-table.h
+++ b/src/sandbox/external-pointer-table.h
@@ -401,6 +401,10 @@ class V8_EXPORT_PRIVATE ExternalPointerTable
uint32_t SweepAndCompact(Space* space, Counters* counters);
uint32_t Sweep(Space* space, Counters* counters);
+ // Updates all evacuation entries with new handle locations. The function
+ // takes the old hanlde location and returns the new one.
+ void UpdateAllEvacuationEntries(Space*, std::function<Address(Address)>);
+
inline bool Contains(Space* space, ExternalPointerHandle handle) const;
// A resource outside of the V8 heap whose lifetime is tied to something

30
patches/v8/merged_turbofan_handle_type_none_in_samevalue.patch Normal file
View file

@ -0,0 +1,30 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Nico Hartmann <nicohartmann@chromium.org>
Date: Tue, 20 Aug 2024 16:26:39 +0200
Subject: Merged: [turbofan] Handle Type::None() in SameValue
Bug: chromium:351865302
(cherry picked from commit f784b0b05be96cda5b769a3df92fc3c012657166)
Change-Id: Idf6bf9276d5510376594f31ee563e4a0bee617f5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5806588
Reviewed-by: Matthias Liedtke <mliedtke@chromium.org>
Commit-Queue: Nico Hartmann <nicohartmann@chromium.org>
Auto-Submit: Nico Hartmann <nicohartmann@chromium.org>
Commit-Queue: Matthias Liedtke <mliedtke@chromium.org>
Cr-Commit-Position: refs/branch-heads/12.8@{#42}
Cr-Branched-From: 70cbb397b153166027e34c75adf8e7993858222e-refs/heads/12.8.374@{#1}
Cr-Branched-From: 451b63ed4251c2b21c56144d8428f8be3331539b-refs/heads/main@{#95151}
diff --git a/src/compiler/operation-typer.cc b/src/compiler/operation-typer.cc
index 11d83ba7a052f8fb7342783eb4a0e207a9842ddd..832e26ce25522af10afd85bc779f08fa75432c75 100644
--- a/src/compiler/operation-typer.cc
+++ b/src/compiler/operation-typer.cc
@@ -1259,6 +1259,7 @@ Type JSType(Type type) {
} // namespace
Type OperationTyper::SameValue(Type lhs, Type rhs) {
+ if (lhs.IsNone() || rhs.IsNone()) return Type::None();
if (!JSType(lhs).Maybe(JSType(rhs))) return singleton_false();
if (lhs.Is(Type::NaN())) {
if (rhs.Is(Type::NaN())) return singleton_true();