[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}
2021-02-10 18:50:33 +01:00 · 2021-02-10 18:50:33 +01:00 · b471bc9318
commit b471bc9318
parent 11b6f1760e
6 changed files with 42 additions and 65 deletions
--- a/src/wasm/module-compiler.cc
+++ b/src/wasm/module-compiler.cc
@ -1928,35 +1928,17 @@ void AsyncCompileJob::FinishCompile(bool is_after_cache_hit) {
    }
  }

-  DCHECK(!isolate_->context().is_null());
-  // Finish the wasm script now and make it public to the debugger.
-  Handle<Script> script(module_object_->script(), isolate_);
-  const WasmModule* module = module_object_->module();
-  if (script->type() == Script::TYPE_WASM &&
-      module->debug_symbols.type == WasmDebugSymbols::Type::SourceMap &&
-      !module->debug_symbols.external_url.is_empty()) {
-    ModuleWireBytes wire_bytes(module_object_->native_module()->wire_bytes());
-    MaybeHandle<String> src_map_str = isolate_->factory()->NewStringFromUtf8(
-        wire_bytes.GetNameOrNull(module->debug_symbols.external_url),
-        AllocationType::kOld);
-    script->set_source_mapping_url(*src_map_str.ToHandleChecked());
-  }
-  {
-    TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.wasm.detailed"),
-                 "wasm.Debug.OnAfterCompile");
-    isolate_->debug()->OnAfterCompile(script);
-  }
-
  // TODO(bbudge) Allow deserialization without wrapper compilation, so we can
  // just compile wrappers here.
  if (!is_after_deserialization) {
    Handle<FixedArray> export_wrappers;
    if (is_after_cache_hit) {
      // TODO(thibaudm): Look into sharing wrappers.
-      CompileJsToWasmWrappers(isolate_, module, &export_wrappers);
+      CompileJsToWasmWrappers(isolate_, module_object_->module(),
+                              &export_wrappers);
    } else {
-      compilation_state->FinalizeJSToWasmWrappers(isolate_, module,
-                                                  &export_wrappers);
+      compilation_state->FinalizeJSToWasmWrappers(
+          isolate_, module_object_->module(), &export_wrappers);
    }
    module_object_->set_export_wrappers(*export_wrappers);
  }
--- a/src/wasm/wasm-engine.cc
+++ b/src/wasm/wasm-engine.cc
@ -542,12 +542,8 @@ MaybeHandle<WasmModuleObject> WasmEngine::SyncCompile(
  // and information needed at instantiation time. This object needs to be
  // serializable. Instantiation may occur off a deserialized version of this
  // object.
-  Handle<WasmModuleObject> module_object = WasmModuleObject::New(
-      isolate, std::move(native_module), script, export_wrappers);
-
-  // Finish the Wasm script now and make it public to the debugger.
-  isolate->debug()->OnAfterCompile(script);
-  return module_object;
+  return WasmModuleObject::New(isolate, std::move(native_module), script,
+                               export_wrappers);
 }

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

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

@ -1347,11 +1341,10 @@ Handle<Script> WasmEngine::GetOrCreateScript(
    auto it = scripts.find(native_module.get());
    if (it != scripts.end()) {
      Handle<Script> weak_global_handle = it->second.handle();
-      if (weak_global_handle.is_null()) {
-        scripts.erase(it);
-      } else {
+      if (!weak_global_handle.is_null()) {
        return Handle<Script>::New(*weak_global_handle, isolate);
      }
+      scripts.erase(it);
    }
  }
  // Temporarily release the mutex to let the GC collect native modules.
@ -1362,8 +1355,14 @@ Handle<Script> WasmEngine::GetOrCreateScript(
    auto& scripts = isolates_[isolate]->scripts;
    DCHECK_EQ(0, scripts.count(native_module.get()));
    scripts.emplace(native_module.get(), WeakScriptHandle(script));
-    return script;
  }
+  // Make the new script available to debuggers.
+  {
+    TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.wasm.detailed"),
+                 "wasm.Debug.OnAfterCompile");
+    isolate->debug()->OnAfterCompile(script);
+  }
+  return script;
 }

 std::shared_ptr<OperationsBarrier>
--- a/src/wasm/wasm-engine.h
+++ b/src/wasm/wasm-engine.h
@ -331,6 +331,9 @@ class V8_EXPORT_PRIVATE WasmEngine {
  void FreeDeadCode(const DeadCodeMap&);
  void FreeDeadCodeLocked(const DeadCodeMap&);

+  // Get the script for a specific native module in a specific isolate. If it
+  // does not exist (or was garbage collected in the meantime), create a new
+  // script and make it public to debuggers.
  Handle<Script> GetOrCreateScript(Isolate*,
                                   const std::shared_ptr<NativeModule>&,
                                   Vector<const char> source_url);
--- a/src/wasm/wasm-serialization.cc
+++ b/src/wasm/wasm-serialization.cc
@ -873,9 +873,6 @@ MaybeHandle<WasmModuleObject> DeserializeNativeModule(
  Handle<WasmModuleObject> module_object = WasmModuleObject::New(
      isolate, shared_native_module, script, export_wrappers);

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

--- a/test/debugger/regress/regress-crbug-1032042.js
+++ b/test/debugger/regress/regress-crbug-1032042.js
@ -2,8 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.

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

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

--- a/test/inspector/debugger/wasm-scripts-expected.txt
+++ b/test/inspector/debugger/wasm-scripts-expected.txt
@ -2,34 +2,24 @@ Tests how wasm scripts are reported
 Check that each inspector gets a wasm script at module creation time.
 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
 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
-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
-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
-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
-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
-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
-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
-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
-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
-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
-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
-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
-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
-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
-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
-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
-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
-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
-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
-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
-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
-Session #1: Source for wasm://wasm/7b04570e:
-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
-Imports: []
-Exports: [main: function]
-Session #2: Source for wasm://wasm/7b04570e:
-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
-Imports: []
-Exports: [main: function]
+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
+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
+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
+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
+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
+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
+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
+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
+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
+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
+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
+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
+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
+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
+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
+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
+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
+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
 Session #1: Source for wasm://wasm/7b04570e:
 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
 Imports: []