Skip to content

Commit b471bc9

Browse files
backesCommit Bot
backes
authored and
Commit Bot
committed
[wasm] Send a single scriptParsed event per script
If a script was shared between multiple modules (because they used the same wire bytes) it could happen that we still triggered multiple "scriptParsed" events via CDP. This was because {WasmEngine::GetOrCreateScript} did not communicate back whether it used a cached script or whether it created a new one. This CL moves the call to {Debug::OnAfterCompile} (which triggers the "scriptParsed" event) to the {WasmEngine::GetOrCreateScript} method, such that we only call it once per script. Since the engine only holds a weak reference to the script, we would still trigger multiple events if the script is garbage-collected in the meantime. In this case there is no way around this, as the new script would have a new ID, hence we need to emit a new event to make it public to the debugger. R=thibaudm@chromium.org CC=bmeurer@chromium.org Bug: chromium:1151211 Change-Id: I1a7986514fd708680541a0e5dc24e60f01f42c28 Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng Cq-Include-Trybots: luci.v8.try:v8_mac64_gc_stress_dbg_ng Cq-Include-Trybots: luci.v8.try:v8_linux_gc_stress_dbg_ng Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2687755 Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Reviewed-by: Thibaud Michaud <thibaudm@chromium.org> Commit-Queue: Clemens Backes <clemensb@chromium.org> Cr-Commit-Position: refs/heads/master@{#72648}
1 parent 11b6f17 commit b471bc9

6 files changed

Lines changed: 42 additions & 65 deletions

File tree

‎src/wasm/module-compiler.cc‎

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1928,35 +1928,17 @@ void AsyncCompileJob::FinishCompile(bool is_after_cache_hit) {
19281928
}
19291929
}
19301930

1931-
DCHECK(!isolate_->context().is_null());
1932-
// Finish the wasm script now and make it public to the debugger.
1933-
Handle<Script> script(module_object_->script(), isolate_);
1934-
const WasmModule* module = module_object_->module();
1935-
if (script->type() == Script::TYPE_WASM &&
1936-
module->debug_symbols.type == WasmDebugSymbols::Type::SourceMap &&
1937-
!module->debug_symbols.external_url.is_empty()) {
1938-
ModuleWireBytes wire_bytes(module_object_->native_module()->wire_bytes());
1939-
MaybeHandle<String> src_map_str = isolate_->factory()->NewStringFromUtf8(
1940-
wire_bytes.GetNameOrNull(module->debug_symbols.external_url),
1941-
AllocationType::kOld);
1942-
script->set_source_mapping_url(*src_map_str.ToHandleChecked());
1943-
}
1944-
{
1945-
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.wasm.detailed"),
1946-
"wasm.Debug.OnAfterCompile");
1947-
isolate_->debug()->OnAfterCompile(script);
1948-
}
1949-
19501931
// TODO(bbudge) Allow deserialization without wrapper compilation, so we can
19511932
// just compile wrappers here.
19521933
if (!is_after_deserialization) {
19531934
Handle<FixedArray> export_wrappers;
19541935
if (is_after_cache_hit) {
19551936
// TODO(thibaudm): Look into sharing wrappers.
1956-
CompileJsToWasmWrappers(isolate_, module, &export_wrappers);
1937+
CompileJsToWasmWrappers(isolate_, module_object_->module(),
1938+
&export_wrappers);
19571939
} else {
1958-
compilation_state->FinalizeJSToWasmWrappers(isolate_, module,
1959-
&export_wrappers);
1940+
compilation_state->FinalizeJSToWasmWrappers(
1941+
isolate_, module_object_->module(), &export_wrappers);
19601942
}
19611943
module_object_->set_export_wrappers(*export_wrappers);
19621944
}

‎src/wasm/wasm-engine.cc‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -542,12 +542,8 @@ MaybeHandle<WasmModuleObject> WasmEngine::SyncCompile(
542542
// and information needed at instantiation time. This object needs to be
543543
// serializable. Instantiation may occur off a deserialized version of this
544544
// object.
545-
Handle<WasmModuleObject> module_object = WasmModuleObject::New(
546-
isolate, std::move(native_module), script, export_wrappers);
547-
548-
// Finish the Wasm script now and make it public to the debugger.
549-
isolate->debug()->OnAfterCompile(script);
550-
return module_object;
545+
return WasmModuleObject::New(isolate, std::move(native_module), script,
546+
export_wrappers);
551547
}
552548

553549
MaybeHandle<WasmInstanceObject> WasmEngine::SyncInstantiate(
@@ -841,8 +837,6 @@ Handle<WasmModuleObject> WasmEngine::ImportNativeModule(
841837
native_modules_[native_module]->isolates.insert(isolate);
842838
}
843839

844-
// Finish the Wasm script now and make it public to the debugger.
845-
isolate->debug()->OnAfterCompile(script);
846840
return module_object;
847841
}
848842

@@ -1347,11 +1341,10 @@ Handle<Script> WasmEngine::GetOrCreateScript(
13471341
auto it = scripts.find(native_module.get());
13481342
if (it != scripts.end()) {
13491343
Handle<Script> weak_global_handle = it->second.handle();
1350-
if (weak_global_handle.is_null()) {
1351-
scripts.erase(it);
1352-
} else {
1344+
if (!weak_global_handle.is_null()) {
13531345
return Handle<Script>::New(*weak_global_handle, isolate);
13541346
}
1347+
scripts.erase(it);
13551348
}
13561349
}
13571350
// Temporarily release the mutex to let the GC collect native modules.
@@ -1362,8 +1355,14 @@ Handle<Script> WasmEngine::GetOrCreateScript(
13621355
auto& scripts = isolates_[isolate]->scripts;
13631356
DCHECK_EQ(0, scripts.count(native_module.get()));
13641357
scripts.emplace(native_module.get(), WeakScriptHandle(script));
1365-
return script;
13661358
}
1359+
// Make the new script available to debuggers.
1360+
{
1361+
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.wasm.detailed"),
1362+
"wasm.Debug.OnAfterCompile");
1363+
isolate->debug()->OnAfterCompile(script);
1364+
}
1365+
return script;
13671366
}
13681367

13691368
std::shared_ptr<OperationsBarrier>

‎src/wasm/wasm-engine.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,9 @@ class V8_EXPORT_PRIVATE WasmEngine {
331331
void FreeDeadCode(const DeadCodeMap&);
332332
void FreeDeadCodeLocked(const DeadCodeMap&);
333333

334+
// Get the script for a specific native module in a specific isolate. If it
335+
// does not exist (or was garbage collected in the meantime), create a new
336+
// script and make it public to debuggers.
334337
Handle<Script> GetOrCreateScript(Isolate*,
335338
const std::shared_ptr<NativeModule>&,
336339
Vector<const char> source_url);

‎src/wasm/wasm-serialization.cc‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -873,9 +873,6 @@ MaybeHandle<WasmModuleObject> DeserializeNativeModule(
873873
Handle<WasmModuleObject> module_object = WasmModuleObject::New(
874874
isolate, shared_native_module, script, export_wrappers);
875875

876-
// Finish the Wasm script now and make it public to the debugger.
877-
isolate->debug()->OnAfterCompile(script);
878-
879876
// Log the code within the generated module for profiling.
880877
shared_native_module->LogWasmCodes(isolate, *script);
881878

‎test/debugger/regress/regress-crbug-1032042.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,14 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
// Flags: --expose-gc
6+
57
load("test/mjsunit/wasm/wasm-module-builder.js");
68

9+
// Run a gc() initially, to get rid of the cached script in stress mode (such
10+
// that it gets reported to the debugger again).
11+
gc();
12+
713
const Debug = new DebugWrapper();
814
Debug.enable();
915

‎test/inspector/debugger/wasm-scripts-expected.txt‎

Lines changed: 18 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,34 +2,24 @@ Tests how wasm scripts are reported
22
Check that each inspector gets a wasm script at module creation time.
33
Session #1: Script #0 parsed. URL: wasm://wasm/7b04570e. Script ID: 0, Source map URL: , debug symbols: None:undefined. module begin: 0, module end: 77, code offset: 34
44
Session #2: Script #0 parsed. URL: wasm://wasm/7b04570e. Script ID: 0, Source map URL: , debug symbols: None:undefined. module begin: 0, module end: 77, code offset: 34
5-
Session #1: Script #1 parsed. URL: wasm://wasm/7b04570e. Script ID: 0, Source map URL: , debug symbols: None:undefined. module begin: 0, module end: 77, code offset: 34
6-
Session #2: Script #1 parsed. URL: wasm://wasm/7b04570e. Script ID: 0, Source map URL: , debug symbols: None:undefined. module begin: 0, module end: 77, code offset: 34
7-
Session #1: Script #2 parsed. URL: wasm://wasm/21e2f406. Script ID: 1, Source map URL: , debug symbols: ExternalDWARF:abc. module begin: 0, module end: 103, code offset: 34
8-
Session #2: Script #2 parsed. URL: wasm://wasm/21e2f406. Script ID: 1, Source map URL: , debug symbols: ExternalDWARF:abc. module begin: 0, module end: 103, code offset: 34
9-
Session #1: Script #3 parsed. URL: wasm://wasm/ba7c35be. Script ID: 2, Source map URL: , debug symbols: EmbeddedDWARF:undefined. module begin: 0, module end: 96, code offset: 34
10-
Session #2: Script #3 parsed. URL: wasm://wasm/ba7c35be. Script ID: 2, Source map URL: , debug symbols: EmbeddedDWARF:undefined. module begin: 0, module end: 96, code offset: 34
11-
Session #1: Script #4 parsed. URL: wasm://wasm/1baa71fe. Script ID: 3, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 100, code offset: 34
12-
Session #2: Script #4 parsed. URL: wasm://wasm/1baa71fe. Script ID: 3, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 100, code offset: 34
13-
Session #1: Script #5 parsed. URL: wasm://wasm/c047292e. Script ID: 4, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 126, code offset: 34
14-
Session #2: Script #5 parsed. URL: wasm://wasm/c047292e. Script ID: 4, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 126, code offset: 34
15-
Session #1: Script #6 parsed. URL: wasm://wasm/e56b2672. Script ID: 5, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 126, code offset: 34
16-
Session #2: Script #6 parsed. URL: wasm://wasm/e56b2672. Script ID: 5, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 126, code offset: 34
17-
Session #1: Script #7 parsed. URL: wasm://wasm/c9614a4e. Script ID: 6, Source map URL: , debug symbols: ExternalDWARF:abc. module begin: 0, module end: 122, code offset: 34
18-
Session #2: Script #7 parsed. URL: wasm://wasm/c9614a4e. Script ID: 6, Source map URL: , debug symbols: ExternalDWARF:abc. module begin: 0, module end: 122, code offset: 34
19-
Session #1: Script #8 parsed. URL: wasm://wasm/639d13c6. Script ID: 7, Source map URL: , debug symbols: ExternalDWARF:abc. module begin: 0, module end: 122, code offset: 34
20-
Session #2: Script #8 parsed. URL: wasm://wasm/639d13c6. Script ID: 7, Source map URL: , debug symbols: ExternalDWARF:abc. module begin: 0, module end: 122, code offset: 34
21-
Session #1: Script #9 parsed. URL: wasm://wasm/95e97206. Script ID: 8, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 119, code offset: 34
22-
Session #2: Script #9 parsed. URL: wasm://wasm/95e97206. Script ID: 8, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 119, code offset: 34
23-
Session #1: Script #10 parsed. URL: wasm://wasm/7ab47392. Script ID: 9, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 119, code offset: 34
24-
Session #2: Script #10 parsed. URL: wasm://wasm/7ab47392. Script ID: 9, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 119, code offset: 34
25-
Session #1: Source for wasm://wasm/7b04570e:
26-
Raw: 00 61 73 6d 01 00 00 00 01 07 02 60 00 00 60 00 00 03 03 02 00 01 07 08 01 04 6d 61 69 6e 00 01 0a 0e 02 03 00 01 0b 08 00 02 40 41 02 1a 0b 0b 00 1b 04 6e 61 6d 65 01 14 02 00 0b 6e 6f 70 46 75 6e 63 74 69 6f 6e 01 04 6d 61 69 6e
27-
Imports: []
28-
Exports: [main: function]
29-
Session #2: Source for wasm://wasm/7b04570e:
30-
Raw: 00 61 73 6d 01 00 00 00 01 07 02 60 00 00 60 00 00 03 03 02 00 01 07 08 01 04 6d 61 69 6e 00 01 0a 0e 02 03 00 01 0b 08 00 02 40 41 02 1a 0b 0b 00 1b 04 6e 61 6d 65 01 14 02 00 0b 6e 6f 70 46 75 6e 63 74 69 6f 6e 01 04 6d 61 69 6e
31-
Imports: []
32-
Exports: [main: function]
5+
Session #1: Script #1 parsed. URL: wasm://wasm/21e2f406. Script ID: 1, Source map URL: , debug symbols: ExternalDWARF:abc. module begin: 0, module end: 103, code offset: 34
6+
Session #2: Script #1 parsed. URL: wasm://wasm/21e2f406. Script ID: 1, Source map URL: , debug symbols: ExternalDWARF:abc. module begin: 0, module end: 103, code offset: 34
7+
Session #1: Script #2 parsed. URL: wasm://wasm/ba7c35be. Script ID: 2, Source map URL: , debug symbols: EmbeddedDWARF:undefined. module begin: 0, module end: 96, code offset: 34
8+
Session #2: Script #2 parsed. URL: wasm://wasm/ba7c35be. Script ID: 2, Source map URL: , debug symbols: EmbeddedDWARF:undefined. module begin: 0, module end: 96, code offset: 34
9+
Session #1: Script #3 parsed. URL: wasm://wasm/1baa71fe. Script ID: 3, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 100, code offset: 34
10+
Session #2: Script #3 parsed. URL: wasm://wasm/1baa71fe. Script ID: 3, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 100, code offset: 34
11+
Session #1: Script #4 parsed. URL: wasm://wasm/c047292e. Script ID: 4, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 126, code offset: 34
12+
Session #2: Script #4 parsed. URL: wasm://wasm/c047292e. Script ID: 4, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 126, code offset: 34
13+
Session #1: Script #5 parsed. URL: wasm://wasm/e56b2672. Script ID: 5, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 126, code offset: 34
14+
Session #2: Script #5 parsed. URL: wasm://wasm/e56b2672. Script ID: 5, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 126, code offset: 34
15+
Session #1: Script #6 parsed. URL: wasm://wasm/c9614a4e. Script ID: 6, Source map URL: , debug symbols: ExternalDWARF:abc. module begin: 0, module end: 122, code offset: 34
16+
Session #2: Script #6 parsed. URL: wasm://wasm/c9614a4e. Script ID: 6, Source map URL: , debug symbols: ExternalDWARF:abc. module begin: 0, module end: 122, code offset: 34
17+
Session #1: Script #7 parsed. URL: wasm://wasm/639d13c6. Script ID: 7, Source map URL: , debug symbols: ExternalDWARF:abc. module begin: 0, module end: 122, code offset: 34
18+
Session #2: Script #7 parsed. URL: wasm://wasm/639d13c6. Script ID: 7, Source map URL: , debug symbols: ExternalDWARF:abc. module begin: 0, module end: 122, code offset: 34
19+
Session #1: Script #8 parsed. URL: wasm://wasm/95e97206. Script ID: 8, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 119, code offset: 34
20+
Session #2: Script #8 parsed. URL: wasm://wasm/95e97206. Script ID: 8, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 119, code offset: 34
21+
Session #1: Script #9 parsed. URL: wasm://wasm/7ab47392. Script ID: 9, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 119, code offset: 34
22+
Session #2: Script #9 parsed. URL: wasm://wasm/7ab47392. Script ID: 9, Source map URL: abc, debug symbols: SourceMap:abc. module begin: 0, module end: 119, code offset: 34
3323
Session #1: Source for wasm://wasm/7b04570e:
3424
Raw: 00 61 73 6d 01 00 00 00 01 07 02 60 00 00 60 00 00 03 03 02 00 01 07 08 01 04 6d 61 69 6e 00 01 0a 0e 02 03 00 01 0b 08 00 02 40 41 02 1a 0b 0b 00 1b 04 6e 61 6d 65 01 14 02 00 0b 6e 6f 70 46 75 6e 63 74 69 6f 6e 01 04 6d 61 69 6e
3525
Imports: []

0 commit comments

Comments
 (0)