chore: cherry-pick 11 changes from 3-M126 (#43141)
* chore: [30-x-y] cherry-pick 11 changes from 3-M126 * d54105311590 from chromium * 43b8b682d05c from chromium * c5dd8839bfaf from chromium * cdbc1d9684a3 from v8 * 38e4483e47f9 from chromium * 70d2fe6b7c47 from v8 * 901377bb2f3b from v8 * 1b9040817119 from chromium * bb28367eed73 from v8 * 99cafbf4b4b9 from chromium * bc545b15a0ee from v8 * chore: update patches * 5639725: [wasm] Fix scanning of wasm-to-js params |5639725
* 5672472: [M120-LTS] Prevent script injection on reload when racing with a navigation |5672472
This commit is contained in:
parent
ec272f1e53
commit
1ec867c8a1
13 changed files with 1150 additions and 0 deletions
|
@ -3,3 +3,8 @@ deps_add_v8_object_setinternalfieldfornodecore.patch
|
|||
cherry-pick-8b400f9b7d66.patch
|
||||
cherry-pick-ba6cab40612d.patch
|
||||
merged_wasm_add_missing_type_canonicalization_for_exceptions_js.patch
|
||||
cherry-pick-cdbc1d9684a3.patch
|
||||
cherry-pick-70d2fe6b7c47.patch
|
||||
cherry-pick-901377bb2f3b.patch
|
||||
cherry-pick-bb28367eed73.patch
|
||||
cherry-pick-bc545b15a0ee.patch
|
||||
|
|
32
patches/v8/cherry-pick-70d2fe6b7c47.patch
Normal file
32
patches/v8/cherry-pick-70d2fe6b7c47.patch
Normal file
|
@ -0,0 +1,32 @@
|
|||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Leszek Swirski <leszeks@chromium.org>
|
||||
Date: Fri, 21 Jun 2024 15:11:40 +0200
|
||||
Subject: Allow reduced hasInstance to abort
|
||||
|
||||
Fixed: 343507800
|
||||
Change-Id: I579041fe82e975d83a72e4744013cb04c4d3dc70
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5644891
|
||||
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
|
||||
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
|
||||
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
|
||||
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
|
||||
Cr-Commit-Position: refs/heads/main@{#94585}
|
||||
|
||||
diff --git a/src/maglev/maglev-graph-builder.cc b/src/maglev/maglev-graph-builder.cc
|
||||
index efcdd6d2028a5e0f0ec1149925ab2e1fe5f90412..78f4dd57d339eaad8b265721ef37137291da7940 100644
|
||||
--- a/src/maglev/maglev-graph-builder.cc
|
||||
+++ b/src/maglev/maglev-graph-builder.cc
|
||||
@@ -8639,10 +8639,9 @@ ReduceResult MaglevGraphBuilder::TryBuildFastInstanceOf(
|
||||
|
||||
if (has_instance_field->IsJSFunction()) {
|
||||
SaveCallSpeculationScope saved(this);
|
||||
- ReduceResult result =
|
||||
- ReduceCallForConstant(has_instance_field->AsJSFunction(), args);
|
||||
- DCHECK(!result.IsDoneWithAbort());
|
||||
- call_result = result.value();
|
||||
+ GET_VALUE_OR_ABORT(
|
||||
+ call_result,
|
||||
+ ReduceCallForConstant(has_instance_field->AsJSFunction(), args));
|
||||
} else {
|
||||
call_result = BuildGenericCall(GetConstant(*has_instance_field),
|
||||
Call::TargetType::kAny, args);
|
221
patches/v8/cherry-pick-901377bb2f3b.patch
Normal file
221
patches/v8/cherry-pick-901377bb2f3b.patch
Normal file
|
@ -0,0 +1,221 @@
|
|||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Thibaud Michaud <thibaudm@chromium.org>
|
||||
Date: Fri, 21 Jun 2024 16:31:15 +0200
|
||||
Subject: Fix scanning of wasm-to-js params
|
||||
|
||||
Wasm-to-js wrappers are sometimes compiled as on-heap Code objects, for
|
||||
example when tiering-up from a WasmFuncRef call origin. The frames of
|
||||
these functions are mapped to a subclass of TypedFrame, however
|
||||
TypedFrame::Iterate() only supports iterating the generic wasm-to-js
|
||||
wrapper.
|
||||
|
||||
Add support for iterating the tagged parameters of optimized wasm-to-js
|
||||
wrappers in TypedFrame::Iterate. For this we also add two 16-bit fields
|
||||
in the Code object to encode the incoming tagged parameter region, which
|
||||
we would normally find in the WasmCode data.
|
||||
|
||||
R=jkummerow@chromium.org
|
||||
|
||||
Fixed: 346597059
|
||||
Change-Id: I425619fca86c38f91f1ca9cbeb70e7b5a7b2d6c1
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5639725
|
||||
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
|
||||
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
|
||||
Cr-Commit-Position: refs/heads/main@{#94589}
|
||||
|
||||
diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc
|
||||
index 0181588337df73bfa97220d895733c40b92bd40b..033469e626ffb35846ae5114632f3dc000400935 100644
|
||||
--- a/src/compiler/pipeline.cc
|
||||
+++ b/src/compiler/pipeline.cc
|
||||
@@ -2295,6 +2295,14 @@ CompilationJob::Status FinalizeWrapperCompilation(
|
||||
Handle<AbstractCode>::cast(code),
|
||||
info->GetDebugName().get()));
|
||||
}
|
||||
+ // Set the wasm-to-js specific code fields needed to scan the incoming stack
|
||||
+ // parameters.
|
||||
+ if (code->kind() == CodeKind::WASM_TO_JS_FUNCTION) {
|
||||
+ code->set_wasm_js_tagged_parameter_count(
|
||||
+ call_descriptor->GetTaggedParameterSlots() & 0xffff);
|
||||
+ code->set_wasm_js_first_tagged_parameter(
|
||||
+ call_descriptor->GetTaggedParameterSlots() >> 16);
|
||||
+ }
|
||||
return CompilationJob::SUCCEEDED;
|
||||
}
|
||||
return CompilationJob::FAILED;
|
||||
diff --git a/src/diagnostics/objects-printer.cc b/src/diagnostics/objects-printer.cc
|
||||
index 7677022ce31c1b8ead0cc2eb37fb505b750639be..12bd5b8fd27f67c73938550acc4af1857eace59a 100644
|
||||
--- a/src/diagnostics/objects-printer.cc
|
||||
+++ b/src/diagnostics/objects-printer.cc
|
||||
@@ -2102,7 +2102,14 @@ void Code::CodePrint(std::ostream& os, const char* name, Address current_pc) {
|
||||
os << "\n - instruction_size: " << instruction_size();
|
||||
os << "\n - metadata_size: " << metadata_size();
|
||||
|
||||
- os << "\n - inlined_bytecode_size: " << inlined_bytecode_size();
|
||||
+ if (kind() != CodeKind::WASM_TO_JS_FUNCTION) {
|
||||
+ os << "\n - inlined_bytecode_size: " << inlined_bytecode_size();
|
||||
+ } else {
|
||||
+ os << "\n - wasm_js_tagged_parameter_count: "
|
||||
+ << wasm_js_tagged_parameter_count();
|
||||
+ os << "\n - wasm_js_first_tagged_parameter: "
|
||||
+ << wasm_js_first_tagged_parameter();
|
||||
+ }
|
||||
os << "\n - osr_offset: " << osr_offset();
|
||||
os << "\n - handler_table_offset: " << handler_table_offset();
|
||||
os << "\n - unwinding_info_offset: " << unwinding_info_offset();
|
||||
diff --git a/src/execution/frames.cc b/src/execution/frames.cc
|
||||
index 92dff2f7e8c8f72e38eef4feb5b10ace9fe2535c..2693fb2a859dc7489ef802c3064beace88608415 100644
|
||||
--- a/src/execution/frames.cc
|
||||
+++ b/src/execution/frames.cc
|
||||
@@ -1562,7 +1562,7 @@ void WasmFrame::Iterate(RootVisitor* v) const {
|
||||
frame_header_limit);
|
||||
}
|
||||
|
||||
-void TypedFrame::IterateParamsOfWasmToJSWrapper(RootVisitor* v) const {
|
||||
+void TypedFrame::IterateParamsOfGenericWasmToJSWrapper(RootVisitor* v) const {
|
||||
Tagged<Object> maybe_signature = Tagged<Object>(
|
||||
Memory<Address>(fp() + WasmToJSWrapperConstants::kSignatureOffset));
|
||||
if (IsSmi(maybe_signature)) {
|
||||
@@ -1678,6 +1678,18 @@ void TypedFrame::IterateParamsOfWasmToJSWrapper(RootVisitor* v) const {
|
||||
}
|
||||
}
|
||||
}
|
||||
+
|
||||
+void TypedFrame::IterateParamsOfOptimizedWasmToJSWrapper(RootVisitor* v) const {
|
||||
+ Tagged<GcSafeCode> code = GcSafeLookupCode();
|
||||
+ if (code->wasm_js_tagged_parameter_count() > 0) {
|
||||
+ FullObjectSlot tagged_parameter_base(&Memory<Address>(caller_sp()));
|
||||
+ tagged_parameter_base += code->wasm_js_first_tagged_parameter();
|
||||
+ FullObjectSlot tagged_parameter_limit =
|
||||
+ tagged_parameter_base + code->wasm_js_tagged_parameter_count();
|
||||
+ v->VisitRootPointers(Root::kStackRoots, nullptr, tagged_parameter_base,
|
||||
+ tagged_parameter_limit);
|
||||
+ }
|
||||
+}
|
||||
#endif // V8_ENABLE_WEBASSEMBLY
|
||||
|
||||
void TypedFrame::Iterate(RootVisitor* v) const {
|
||||
@@ -1709,10 +1721,13 @@ void TypedFrame::Iterate(RootVisitor* v) const {
|
||||
CHECK(entry->code.has_value());
|
||||
Tagged<GcSafeCode> code = entry->code.value();
|
||||
#if V8_ENABLE_WEBASSEMBLY
|
||||
- bool is_wasm_to_js =
|
||||
+ bool is_generic_wasm_to_js =
|
||||
code->is_builtin() && code->builtin_id() == Builtin::kWasmToJsWrapperCSA;
|
||||
- if (is_wasm_to_js) {
|
||||
- IterateParamsOfWasmToJSWrapper(v);
|
||||
+ bool is_optimized_wasm_to_js = this->type() == WASM_TO_JS_FUNCTION;
|
||||
+ if (is_generic_wasm_to_js) {
|
||||
+ IterateParamsOfGenericWasmToJSWrapper(v);
|
||||
+ } else if (is_optimized_wasm_to_js) {
|
||||
+ IterateParamsOfOptimizedWasmToJSWrapper(v);
|
||||
}
|
||||
#endif // V8_ENABLE_WEBASSEMBLY
|
||||
DCHECK(code->is_turbofanned());
|
||||
@@ -1745,10 +1760,14 @@ void TypedFrame::Iterate(RootVisitor* v) const {
|
||||
// wrapper switched to before pushing the outgoing stack parameters and
|
||||
// calling the target. It marks the limit of the stack param area, and is
|
||||
// distinct from the beginning of the spill area.
|
||||
- Address central_stack_sp =
|
||||
- Memory<Address>(fp() + WasmToJSWrapperConstants::kCentralStackSPOffset);
|
||||
+ int central_stack_sp_offset =
|
||||
+ is_generic_wasm_to_js
|
||||
+ ? WasmToJSWrapperConstants::kCentralStackSPOffset
|
||||
+ : WasmImportWrapperFrameConstants::kCentralStackSPOffset;
|
||||
+ Address central_stack_sp = Memory<Address>(fp() + central_stack_sp_offset);
|
||||
FullObjectSlot parameters_limit(
|
||||
- is_wasm_to_js && central_stack_sp != kNullAddress
|
||||
+ (is_generic_wasm_to_js || is_optimized_wasm_to_js) &&
|
||||
+ central_stack_sp != kNullAddress
|
||||
? central_stack_sp
|
||||
: frame_header_base.address() - spill_slots_size);
|
||||
#else
|
||||
diff --git a/src/execution/frames.h b/src/execution/frames.h
|
||||
index 081c74bf124ccfa57e4b40f75cbe42b00c771b2e..908239b4161235aeda04fe5526ddea8d56b09425 100644
|
||||
--- a/src/execution/frames.h
|
||||
+++ b/src/execution/frames.h
|
||||
@@ -634,7 +634,8 @@ class TypedFrame : public CommonFrame {
|
||||
Tagged<HeapObject> unchecked_code() const override { return {}; }
|
||||
void Iterate(RootVisitor* v) const override;
|
||||
|
||||
- void IterateParamsOfWasmToJSWrapper(RootVisitor* v) const;
|
||||
+ void IterateParamsOfGenericWasmToJSWrapper(RootVisitor* v) const;
|
||||
+ void IterateParamsOfOptimizedWasmToJSWrapper(RootVisitor* v) const;
|
||||
|
||||
protected:
|
||||
inline explicit TypedFrame(StackFrameIteratorBase* iterator);
|
||||
diff --git a/src/objects/code-inl.h b/src/objects/code-inl.h
|
||||
index baaced29afdbc87eae1c34b8df779e17e41410c4..1e1d66a87ee633cbdb49265444f53b5db790d9dd 100644
|
||||
--- a/src/objects/code-inl.h
|
||||
+++ b/src/objects/code-inl.h
|
||||
@@ -48,6 +48,8 @@ GCSAFE_CODE_FWD_ACCESSOR(bool, has_tagged_outgoing_params)
|
||||
GCSAFE_CODE_FWD_ACCESSOR(bool, marked_for_deoptimization)
|
||||
GCSAFE_CODE_FWD_ACCESSOR(Tagged<Object>, raw_instruction_stream)
|
||||
GCSAFE_CODE_FWD_ACCESSOR(int, stack_slots)
|
||||
+GCSAFE_CODE_FWD_ACCESSOR(uint16_t, wasm_js_tagged_parameter_count)
|
||||
+GCSAFE_CODE_FWD_ACCESSOR(uint16_t, wasm_js_first_tagged_parameter)
|
||||
GCSAFE_CODE_FWD_ACCESSOR(Address, constant_pool)
|
||||
GCSAFE_CODE_FWD_ACCESSOR(Address, safepoint_table_address)
|
||||
#undef GCSAFE_CODE_FWD_ACCESSOR
|
||||
@@ -428,6 +430,31 @@ void Code::set_inlined_bytecode_size(unsigned size) {
|
||||
RELAXED_WRITE_UINT_FIELD(*this, kInlinedBytecodeSizeOffset, size);
|
||||
}
|
||||
|
||||
+// For optimized on-heap wasm-js wrappers, we repurpose the (otherwise unused)
|
||||
+// 32-bit InlinedBytecodeSize field to encode two 16 values needed for scanning
|
||||
+// the frame: the count and starting offset of incoming tagged parameters.
|
||||
+// TODO(wasm): Eventually the wrappers should be managed off-heap by the wasm
|
||||
+// engine. Remove these accessors when that is the case.
|
||||
+void Code::set_wasm_js_tagged_parameter_count(uint16_t count) {
|
||||
+ DCHECK_EQ(kind(), CodeKind::WASM_TO_JS_FUNCTION);
|
||||
+ RELAXED_WRITE_UINT16_FIELD(*this, kInlinedBytecodeSizeOffset, count);
|
||||
+}
|
||||
+
|
||||
+uint16_t Code::wasm_js_tagged_parameter_count() const {
|
||||
+ DCHECK_EQ(kind(), CodeKind::WASM_TO_JS_FUNCTION);
|
||||
+ return RELAXED_READ_UINT16_FIELD(*this, kInlinedBytecodeSizeOffset);
|
||||
+}
|
||||
+
|
||||
+void Code::set_wasm_js_first_tagged_parameter(uint16_t count) {
|
||||
+ DCHECK_EQ(kind(), CodeKind::WASM_TO_JS_FUNCTION);
|
||||
+ RELAXED_WRITE_UINT16_FIELD(*this, kInlinedBytecodeSizeOffset + 2, count);
|
||||
+}
|
||||
+
|
||||
+uint16_t Code::wasm_js_first_tagged_parameter() const {
|
||||
+ DCHECK_EQ(kind(), CodeKind::WASM_TO_JS_FUNCTION);
|
||||
+ return RELAXED_READ_UINT16_FIELD(*this, kInlinedBytecodeSizeOffset + 2);
|
||||
+}
|
||||
+
|
||||
BytecodeOffset Code::osr_offset() const {
|
||||
return BytecodeOffset(RELAXED_READ_INT32_FIELD(*this, kOsrOffsetOffset));
|
||||
}
|
||||
diff --git a/src/objects/code.h b/src/objects/code.h
|
||||
index 9a079a94ba0126b24532362a0ce233477f42c221..1da011899807125d6dc9ffb6d56622f5f15ad465 100644
|
||||
--- a/src/objects/code.h
|
||||
+++ b/src/objects/code.h
|
||||
@@ -124,6 +124,15 @@ class Code : public ExposedTrustedObject {
|
||||
// [deoptimization_data]: Array containing data for deopt for non-baseline
|
||||
// code.
|
||||
DECL_ACCESSORS(deoptimization_data, Tagged<TrustedFixedArray>)
|
||||
+ // [parameter_count]: The number of formal parameters, including the
|
||||
+ // receiver. Currently only available for optimized functions.
|
||||
+ // TODO(saelo): make this always available. This is just a matter of figuring
|
||||
+ // out how to obtain the parameter count during code generation when no
|
||||
+ // BytecodeArray is available from which it can be copied.
|
||||
+ DECL_PRIMITIVE_ACCESSORS(parameter_count, uint16_t)
|
||||
+ inline uint16_t parameter_count_without_receiver() const;
|
||||
+ DECL_PRIMITIVE_ACCESSORS(wasm_js_tagged_parameter_count, uint16_t)
|
||||
+ DECL_PRIMITIVE_ACCESSORS(wasm_js_first_tagged_parameter, uint16_t)
|
||||
|
||||
// Whether this type of Code uses deoptimization data, in which case the
|
||||
// deoptimization_data field will be populated.
|
||||
@@ -503,6 +512,10 @@ class GcSafeCode : public HeapObject {
|
||||
inline bool CanDeoptAt(Isolate* isolate, Address pc) const;
|
||||
inline Tagged<Object> raw_instruction_stream(
|
||||
PtrComprCageBase code_cage_base) const;
|
||||
+ // The two following accessors repurpose the InlinedBytecodeSize field, see
|
||||
+ // comment in code-inl.h.
|
||||
+ inline uint16_t wasm_js_tagged_parameter_count() const;
|
||||
+ inline uint16_t wasm_js_first_tagged_parameter() const;
|
||||
|
||||
private:
|
||||
OBJECT_CONSTRUCTORS(GcSafeCode, HeapObject);
|
64
patches/v8/cherry-pick-bb28367eed73.patch
Normal file
64
patches/v8/cherry-pick-bb28367eed73.patch
Normal file
|
@ -0,0 +1,64 @@
|
|||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: =?UTF-8?q?Olivier=20Fl=C3=BCckiger?= <olivf@chromium.org>
|
||||
Date: Mon, 24 Jun 2024 16:22:09 +0200
|
||||
Subject: Fix skipped smi check due to phi hoisting
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
Given we have a phi (29) assumed to be smi at graph building time, we
|
||||
must not untag it's input phis (10,12) to float64.
|
||||
|
||||
╭─────►Block b2
|
||||
│ 10: φᵀ r0 (n4, n29) (compressed) → (x), 3 uses
|
||||
│ 12: φᵀ r2 (n6, n39) (compressed) → (x), 6 uses
|
||||
...
|
||||
│ 13: CheckedSmiUntag [n10:(x)] → (x), 2 uses
|
||||
│ 14: CheckedSmiUntag [n12:(x)] → (x), 1 uses
|
||||
...
|
||||
│╭──────17: BranchIfToBooleanTrue [n16:(x)] b3 b9
|
||||
...
|
||||
││ │ 29: φᵀ <accumulator> (n10, n12) (compressed) → (x), 4 uses
|
||||
...
|
||||
││ │ 33: UnsafeSmiUntag [n29:(x)] → (x), 1 uses
|
||||
|
||||
Doing so could invalidate the `UnsafeSmiUntag` instruction.
|
||||
|
||||
This can only happen when hoisting the untagging out of the loop, as
|
||||
this will remove the original `CheckedSmiUntag` instruction.
|
||||
|
||||
Fixed: 348567825
|
||||
Change-Id: I2d7f2c3b544f991be9850d44bf11c3f632a0bb46
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5645901
|
||||
Reviewed-by: Darius Mercadier <dmercadier@chromium.org>
|
||||
Commit-Queue: Darius Mercadier <dmercadier@chromium.org>
|
||||
Auto-Submit: Olivier Flückiger <olivf@chromium.org>
|
||||
Cr-Commit-Position: refs/heads/main@{#94615}
|
||||
|
||||
diff --git a/src/maglev/maglev-phi-representation-selector.cc b/src/maglev/maglev-phi-representation-selector.cc
|
||||
index 02e030bcb543287433c944e5f298998cdc5b7ce9..cabe3a29383b111fc0f1568a40d439e1513bc154 100644
|
||||
--- a/src/maglev/maglev-phi-representation-selector.cc
|
||||
+++ b/src/maglev/maglev-phi-representation-selector.cc
|
||||
@@ -285,6 +285,14 @@ MaglevPhiRepresentationSelector::ProcessPhi(Phi* node) {
|
||||
ValueRepresentation::kHoleyFloat64};
|
||||
}
|
||||
|
||||
+ // When hoisting we must ensure that we don't turn a tagged flowing into
|
||||
+ // CheckedSmiUntag into a float64. This would cause us to loose the smi check
|
||||
+ // which in turn can invalidate assumptions on aliasing values.
|
||||
+ if (hoist_untagging.size() && node->uses_require_31_bit_value()) {
|
||||
+ allowed_inputs_for_uses.Remove(
|
||||
+ {ValueRepresentation::kFloat64, ValueRepresentation::kHoleyFloat64});
|
||||
+ }
|
||||
+
|
||||
auto intersection = possible_inputs & allowed_inputs_for_uses;
|
||||
|
||||
TRACE_UNTAGGING(" + intersection reprs: " << intersection);
|
||||
@@ -615,6 +623,7 @@ void MaglevPhiRepresentationSelector::ConvertTaggedPhiTo(
|
||||
TaggedToFloat64ConversionType::kOnlyNumber),
|
||||
block, NewNodePosition::kEnd);
|
||||
} else {
|
||||
+ DCHECK(!phi->uses_require_31_bit_value());
|
||||
untagged = AddNode(NodeBase::New<CheckedNumberOrOddballToFloat64>(
|
||||
builder_->zone(), {input},
|
||||
TaggedToFloat64ConversionType::kOnlyNumber),
|
39
patches/v8/cherry-pick-bc545b15a0ee.patch
Normal file
39
patches/v8/cherry-pick-bc545b15a0ee.patch
Normal file
|
@ -0,0 +1,39 @@
|
|||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Jakob Kummerow <jkummerow@chromium.org>
|
||||
Date: Thu, 11 Jul 2024 16:34:00 +0200
|
||||
Subject: Fix cast of memory index
|
||||
|
||||
"uint8_t" must have been a typo.
|
||||
|
||||
Fixed: 351327767
|
||||
Bug: 42203854
|
||||
Change-Id: I196c961ec2f2ed16acfe16bf304d7eae6551aacc
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5695665
|
||||
Reviewed-by: Matthias Liedtke <mliedtke@chromium.org>
|
||||
Commit-Queue: Matthias Liedtke <mliedtke@chromium.org>
|
||||
Auto-Submit: Jakob Kummerow <jkummerow@chromium.org>
|
||||
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
|
||||
Cr-Commit-Position: refs/heads/main@{#94981}
|
||||
|
||||
diff --git a/src/compiler/wasm-compiler.cc b/src/compiler/wasm-compiler.cc
|
||||
index 16f1f1470b782ce3584ba4bb6626501c9f9551b7..02364fdf5d3d17fa0f03fd48d21523a4bae3f21e 100644
|
||||
--- a/src/compiler/wasm-compiler.cc
|
||||
+++ b/src/compiler/wasm-compiler.cc
|
||||
@@ -3395,7 +3395,7 @@ Node* WasmGraphBuilder::MemStart(uint32_t mem_index) {
|
||||
DCHECK_NOT_NULL(instance_cache_);
|
||||
V8_ASSUME(cached_memory_index_ == kNoCachedMemoryIndex ||
|
||||
cached_memory_index_ >= 0);
|
||||
- if (mem_index == static_cast<uint8_t>(cached_memory_index_)) {
|
||||
+ if (mem_index == static_cast<uint32_t>(cached_memory_index_)) {
|
||||
return instance_cache_->mem_start;
|
||||
}
|
||||
return LoadMemStart(mem_index);
|
||||
@@ -3405,7 +3405,7 @@ Node* WasmGraphBuilder::MemSize(uint32_t mem_index) {
|
||||
DCHECK_NOT_NULL(instance_cache_);
|
||||
V8_ASSUME(cached_memory_index_ == kNoCachedMemoryIndex ||
|
||||
cached_memory_index_ >= 0);
|
||||
- if (mem_index == static_cast<uint8_t>(cached_memory_index_)) {
|
||||
+ if (mem_index == static_cast<uint32_t>(cached_memory_index_)) {
|
||||
return instance_cache_->mem_size;
|
||||
}
|
||||
|
112
patches/v8/cherry-pick-cdbc1d9684a3.patch
Normal file
112
patches/v8/cherry-pick-cdbc1d9684a3.patch
Normal file
|
@ -0,0 +1,112 @@
|
|||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Darius Mercadier <dmercadier@chromium.org>
|
||||
Date: Tue, 18 Jun 2024 16:10:26 +0200
|
||||
Subject: Lower LoadStackArgument to a Tagged load
|
||||
|
||||
If we start with a graph that looks like
|
||||
|
||||
```
|
||||
x = LoadStackArgument(a, 40)
|
||||
...
|
||||
Allocate()
|
||||
...
|
||||
y = LoadStackArgument(a, 40)
|
||||
```
|
||||
|
||||
This used to be lowered to
|
||||
|
||||
```
|
||||
x1 = Load<WordPtr>(a, 40)
|
||||
x2 = TaggedBitcast(x1, WordPtr->Tagged)
|
||||
...
|
||||
Allocate()
|
||||
...
|
||||
y1 = Load<WordPtr>(a, 40)
|
||||
y2 = TaggedBitcast(y1, WordPtr->Tagged)
|
||||
```
|
||||
|
||||
And then, Load Elimination would remove the second Load, and we'd get:
|
||||
|
||||
```
|
||||
x1 = Load<WordPtr>(a, 40)
|
||||
x2 = TaggedBitcast(x1, WordPtr->Tagged)
|
||||
...
|
||||
Allocate()
|
||||
...
|
||||
y2 = TaggedBitcast(x1, WordPtr->Tagged)
|
||||
```
|
||||
|
||||
And now we would be in trouble: if the allocation in the middle
|
||||
triggers a GC, then `x1` could move, and thus `y2` could refer to a
|
||||
stale pointer. In theory, Turbofan knows where tagged values are, and
|
||||
can thus update them when the GC moves things, but here, `x1` is not
|
||||
marked as Tagged (but rather as a raw WordPtr).
|
||||
|
||||
This CL fixes this issue by doing a Tagged load from the start, since
|
||||
the value we're loading is clearly tagged.
|
||||
|
||||
Fixed: chromium:347724915
|
||||
Change-Id: Ia659155fbc602907ab9a50fb992c79df6ccdaa44
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5630530
|
||||
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
|
||||
Auto-Submit: Darius Mercadier <dmercadier@chromium.org>
|
||||
Commit-Queue: Nico Hartmann <nicohartmann@chromium.org>
|
||||
Cr-Commit-Position: refs/heads/main@{#94527}
|
||||
|
||||
diff --git a/src/compiler/turboshaft/machine-lowering-reducer-inl.h b/src/compiler/turboshaft/machine-lowering-reducer-inl.h
|
||||
index 8f37ef00f7edc1395586439fad4c39f426520d87..1f8ecb428f7d53d20369b82d958b2309ab09d2eb 100644
|
||||
--- a/src/compiler/turboshaft/machine-lowering-reducer-inl.h
|
||||
+++ b/src/compiler/turboshaft/machine-lowering-reducer-inl.h
|
||||
@@ -2372,9 +2372,17 @@ class MachineLoweringReducer : public Next {
|
||||
}
|
||||
|
||||
V<Object> REDUCE(LoadStackArgument)(V<WordPtr> base, V<WordPtr> index) {
|
||||
- V<WordPtr> argument = __ template LoadNonArrayBufferElement<WordPtr>(
|
||||
- base, AccessBuilder::ForStackArgument(), index);
|
||||
- return __ BitcastWordPtrToTagged(argument);
|
||||
+ // Note that this is a load of a Tagged value
|
||||
+ // (MemoryRepresentation::TaggedPointer()), but since it's on the stack
|
||||
+ // where stack slots are all kSystemPointerSize, we use kSystemPointerSize
|
||||
+ // for element_size_log2. On 64-bit plateforms with pointer compression,
|
||||
+ // this means that we're kinda loading a 32-bit value from an array of
|
||||
+ // 64-bit values.
|
||||
+ return __ Load(
|
||||
+ base, index, LoadOp::Kind::RawAligned(),
|
||||
+ MemoryRepresentation::TaggedPointer(),
|
||||
+ CommonFrameConstants::kFixedFrameSizeAboveFp - kSystemPointerSize,
|
||||
+ kSystemPointerSizeLog2);
|
||||
}
|
||||
|
||||
OpIndex REDUCE(StoreTypedElement)(OpIndex buffer, V<Object> base,
|
||||
diff --git a/test/mjsunit/compiler/regress-347724915.js b/test/mjsunit/compiler/regress-347724915.js
|
||||
new file mode 100644
|
||||
index 0000000000000000000000000000000000000000..4a5d1a9a2e3dd7674bf0872c94a971b5f28ddf72
|
||||
--- /dev/null
|
||||
+++ b/test/mjsunit/compiler/regress-347724915.js
|
||||
@@ -0,0 +1,26 @@
|
||||
+// Copyright 2024 the V8 project authors. All rights reserved.
|
||||
+// Use of this source code is governed by a BSD-style license that can be
|
||||
+// found in the LICENSE file.
|
||||
+
|
||||
+// Flags: --allow-natives-syntax
|
||||
+
|
||||
+function f(...args) {
|
||||
+ let arr1 = [ undefined, undefined, undefined ];
|
||||
+ %SimulateNewspaceFull();
|
||||
+ arr1[0] = args[0];
|
||||
+ // The following allocation will trigger a full GC, which will move the
|
||||
+ // argument passed to the function (because it was a young object).
|
||||
+ let arr2 = [ arr1 ];
|
||||
+ // Here we're accessing `args[0]` again. This might be load-eliminated with
|
||||
+ // the `args[0]` load from a few lines above, which has been moved by the GC
|
||||
+ // since then. This should be fine though, as the loaded value should be
|
||||
+ // marked as Tagged, which means that it shouldn't point to the stale value
|
||||
+ // but instead have been updated during GC.
|
||||
+ arr1[1] = args[0]
|
||||
+ return arr2;
|
||||
+}
|
||||
+
|
||||
+%PrepareFunctionForOptimization(f);
|
||||
+let expected = f({ x : 42 });
|
||||
+%OptimizeFunctionOnNextCall(f);
|
||||
+assertEquals(expected, f({x : 42}));
|
Loading…
Add table
Add a link
Reference in a new issue