github/electron
1
1
Fork 0
mirror of https://github.com/electron/electron.git synced 2026-04-10 03:01:51 -04:00
Code Issues Packages Projects Releases Wiki Activity

chore: cherry-pick 3 changes from 1-M133 (#45716)

Browse Source 
chore: [33-x-y] cherry-pick 3 changes from 1-M133

* 924c1a79f139 from v8
This commit is contained in:
Pedro Pontes
2025-03-05 07:25:29 +00:00
committed by GitHub
parent f49816e855
commit 1686a38c2a
7 changed files with 675 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
patches/v8/.patches
View File

@@ -10,3 +10,6 @@ cherry-pick-9209292e7898.patch
cherry-pick-97e828af5cbc.patch
cherry-pick-ca504d096c39.patch
cherry-pick-8131c09bc129.patch
wasm_fix_freeing_of_identical_shared_wrappers.patch
merged_wasm_replace_dead_code_set_with_is_dying_bit.patch
merged_fix_out_of_bound_string_access.patch

21
patches/v8/cherry-pick-1c7ff4d5477f.patch
View File

@@ -1,7 +1,11 @@
From 1c7ff4d5477f0e2bc7e20ce3b0f4f8eef71e6d13 Mon Sep 17 00:00:00 2001
From: Olivier Flückiger <olivf@chromium.org>
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Olivier=20Fl=C3=BCckiger?= <olivf@chromium.org>
Date: Mon, 27 Jan 2025 14:50:34 +0100
Subject: [PATCH] Merged: [turbofan] LoadField's type with recorded FieldType depends on stability
Subject: Merged: [turbofan] LoadField's type with recorded FieldType depends
on stability
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Bug: 390465670
(cherry picked from commit f920b6b2c2b1132cd1fbe1198500ceeaedcaa146)
@@ -15,13 +19,12 @@ Commit-Queue: Olivier Flückiger <olivf@chromium.org>
Cr-Commit-Position: refs/branch-heads/13.2@{#72}
Cr-Branched-From: 24068c59cedad9ee976ddc05431f5f497b1ebd71-refs/heads/13.2.152@{#1}
Cr-Branched-From: 6054ba94db0969220be4f94dc1677fc4696bdc4f-refs/heads/main@{#97085}
---
diff --git a/src/compiler/access-info.cc b/src/compiler/access-info.cc
index 7247c49..53e55a6 100644
index 595f4ad9fb5a0c744aed9226eb9c536ef156b5cf..8338b2d13daf37ef238fe5cdf22a4ff9b2a0362a 100644
--- a/src/compiler/access-info.cc
+++ b/src/compiler/access-info.cc
@@ -484,8 +484,9 @@
@@ -474,8 +474,9 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo(
OptionalMapRef maybe_field_map =
TryMakeRef(broker(), FieldType::AsClass(*descriptors_field_type));
if (!maybe_field_map.has_value()) return Invalid();
@@ -32,7 +35,7 @@ index 7247c49..53e55a6 100644
}
} else {
CHECK(details_representation.IsTagged());
@@ -1186,8 +1187,9 @@
@@ -1168,8 +1169,9 @@ PropertyAccessInfo AccessInfoFactory::LookupTransition(
OptionalMapRef maybe_field_map =
TryMakeRef(broker(), FieldType::AsClass(*descriptors_field_type));
if (!maybe_field_map.has_value()) return Invalid();
@@ -44,10 +47,10 @@ index 7247c49..53e55a6 100644
}
diff --git a/src/compiler/property-access-builder.cc b/src/compiler/property-access-builder.cc
index 32c0ccf..bcaf221 100644
index 43203d2390ff3ed06a6de9fe54409de86820e729..8714c91d7572329fead93d65fdd439d81b3ea36f 100644
--- a/src/compiler/property-access-builder.cc
+++ b/src/compiler/property-access-builder.cc
@@ -337,6 +337,7 @@
@@ -318,6 +318,7 @@ Node* PropertyAccessBuilder::BuildLoadDataField(
if (field_map->is_stable()) {
dependencies()->DependOnStableMap(field_map.value());
field_access.map = field_map;

14
patches/v8/cherry-pick-9209292e7898.patch
View File

@@ -1,7 +1,8 @@
From 9209292e7898a05a98d271c3c597a09d66819292 Mon Sep 17 00:00:00 2001
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Darius Mercadier <dmercadier@chromium.org>
Date: Wed, 08 Jan 2025 14:37:24 +0100
Subject: [PATCH] Merged: [turbofan] Fix CallWithSpread bug when array prototype has changed
Date: Wed, 8 Jan 2025 14:37:24 +0100
Subject: Merged: [turbofan] Fix CallWithSpread bug when array prototype has
changed
Bug: chromium:385386138
(cherry picked from commit 1be46f0e2e71159a74d8614f9ac108d334797729)
@@ -14,13 +15,12 @@ Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Cr-Commit-Position: refs/branch-heads/13.2@{#58}
Cr-Branched-From: 24068c59cedad9ee976ddc05431f5f497b1ebd71-refs/heads/13.2.152@{#1}
Cr-Branched-From: 6054ba94db0969220be4f94dc1677fc4696bdc4f-refs/heads/main@{#97085}
---
diff --git a/src/compiler/js-call-reducer.cc b/src/compiler/js-call-reducer.cc
index a29bd4a..08fd278 100644
index cc9ec63ce09cbd09e8484b9ab075545e351de7eb..7c248c80a519708eaa2a25e9289fd6094ed52424 100644
--- a/src/compiler/js-call-reducer.cc
+++ b/src/compiler/js-call-reducer.cc
@@ -5226,6 +5226,18 @@
@@ -5223,6 +5223,18 @@ TNode<Object> JSCallReducerAssembler::ReduceJSCallWithArrayLikeOrSpreadOfEmpty(
DCHECK_EQ(static_cast<Node*>(arguments_list)->opcode(),
IrOpcode::kJSCreateEmptyLiteralArray);
@@ -41,7 +41,7 @@ index a29bd4a..08fd278 100644
// "arguments_list array is still empty?"
diff --git a/test/mjsunit/compiler/regress-385386138.js b/test/mjsunit/compiler/regress-385386138.js
new file mode 100644
index 0000000..72dadea
index 0000000000000000000000000000000000000000..72dadea5ff5b0732822e6a15a34bd3357d5ff7c2
--- /dev/null
+++ b/test/mjsunit/compiler/regress-385386138.js
@@ -0,0 +1,27 @@

26
patches/v8/cherry-pick-97e828af5cbc.patch
View File

@@ -1,7 +1,10 @@
From 97e828af5cbcf50c3ff0064a4a5c22e18c18b4b5 Mon Sep 17 00:00:00 2001
From: Olivier Flückiger <olivf@chromium.org>
Date: Wed, 08 Jan 2025 16:06:43 +0100
Subject: [PATCH] Merged: [maglev] regalloc: handle non-loop resumable_loops
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Olivier=20Fl=C3=BCckiger?= <olivf@chromium.org>
Date: Wed, 8 Jan 2025 16:06:43 +0100
Subject: Merged: [maglev] regalloc: handle non-loop resumable_loops
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Resumable loops which are not loops can be either:
@@ -21,13 +24,12 @@ Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/branch-heads/13.2@{#60}
Cr-Branched-From: 24068c59cedad9ee976ddc05431f5f497b1ebd71-refs/heads/13.2.152@{#1}
Cr-Branched-From: 6054ba94db0969220be4f94dc1677fc4696bdc4f-refs/heads/main@{#97085}
---
diff --git a/src/maglev/maglev-interpreter-frame-state.cc b/src/maglev/maglev-interpreter-frame-state.cc
index 7d904bb..dce7517 100644
index 562f88cabaf9a32b0197c7762bb1336541b95a29..21561ea57364d9fbcecb118596141ac3af081798 100644
--- a/src/maglev/maglev-interpreter-frame-state.cc
+++ b/src/maglev/maglev-interpreter-frame-state.cc
@@ -1370,6 +1370,23 @@
@@ -1338,6 +1338,23 @@ void MergePointInterpreterFrameState::ReducePhiPredecessorCount(unsigned num) {
}
}
@@ -52,10 +54,10 @@ index 7d904bb..dce7517 100644
} // namespace internal
} // namespace v8
diff --git a/src/maglev/maglev-interpreter-frame-state.h b/src/maglev/maglev-interpreter-frame-state.h
index d8e618f..3d04b93 100644
index 0f84499a4f605013d4a9328497ef9fa1825a53af..f3802780bc49aa10869e1dd725aa769474c7697a 100644
--- a/src/maglev/maglev-interpreter-frame-state.h
+++ b/src/maglev/maglev-interpreter-frame-state.h
@@ -948,6 +948,8 @@
@@ -935,6 +935,8 @@ class MergePointInterpreterFrameState {
predecessors_so_far_ == 0;
}
@@ -65,10 +67,10 @@ index d8e618f..3d04b93 100644
return kBasicBlockTypeBits::decode(bitfield_);
}
diff --git a/src/maglev/maglev-regalloc.cc b/src/maglev/maglev-regalloc.cc
index 00456fa..a1d24ed 100644
index d21bc5128b9b8d70b86bf121a75846f57e0f113b..b11a8684e457f6a1abb6a7b1b24eab7296ae488c 100644
--- a/src/maglev/maglev-regalloc.cc
+++ b/src/maglev/maglev-regalloc.cc
@@ -157,7 +157,7 @@
@@ -155,7 +155,7 @@ bool IsLiveAtTarget(ValueNode* node, ControlNode* source, BasicBlock* target) {
}
// Drop all values on resumable loop headers.
@@ -77,7 +79,7 @@ index 00456fa..a1d24ed 100644
// TODO(verwaest): This should be true but isn't because we don't yet
// eliminate dead code.
@@ -409,13 +409,9 @@
@@ -407,13 +407,9 @@ void StraightForwardRegisterAllocator::AllocateRegisters() {
if (block->state()->is_exception_handler()) {
// Exceptions start from a blank state of register values.
ClearRegisterValues();

44
patches/v8/merged_fix_out_of_bound_string_access.patch Normal file
View File

@@ -0,0 +1,44 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Frank Tang <ftang@chromium.org>
Date: Tue, 4 Feb 2025 17:16:24 -0800
Subject: Merged: Fix out of bound string access
Bug: 386857213
(cherry picked from commit 0242cac4b20305b03b74c2e9588003378eebeb77)
Change-Id: I354e9a246ccfe36bf12fce13597f295093cde2a3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6245138
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Commit-Queue: Deepti Gandluri <gdeepti@chromium.org>
Reviewed-by: Deepti Gandluri <gdeepti@chromium.org>
Auto-Submit: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/branch-heads/13.2@{#78}
Cr-Branched-From: 24068c59cedad9ee976ddc05431f5f497b1ebd71-refs/heads/13.2.152@{#1}
Cr-Branched-From: 6054ba94db0969220be4f94dc1677fc4696bdc4f-refs/heads/main@{#97085}
diff --git a/src/objects/js-date-time-format.cc b/src/objects/js-date-time-format.cc
index 038929afeb0c8340b7e1b01e4e9d9810a10f1fee..5de38e23ea136711ace1e019cc2189a9a9bad57a 100644
--- a/src/objects/js-date-time-format.cc
+++ b/src/objects/js-date-time-format.cc
@@ -1685,6 +1685,10 @@ std::optional<std::string> GetOffsetTimeZone(Isolate* isolate,
if (m0 == ':') {
// Ignore ':'
p++;
+ if (len == p) {
+ // Error
+ return std::nullopt;
+ }
m0 = flat.Get(p);
}
if (len - p != 2) {
diff --git a/test/intl/regress-386857213.js b/test/intl/regress-386857213.js
new file mode 100644
index 0000000000000000000000000000000000000000..fec35fcc4bbc6c9920c56b7b7be1b5588254c460
--- /dev/null
+++ b/test/intl/regress-386857213.js
@@ -0,0 +1,5 @@
+// Copyright 2025 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.
+//
+assertThrows(() => new Intl.DateTimeFormat("en", {timeZone: "+09:"}), RangeError);

198
patches/v8/merged_wasm_replace_dead_code_set_with_is_dying_bit.patch Normal file
View File

@@ -0,0 +1,198 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Jakob Kummerow <jkummerow@chromium.org>
Date: Thu, 6 Feb 2025 14:42:13 +0100
Subject: Merged: [wasm] Replace {dead_code_} set with {is_dying_} bit
This saves some memory, and fixes a bug.
Fixed: 391907159
(cherry picked from commit 33ca4f51e5dbba9817eba16fd3249e66a880cf33)
Change-Id: Iad93b3e7290c25ddcedf806cc85c4401a5fcb0fc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6239106
Auto-Submit: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Matthias Liedtke <mliedtke@chromium.org>
Reviewed-by: Matthias Liedtke <mliedtke@chromium.org>
Cr-Commit-Position: refs/branch-heads/13.2@{#76}
Cr-Branched-From: 24068c59cedad9ee976ddc05431f5f497b1ebd71-refs/heads/13.2.152@{#1}
Cr-Branched-From: 6054ba94db0969220be4f94dc1677fc4696bdc4f-refs/heads/main@{#97085}
diff --git a/src/wasm/wasm-code-manager.h b/src/wasm/wasm-code-manager.h
index e0ec794fab33445b088220fbeb68ea02c1f35912..c04de89acf98209fe2dd32c5aa3868e4c2a234af 100644
--- a/src/wasm/wasm-code-manager.h
+++ b/src/wasm/wasm-code-manager.h
@@ -316,6 +316,9 @@ class V8_EXPORT_PRIVATE WasmCode final {
return ForDebuggingField::decode(flags_);
}
+ bool is_dying() const { return dying_; }
+ void mark_as_dying() { dying_ = true; }
+
// Returns {true} for Liftoff code that sets up a feedback vector slot in its
// stack frame.
// TODO(jkummerow): This can be dropped when we ship Wasm inlining.
@@ -450,6 +453,10 @@ class V8_EXPORT_PRIVATE WasmCode final {
using ForDebuggingField = ExecutionTierField::Next<ForDebugging, 2>;
using FrameHasFeedbackSlotField = ForDebuggingField::Next<bool, 1>;
+ // Will be set to {true} the first time this code object is considered
+ // "potentially dead" (to be confirmed by the next Wasm Code GC cycle).
+ std::atomic<bool> dying_{false};
+
// WasmCode is ref counted. Counters are held by:
// 1) The jump table / code table.
// 2) {WasmCodeRefScope}s.
diff --git a/src/wasm/wasm-engine.cc b/src/wasm/wasm-engine.cc
index 54ac5a1a4927d858577b9a4d984dd554999107a4..33e008c1678037e3ba7cce3a6cc16e3273e1b2ce 100644
--- a/src/wasm/wasm-engine.cc
+++ b/src/wasm/wasm-engine.cc
@@ -1700,7 +1700,6 @@ void WasmEngine::FreeNativeModule(NativeModule* native_module) {
}
// If any code objects are currently tracked as dead or near-dead, remove
// references belonging to the NativeModule that's being deleted.
- std::erase_if(dead_code_, part_of_native_module);
std::erase_if(potentially_dead_code_, part_of_native_module);
native_module_cache_.Erase(native_module);
@@ -1783,9 +1782,11 @@ void WasmEngine::ReportLiveCodeFromStackForGC(Isolate* isolate) {
bool WasmEngine::AddPotentiallyDeadCode(WasmCode* code) {
base::MutexGuard guard(&mutex_);
- if (dead_code_.contains(code)) return false; // Code is already dead.
+ if (code->is_dying()) return false;
auto added = potentially_dead_code_.insert(code);
- if (!added.second) return false; // An entry already existed.
+ DCHECK(added.second);
+ USE(added);
+ code->mark_as_dying();
new_potentially_dead_code_size_ += code->instructions().size();
if (v8_flags.wasm_code_gc) {
// Trigger a GC if 64kB plus 10% of committed code are potentially dead.
@@ -1831,19 +1832,17 @@ void WasmEngine::FreeDeadCodeLocked(const DeadCodeMap& dead_code,
const std::vector<WasmCode*>& code_vec = dead_code_entry.second;
TRACE_CODE_GC("Freeing %zu code object%s of module %p.\n", code_vec.size(),
code_vec.size() == 1 ? "" : "s", native_module);
- for (WasmCode* code : code_vec) {
- DCHECK(dead_code_.contains(code));
- dead_code_.erase(code);
- }
+#if DEBUG
+ for (WasmCode* code : code_vec) DCHECK(code->is_dying());
+#endif // DEBUG
native_module->FreeCode(base::VectorOf(code_vec));
}
if (dead_wrappers.size()) {
TRACE_CODE_GC("Freeing %zu wrapper%s.\n", dead_wrappers.size(),
dead_wrappers.size() == 1 ? "" : "s");
- for (WasmCode* code : dead_wrappers) {
- DCHECK(dead_code_.contains(code));
- dead_code_.erase(code);
- }
+#if DEBUG
+ for (WasmCode* code : dead_wrappers) DCHECK(code->is_dying());
+#endif // DEBUG
GetWasmImportWrapperCache()->Free(dead_wrappers);
}
}
@@ -1933,16 +1932,15 @@ void WasmEngine::PotentiallyFinishCurrentGC() {
if (!current_gc_info_->outstanding_isolates.empty()) return;
// All remaining code in {current_gc_info->dead_code} is really dead.
- // Move it from the set of potentially dead code to the set of dead code,
- // and decrement its ref count.
+ // Remove it from the set of potentially dead code, and decrement its
+ // ref count.
size_t num_freed = 0;
DeadCodeMap dead_code;
std::vector<WasmCode*> dead_wrappers;
for (WasmCode* code : current_gc_info_->dead_code) {
DCHECK(potentially_dead_code_.contains(code));
+ DCHECK(code->is_dying());
potentially_dead_code_.erase(code);
- DCHECK(!dead_code_.contains(code));
- dead_code_.insert(code);
if (code->DecRefOnDeadCode()) {
NativeModule* native_module = code->native_module();
if (native_module) {
@@ -1966,7 +1964,7 @@ void WasmEngine::PotentiallyFinishCurrentGC() {
}
size_t WasmEngine::EstimateCurrentMemoryConsumption() const {
- UPDATE_WHEN_CLASS_CHANGES(WasmEngine, 800);
+ UPDATE_WHEN_CLASS_CHANGES(WasmEngine, 760);
UPDATE_WHEN_CLASS_CHANGES(IsolateInfo, 184);
UPDATE_WHEN_CLASS_CHANGES(NativeModuleInfo, 56);
UPDATE_WHEN_CLASS_CHANGES(CurrentGCInfo, 96);
@@ -1977,7 +1975,6 @@ size_t WasmEngine::EstimateCurrentMemoryConsumption() const {
result += ContentSize(async_compile_jobs_);
result += async_compile_jobs_.size() * sizeof(AsyncCompileJob);
result += ContentSize(potentially_dead_code_);
- result += ContentSize(dead_code_);
// TODO(14106): Do we care about {compilation_stats_}?
// TODO(14106): Do we care about {code_tracer_}?
diff --git a/src/wasm/wasm-engine.h b/src/wasm/wasm-engine.h
index 1511753825110dedf8a6e9b8137e593a0fd2f618..50bf7c84996eef7f5ca7c160b1047050c2da30b9 100644
--- a/src/wasm/wasm-engine.h
+++ b/src/wasm/wasm-engine.h
@@ -483,12 +483,8 @@ class V8_EXPORT_PRIVATE WasmEngine {
size_t new_potentially_dead_code_size_ = 0;
// Set of potentially dead code. This set holds one ref for each code object,
// until code is detected to be really dead. At that point, the ref count is
- // decremented and code is moved to the {dead_code} set. If the code is
- // finally deleted, it is also removed from {dead_code}.
+ // decremented and code is removed from the set.
std::unordered_set<WasmCode*> potentially_dead_code_;
- // Code that is not being executed in any isolate any more, but the ref count
- // did not drop to zero yet.
- std::unordered_set<WasmCode*> dead_code_;
int8_t num_code_gcs_triggered_ = 0;
// If an engine-wide GC is currently running, this pointer stores information
diff --git a/src/wasm/wasm-import-wrapper-cache.cc b/src/wasm/wasm-import-wrapper-cache.cc
index 0554f3d50a288b34bd4729a33ac523c4902d93e0..56fa64302665fdbd036c95e29610748ccd541959 100644
--- a/src/wasm/wasm-import-wrapper-cache.cc
+++ b/src/wasm/wasm-import-wrapper-cache.cc
@@ -150,7 +150,10 @@ WasmCode* WasmImportWrapperCache::CompileWasmImportCallWrapper(
// Now that we have the lock (in the form of the cache_scope), check
// again whether another thread has just created the wrapper.
wasm_code = cache_scope[key];
- if (wasm_code) return wasm_code;
+ if (wasm_code) {
+ WasmCodeRefScope::AddRef(wasm_code);
+ if (!wasm_code->is_dying()) return wasm_code;
+ }
wasm_code = cache_scope.AddWrapper(key, std::move(result),
WasmCode::Kind::kWasmToJsWrapper);
@@ -216,6 +219,7 @@ WasmCode* WasmImportWrapperCache::MaybeGet(ImportCallKind kind,
entry_map_.find({kind, canonical_type_index, expected_arity, suspend});
if (it == entry_map_.end()) return nullptr;
WasmCodeRefScope::AddRef(it->second);
+ if (it->second->is_dying()) return nullptr;
return it->second;
}
@@ -230,6 +234,9 @@ WasmCode* WasmImportWrapperCache::Lookup(Address pc) const {
DCHECK_EQ(candidate->instruction_start(), iter->first);
if (!candidate->contains(pc)) return nullptr;
WasmCodeRefScope::AddRef(candidate);
+ // Note: this function is used for iterating the stack, where dying
+ // code objects can still have their last few activations, so we
+ // must return {candidate} even if {candidate->is_dying()}.
return candidate;
}
diff --git a/test/cctest/wasm/wasm-run-utils.cc b/test/cctest/wasm/wasm-run-utils.cc
index fb4f0c3c39aaba90729dfd1f6b9ed3f90d25ed1d..2063983ecb44eb3f50b33258e40cf7cc6d7e3089 100644
--- a/test/cctest/wasm/wasm-run-utils.cc
+++ b/test/cctest/wasm/wasm-run-utils.cc
@@ -294,6 +294,7 @@ void TestingModuleBuilder::AddIndirectFunctionTable(
WasmTableObject::AddUse(isolate_, table_obj, instance_object_, table_index);
if (function_indexes) {
+ WasmCodeRefScope code_ref_scope;
for (uint32_t i = 0; i < table_size; ++i) {
WasmFunction& function = test_module_->functions[function_indexes[i]];
int sig_id = test_module_->canonical_sig_id(function.sig_index);

397
patches/v8/wasm_fix_freeing_of_identical_shared_wrappers.patch Normal file
View File

@@ -0,0 +1,397 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Jakob Kummerow <jkummerow@chromium.org>
Date: Wed, 18 Sep 2024 11:16:07 +0200
Subject: Fix freeing of identical shared wrappers
The early-return path of WasmImportWrapperCache::Free checked the
wrong map for emptiness: {entry_map_} can have key collisions, so
previous Free() calls can empty it even though {codes_} is still
non-empty.
Drive-by: the failing test case should not have created identical
wrappers, but could do so when running multi-threaded; while this
is harmless, it's cleaner to avoid that.
Fixed: 366007153
Bug: 42204526
Change-Id: I5e387c83ec9b45a650044b7243d6e68907bc5219
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5869839
Reviewed-by: 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@{#96155}
diff --git a/src/runtime/runtime-wasm.cc b/src/runtime/runtime-wasm.cc
index 2fba518bb41e950da75012b95bccf240ac9ca166..bfd7408b78db6e8dd53e28d0e61085ee8e6e97f7 100644
--- a/src/runtime/runtime-wasm.cc
+++ b/src/runtime/runtime-wasm.cc
@@ -18,7 +18,6 @@
#include "src/objects/objects-inl.h"
#include "src/strings/unicode-inl.h"
#include "src/trap-handler/trap-handler.h"
-#include "src/wasm/compilation-environment-inl.h"
#include "src/wasm/module-compiler.h"
#include "src/wasm/serialized-signature-inl.h"
#include "src/wasm/value-type.h"
@@ -756,28 +755,9 @@ RUNTIME_FUNCTION(Runtime_TierUpWasmToJSWrapper) {
wasm::WasmCode* wasm_code =
cache->MaybeGet(kind, canonical_sig_index, expected_arity, suspend);
if (!wasm_code) {
- wasm::CompilationEnv env = wasm::CompilationEnv::ForModule(native_module);
- wasm::WasmCompilationResult result = compiler::CompileWasmImportCallWrapper(
- &env, kind, &sig, false, expected_arity, suspend);
- {
- wasm::WasmImportWrapperCache::ModificationScope cache_scope(cache);
- wasm::WasmImportWrapperCache::CacheKey key(kind, canonical_sig_index,
- expected_arity, suspend);
- wasm_code = cache_scope.AddWrapper(
- key, std::move(result), wasm::WasmCode::Kind::kWasmToJsWrapper);
- }
- // To avoid lock order inversion, code printing must happen after the
- // end of the {cache_scope}.
- wasm_code->MaybePrint();
- isolate->counters()->wasm_generated_code_size()->Increment(
- wasm_code->instructions().length());
- isolate->counters()->wasm_reloc_size()->Increment(
- wasm_code->reloc_info().length());
- if (V8_UNLIKELY(native_module->log_code())) {
- wasm::GetWasmEngine()->LogWrapperCode(base::VectorOf(&wasm_code, 1));
- // Log the code immediately in the current isolate.
- wasm::GetWasmEngine()->LogOutstandingCodesForIsolate(isolate);
- }
+ wasm_code = cache->CompileWasmImportCallWrapper(
+ isolate, native_module, kind, &sig, canonical_sig_index, false,
+ expected_arity, suspend);
}
// Note: we don't need to decrement any refcounts here, because tier-up
// doesn't overwrite an existing compiled wrapper, and the generic wrapper
diff --git a/src/wasm/module-compiler.cc b/src/wasm/module-compiler.cc
index 43f1215ea5a394f93d837603de50859eef78ce3f..d5a9e007258e1d7aab33c8b0a29a6fcdaf4452b4 100644
--- a/src/wasm/module-compiler.cc
+++ b/src/wasm/module-compiler.cc
@@ -4622,8 +4622,9 @@ void CompileJsToWasmWrappers(Isolate* isolate, const WasmModule* module) {
}
}
-WasmCode* CompileImportWrapperForTest(NativeModule* native_module,
- Counters* counters, ImportCallKind kind,
+WasmCode* CompileImportWrapperForTest(Isolate* isolate,
+ NativeModule* native_module,
+ ImportCallKind kind,
const FunctionSig* sig,
uint32_t canonical_type_index,
int expected_arity, Suspend suspend) {
@@ -4637,35 +4638,9 @@ WasmCode* CompileImportWrapperForTest(NativeModule* native_module,
return nullptr;
}
- CompilationEnv env = CompilationEnv::ForModule(native_module);
- WasmCompilationResult result = compiler::CompileWasmImportCallWrapper(
- &env, kind, sig, source_positions, expected_arity, suspend);
-
- DCHECK(result.inlining_positions.empty());
- DCHECK(result.deopt_data.empty());
- WasmCode* code;
- {
- // There was no cache entry when we called this function, but in the
- // meantime a different module could have created one. Simply discard the
- // new wrapper if so.
- WasmImportWrapperCache::ModificationScope cache_scope(
- GetWasmImportWrapperCache());
- WasmImportWrapperCache::CacheKey key(kind, canonical_type_index,
- expected_arity, suspend);
- if (V8_UNLIKELY(cache_scope[key] != nullptr)) return cache_scope[key];
- code = cache_scope.AddWrapper(key, std::move(result),
- WasmCode::Kind::kWasmToJsWrapper);
- }
- // To avoid lock order inversion, code printing must happen after the
- // end of the {cache_scope}.
- code->MaybePrint();
- counters->wasm_generated_code_size()->Increment(
- code->instructions().length());
- counters->wasm_reloc_size()->Increment(code->reloc_info().length());
- if (native_module->log_code()) {
- GetWasmEngine()->LogWrapperCode(base::VectorOf(&code, 1));
- }
- return code;
+ return GetWasmImportWrapperCache()->CompileWasmImportCallWrapper(
+ isolate, native_module, kind, sig, canonical_type_index, source_positions,
+ expected_arity, suspend);
}
} // namespace v8::internal::wasm
diff --git a/src/wasm/module-compiler.h b/src/wasm/module-compiler.h
index d6cf89381ceaaf38f299f4d3f75d474d54e82633..b757e6385301af9a2d3816d3d4d3fb552c4842b8 100644
--- a/src/wasm/module-compiler.h
+++ b/src/wasm/module-compiler.h
@@ -70,8 +70,9 @@ V8_EXPORT_PRIVATE WasmError ValidateAndSetBuiltinImports(
// cache entry. Assumes the key already exists in the cache but has not been
// compiled yet.
V8_EXPORT_PRIVATE
-WasmCode* CompileImportWrapperForTest(NativeModule* native_module,
- Counters* counters, ImportCallKind kind,
+WasmCode* CompileImportWrapperForTest(Isolate* isolate,
+ NativeModule* native_module,
+ ImportCallKind kind,
const FunctionSig* sig,
uint32_t canonical_type_index,
int expected_arity, Suspend suspend);
diff --git a/src/wasm/module-instantiate.cc b/src/wasm/module-instantiate.cc
index f4c2a75f7fd44d6ff2b2ab61d06d9d8b2f1965c2..8d9f81a7f1dad2f37f2b036fdf8b4da3c70e2c38 100644
--- a/src/wasm/module-instantiate.cc
+++ b/src/wasm/module-instantiate.cc
@@ -2007,38 +2007,9 @@ bool InstanceBuilder::ProcessImportedFunction(
// generic wrapper will be used (see above).
NativeModule* native_module = trusted_instance_data->native_module();
bool source_positions = is_asmjs_module(native_module->module());
- CompilationEnv env = CompilationEnv::ForModule(native_module);
- WasmCompilationResult result = compiler::CompileWasmImportCallWrapper(
- &env, kind, expected_sig, source_positions, expected_arity,
- resolved.suspend());
- bool code_is_new = false;
- {
- WasmImportWrapperCache::ModificationScope cache_scope(cache);
- WasmImportWrapperCache::CacheKey key(
- kind, canonical_sig_id, expected_arity, resolved.suspend());
- // Now that we have the lock (in the form of the cache_scope), check
- // again whether another thread has just created the wrapper.
- wasm_code = cache_scope[key];
- if (!wasm_code) {
- wasm_code = cache_scope.AddWrapper(
- key, std::move(result), WasmCode::Kind::kWasmToJsWrapper);
- code_is_new = true;
- }
- }
- if (code_is_new) {
- // To avoid lock order inversion, code printing must happen after the
- // end of the {cache_scope}.
- wasm_code->MaybePrint();
- isolate_->counters()->wasm_generated_code_size()->Increment(
- wasm_code->instructions().length());
- isolate_->counters()->wasm_reloc_size()->Increment(
- wasm_code->reloc_info().length());
- if (V8_UNLIKELY(native_module->log_code())) {
- GetWasmEngine()->LogWrapperCode(base::VectorOf(&wasm_code, 1));
- // Log the code immediately in the current isolate.
- GetWasmEngine()->LogOutstandingCodesForIsolate(isolate_);
- }
- }
+ wasm_code = cache->CompileWasmImportCallWrapper(
+ isolate_, native_module, kind, expected_sig, canonical_sig_id,
+ source_positions, expected_arity, resolved.suspend());
}
DCHECK_NOT_NULL(wasm_code);
diff --git a/src/wasm/wasm-code-pointer-table.cc b/src/wasm/wasm-code-pointer-table.cc
index c7b5ae3455bb1290651b5fcf7460a31772069d88..725cca9de5f59c7430c2c4f437c95342bb93d928 100644
--- a/src/wasm/wasm-code-pointer-table.cc
+++ b/src/wasm/wasm-code-pointer-table.cc
@@ -13,10 +13,7 @@ void WasmCodePointerTable::Initialize() { Base::Initialize(); }
void WasmCodePointerTable::TearDown() {
SweepSegments(0);
- // TODO(366007153): the WasmCode destructor sometimes doesn't get called and
- // we can have a leftover entry in the table. Re-enable the DCHECK once the
- // bug is fixed.
- // DCHECK(freelist_head_.load().is_empty());
+ DCHECK(freelist_head_.load().is_empty());
Base::TearDown();
}
diff --git a/src/wasm/wasm-import-wrapper-cache.cc b/src/wasm/wasm-import-wrapper-cache.cc
index 92b6470ce094a0c8b026ea4208a59c25384da3d4..0554f3d50a288b34bd4729a33ac523c4902d93e0 100644
--- a/src/wasm/wasm-import-wrapper-cache.cc
+++ b/src/wasm/wasm-import-wrapper-cache.cc
@@ -9,6 +9,8 @@
#include "src/codegen/assembler-inl.h"
#include "src/codegen/flush-instruction-cache.h"
#include "src/common/code-memory-access-inl.h"
+#include "src/compiler/wasm-compiler.h"
+#include "src/wasm/compilation-environment-inl.h"
#include "src/wasm/function-compiler.h"
#include "src/wasm/std-object-sizes.h"
#include "src/wasm/wasm-code-manager.h"
@@ -134,6 +136,41 @@ WasmCode* WasmImportWrapperCache::ModificationScope::AddWrapper(
return code;
}
+WasmCode* WasmImportWrapperCache::CompileWasmImportCallWrapper(
+ Isolate* isolate, NativeModule* native_module, ImportCallKind kind,
+ const FunctionSig* sig, uint32_t canonical_sig_index, bool source_positions,
+ int expected_arity, Suspend suspend) {
+ CompilationEnv env = CompilationEnv::ForModule(native_module);
+ WasmCompilationResult result = compiler::CompileWasmImportCallWrapper(
+ &env, kind, sig, source_positions, expected_arity, suspend);
+ WasmCode* wasm_code;
+ {
+ ModificationScope cache_scope(this);
+ CacheKey key(kind, canonical_sig_index, expected_arity, suspend);
+ // Now that we have the lock (in the form of the cache_scope), check
+ // again whether another thread has just created the wrapper.
+ wasm_code = cache_scope[key];
+ if (wasm_code) return wasm_code;
+
+ wasm_code = cache_scope.AddWrapper(key, std::move(result),
+ WasmCode::Kind::kWasmToJsWrapper);
+ }
+
+ // To avoid lock order inversion, code printing must happen after the
+ // end of the {cache_scope}.
+ wasm_code->MaybePrint();
+ isolate->counters()->wasm_generated_code_size()->Increment(
+ wasm_code->instructions().length());
+ isolate->counters()->wasm_reloc_size()->Increment(
+ wasm_code->reloc_info().length());
+ if (V8_UNLIKELY(native_module->log_code())) {
+ GetWasmEngine()->LogWrapperCode(base::VectorOf(&wasm_code, 1));
+ // Log the code immediately in the current isolate.
+ GetWasmEngine()->LogOutstandingCodesForIsolate(isolate);
+ }
+ return wasm_code;
+}
+
void WasmImportWrapperCache::LogForIsolate(Isolate* isolate) {
for (const auto& entry : codes_) {
entry.second->LogCode(isolate, "", -1); // No source URL, no ScriptId.
@@ -142,7 +179,7 @@ void WasmImportWrapperCache::LogForIsolate(Isolate* isolate) {
void WasmImportWrapperCache::Free(std::vector<WasmCode*>& wrappers) {
base::MutexGuard lock(&mutex_);
- if (entry_map_.empty() || wrappers.empty()) return;
+ if (codes_.empty() || wrappers.empty()) return;
// {WasmCodeAllocator::FreeCode()} wants code objects to be sorted.
std::sort(wrappers.begin(), wrappers.end(), [](WasmCode* a, WasmCode* b) {
return a->instruction_start() < b->instruction_start();
diff --git a/src/wasm/wasm-import-wrapper-cache.h b/src/wasm/wasm-import-wrapper-cache.h
index 4768e90d0d2332650eacf093932465262676d1c7..4124bd77059c9b59e691dcc9c2bda44efe9f4609 100644
--- a/src/wasm/wasm-import-wrapper-cache.h
+++ b/src/wasm/wasm-import-wrapper-cache.h
@@ -98,6 +98,11 @@ class WasmImportWrapperCache {
return iter->second;
}
+ WasmCode* CompileWasmImportCallWrapper(
+ Isolate* isolate, NativeModule* native_module, ImportCallKind kind,
+ const FunctionSig* sig, uint32_t canonical_sig_index,
+ bool source_positions, int expected_arity, Suspend suspend);
+
private:
std::unique_ptr<WasmCodeAllocator> code_allocator_;
mutable base::Mutex mutex_;
diff --git a/src/wasm/wasm-objects.cc b/src/wasm/wasm-objects.cc
index f6c100687a88d8fa269ce02506ab0a82b2c5ffbb..a34d48c24cb8cd478705ecb31664186bcd9ac4f3 100644
--- a/src/wasm/wasm-objects.cc
+++ b/src/wasm/wasm-objects.cc
@@ -19,7 +19,6 @@
#include "src/roots/roots-inl.h"
#include "src/utils/utils.h"
#include "src/wasm/code-space-access.h"
-#include "src/wasm/compilation-environment-inl.h"
#include "src/wasm/module-compiler.h"
#include "src/wasm/module-decoder.h"
#include "src/wasm/module-instantiate.h"
@@ -1919,28 +1918,9 @@ void WasmTrustedInstanceData::ImportWasmJSFunctionIntoTable(
} else if (UseGenericWasmToJSWrapper(kind, sig, resolved.suspend())) {
call_target = Builtins::EntryOf(Builtin::kWasmToJsWrapperAsm, isolate);
} else {
- wasm::CompilationEnv env = wasm::CompilationEnv::ForModule(native_module);
- wasm::WasmCompilationResult result = compiler::CompileWasmImportCallWrapper(
- &env, kind, sig, false, expected_arity, suspend);
- {
- wasm::WasmImportWrapperCache::ModificationScope cache_scope(cache);
- wasm::WasmImportWrapperCache::CacheKey key(kind, canonical_sig_id,
- expected_arity, suspend);
- wasm_code = cache_scope.AddWrapper(
- key, std::move(result), wasm::WasmCode::Kind::kWasmToJsWrapper);
- }
- // To avoid lock order inversion, code printing must happen after the
- // end of the {cache_scope}.
- wasm_code->MaybePrint();
- isolate->counters()->wasm_generated_code_size()->Increment(
- wasm_code->instructions().length());
- isolate->counters()->wasm_reloc_size()->Increment(
- wasm_code->reloc_info().length());
- if (V8_UNLIKELY(native_module->log_code())) {
- wasm::GetWasmEngine()->LogWrapperCode(base::VectorOf(&wasm_code, 1));
- // Log the code immediately in the current isolate.
- wasm::GetWasmEngine()->LogOutstandingCodesForIsolate(isolate);
- }
+ wasm_code = cache->CompileWasmImportCallWrapper(
+ isolate, native_module, kind, sig, canonical_sig_id, false,
+ expected_arity, suspend);
call_target = wasm_code->instruction_start();
}
diff --git a/test/cctest/wasm/test-wasm-import-wrapper-cache.cc b/test/cctest/wasm/test-wasm-import-wrapper-cache.cc
index e713648644e0d6dfc191f35d61a6e3be71ca6d38..57c5caa5eea3dd059348431270d58f48d8bb318e 100644
--- a/test/cctest/wasm/test-wasm-import-wrapper-cache.cc
+++ b/test/cctest/wasm/test-wasm-import-wrapper-cache.cc
@@ -42,9 +42,9 @@ TEST(CacheHit) {
int expected_arity = static_cast<int>(sig->parameter_count());
{
WasmCodeRefScope wasm_code_ref_scope;
- WasmCode* c1 = CompileImportWrapperForTest(
- module.get(), isolate->counters(), kind, sig, canonical_type_index,
- expected_arity, kNoSuspend);
+ WasmCode* c1 = CompileImportWrapperForTest(isolate, module.get(), kind, sig,
+ canonical_type_index,
+ expected_arity, kNoSuspend);
CHECK_NOT_NULL(c1);
CHECK_EQ(WasmCode::Kind::kWasmToJsWrapper, c1->kind());
@@ -79,8 +79,8 @@ TEST(CacheMissSig) {
uint32_t canonical_type_index2 =
GetTypeCanonicalizer()->AddRecursiveGroup(sig2);
- WasmCode* c1 = CompileImportWrapperForTest(module.get(), isolate->counters(),
- kind, sig1, canonical_type_index1,
+ WasmCode* c1 = CompileImportWrapperForTest(isolate, module.get(), kind, sig1,
+ canonical_type_index1,
expected_arity1, kNoSuspend);
CHECK_NOT_NULL(c1);
@@ -105,8 +105,8 @@ TEST(CacheMissKind) {
uint32_t canonical_type_index =
GetTypeCanonicalizer()->AddRecursiveGroup(sig);
- WasmCode* c1 = CompileImportWrapperForTest(module.get(), isolate->counters(),
- kind1, sig, canonical_type_index,
+ WasmCode* c1 = CompileImportWrapperForTest(isolate, module.get(), kind1, sig,
+ canonical_type_index,
expected_arity, kNoSuspend);
CHECK_NOT_NULL(c1);
@@ -134,8 +134,8 @@ TEST(CacheHitMissSig) {
uint32_t canonical_type_index2 =
GetTypeCanonicalizer()->AddRecursiveGroup(sig2);
- WasmCode* c1 = CompileImportWrapperForTest(module.get(), isolate->counters(),
- kind, sig1, canonical_type_index1,
+ WasmCode* c1 = CompileImportWrapperForTest(isolate, module.get(), kind, sig1,
+ canonical_type_index1,
expected_arity1, kNoSuspend);
CHECK_NOT_NULL(c1);
@@ -146,8 +146,8 @@ TEST(CacheHitMissSig) {
CHECK_NULL(c2);
- c2 = CompileImportWrapperForTest(module.get(), isolate->counters(), kind,
- sig2, canonical_type_index2, expected_arity2,
+ c2 = CompileImportWrapperForTest(isolate, module.get(), kind, sig2,
+ canonical_type_index2, expected_arity2,
kNoSuspend);
CHECK_NE(c1, c2);
diff --git a/test/cctest/wasm/wasm-run-utils.cc b/test/cctest/wasm/wasm-run-utils.cc
index a8b93c9ebd53cf10b0c833139358254127d5deca..fb4f0c3c39aaba90729dfd1f6b9ed3f90d25ed1d 100644
--- a/test/cctest/wasm/wasm-run-utils.cc
+++ b/test/cctest/wasm/wasm-run-utils.cc
@@ -99,7 +99,7 @@ TestingModuleBuilder::TestingModuleBuilder(
kNoSuspend);
if (import_wrapper == nullptr) {
import_wrapper = CompileImportWrapperForTest(
- native_module_, isolate_->counters(), kind, sig, canonical_type_index,
+ isolate_, native_module_, kind, sig, canonical_type_index,
static_cast<int>(sig->parameter_count()), kNoSuspend);
}