[wasm][debug] Simplify debug name handling.

This moves the logic for the debug name heuristic, which derives names
for imported and exported entities from the relevant tables, into
wasm-debug.{cc,h} and stores these maps on the DebugInfoImpl rather than
on the WasmModule.

Drive-by-fix: Also use the import table based heuristic for function
names, just like we use it for everything else.

Bug: chromium:1164305
Change-Id: I8a21e0880c680079f63e6607b5b62c788049b9e1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2625870
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72061}

src/debug/debug-wasm-support.cc

@@ -78,6 +78,36 @@ Handle<String> GetNameOrDefault(Isolate* isolate,
   return isolate->factory()->InternalizeString(value.SubVector(0, len));
 }
 
+MaybeHandle<String> GetNameFromImportsAndExportsOrNull(
+    Isolate* isolate, Handle<WasmInstanceObject> instance,
+    wasm::ImportExportKindCode kind, uint32_t index) {
+  auto debug_info = instance->module_object().native_module()->GetDebugInfo();
+  wasm::ModuleWireBytes wire_bytes(
+      instance->module_object().native_module()->wire_bytes());
+
+  auto import_name_ref = debug_info->GetImportName(kind, index);
+  if (!import_name_ref.first.is_empty()) {
+    ScopedVector<char> name(import_name_ref.first.length() + 1 +
+                            import_name_ref.second.length());
+    auto name_begin = &name.first(), name_end = name_begin;
+    auto module_name = wire_bytes.GetNameOrNull(import_name_ref.first);
+    name_end = std::copy(module_name.begin(), module_name.end(), name_end);
+    *name_end++ = '.';
+    auto field_name = wire_bytes.GetNameOrNull(import_name_ref.second);
+    name_end = std::copy(field_name.begin(), field_name.end(), name_end);
+    return isolate->factory()->NewStringFromUtf8(
+        VectorOf(name_begin, name_end - name_begin));
+  }
+
+  auto export_name_ref = debug_info->GetExportName(kind, index);
+  if (!export_name_ref.is_empty()) {
+    auto name = wire_bytes.GetNameOrNull(export_name_ref);
+    return isolate->factory()->NewStringFromUtf8(name);
+  }
+
+  return {};
+}
+
 enum DebugProxyId {
   kFunctionsProxy,
   kGlobalsProxy,
@@ -340,18 +370,15 @@ struct FunctionsProxy : NamedDebugProxy<FunctionsProxy, kFunctionsProxy> {
   static Handle<String> GetName(Isolate* isolate,
                                 Handle<WasmInstanceObject> instance,
                                 uint32_t index) {
-    wasm::ModuleWireBytes wire_bytes(
-        instance->module_object().native_module()->wire_bytes());
-    auto* module = instance->module();
-    wasm::WireBytesRef name_ref =
-        module->lazily_generated_names.LookupFunctionName(
-            wire_bytes, index, VectorOf(module->export_table));
-    Vector<const char> name_vec = wire_bytes.GetNameOrNull(name_ref);
-    return GetNameOrDefault(
-        isolate,
-        name_vec.empty() ? MaybeHandle<String>()
-                         : isolate->factory()->NewStringFromUtf8(name_vec),
-        "$func", index);
+    Handle<WasmModuleObject> module_object(instance->module_object(), isolate);
+    MaybeHandle<String> name =
+        WasmModuleObject::GetFunctionNameOrNull(isolate, module_object, index);
+    if (name.is_null()) {
+      name = GetNameFromImportsAndExportsOrNull(
+          isolate, instance, wasm::ImportExportKindCode::kExternalFunction,
+          index);
+    }
+    return GetNameOrDefault(isolate, name, "$func", index);
   }
 };
 
@@ -376,7 +403,9 @@ struct GlobalsProxy : NamedDebugProxy<GlobalsProxy, kGlobalsProxy> {
                                 uint32_t index) {
     return GetNameOrDefault(
         isolate,
-        WasmInstanceObject::GetGlobalNameOrNull(isolate, instance, index),
+        GetNameFromImportsAndExportsOrNull(
+            isolate, instance, wasm::ImportExportKindCode::kExternalGlobal,
+            index),
         "$global", index);
   }
 };
@@ -400,7 +429,9 @@ struct MemoriesProxy : NamedDebugProxy<MemoriesProxy, kMemoriesProxy> {
                                 uint32_t index) {
     return GetNameOrDefault(
         isolate,
-        WasmInstanceObject::GetMemoryNameOrNull(isolate, instance, index),
+        GetNameFromImportsAndExportsOrNull(
+            isolate, instance, wasm::ImportExportKindCode::kExternalMemory,
+            index),
         "$memory", index);
   }
 };
@@ -424,7 +455,9 @@ struct TablesProxy : NamedDebugProxy<TablesProxy, kTablesProxy> {
                                 uint32_t index) {
     return GetNameOrDefault(
         isolate,
-        WasmInstanceObject::GetTableNameOrNull(isolate, instance, index),
+        GetNameFromImportsAndExportsOrNull(
+            isolate, instance, wasm::ImportExportKindCode::kExternalTable,
+            index),
         "$table", index);
   }
 };

src/wasm/module-decoder.cc

@@ -2412,37 +2412,6 @@ void DecodeFunctionNames(const byte* module_start, const byte* module_end,
   }
 }
 
-void GenerateNamesFromImportsAndExports(
-    ImportExportKindCode kind, const Vector<const WasmImport> import_table,
-    const Vector<const WasmExport> export_table,
-    std::unordered_map<uint32_t, std::pair<WireBytesRef, WireBytesRef>>*
-        names) {
-  DCHECK_NOT_NULL(names);
-  DCHECK(names->empty());
-  DCHECK(kind == kExternalGlobal || kind == kExternalMemory ||
-         kind == kExternalTable);
-
-  // Extract from import table.
-  for (const WasmImport& imp : import_table) {
-    if (imp.kind != kind) continue;
-    if (!imp.module_name.is_set() || !imp.field_name.is_set()) continue;
-    if (names->count(imp.index) == 0) {
-      names->insert(std::make_pair(
-          imp.index, std::make_pair(imp.module_name, imp.field_name)));
-    }
-  }
-
-  // Extract from export table.
-  for (const WasmExport& exp : export_table) {
-    if (exp.kind != kind) continue;
-    if (!exp.name.is_set()) continue;
-    if (names->count(exp.index) == 0) {
-      names->insert(
-          std::make_pair(exp.index, std::make_pair(WireBytesRef(), exp.name)));
-    }
-  }
-}
-
 LocalNames DecodeLocalNames(Vector<const uint8_t> module_bytes) {
   Decoder decoder(module_bytes);
   if (!FindNameSection(&decoder)) return LocalNames{{}};

src/wasm/module-decoder.h

@@ -179,13 +179,6 @@ void DecodeFunctionNames(const byte* module_start, const byte* module_end,
                          std::unordered_map<uint32_t, WireBytesRef>* names,
                          const Vector<const WasmExport> export_table);
 
-// Decode the global or memory names from import table and export table. Returns
-// the result as an unordered map.
-void GenerateNamesFromImportsAndExports(
-    ImportExportKindCode kind, const Vector<const WasmImport> import_table,
-    const Vector<const WasmExport> export_table,
-    std::unordered_map<uint32_t, std::pair<WireBytesRef, WireBytesRef>>* names);
-
 // Decode the local names assignment from the name section.
 // The result will be empty if no name section is present. On encountering an
 // error in the name section, returns all information decoded up to the first

src/wasm/wasm-debug.cc

@@ -33,6 +33,8 @@ namespace wasm {
 
 namespace {
 
+using ImportExportKey = std::pair<ImportExportKindCode, uint32_t>;
+
 enum ReturnLocation { kAfterBreakpoint, kAfterWasmCall };
 
 Address FindNewPC(WasmFrame* frame, WasmCode* wasm_code, int byte_offset,
@@ -151,6 +153,39 @@ class DebugInfoImpl {
     return module->functions[scope.code->index()];
   }
 
+  WireBytesRef GetExportName(ImportExportKindCode kind, uint32_t index) {
+    base::MutexGuard guard(&mutex_);
+    if (!export_names_) {
+      export_names_ =
+          std::make_unique<std::map<ImportExportKey, WireBytesRef>>();
+      for (auto exp : native_module_->module()->export_table) {
+        auto exp_key = std::make_pair(exp.kind, exp.index);
+        if (export_names_->find(exp_key) != export_names_->end()) continue;
+        export_names_->insert(std::make_pair(exp_key, exp.name));
+      }
+    }
+    auto it = export_names_->find(std::make_pair(kind, index));
+    if (it != export_names_->end()) return it->second;
+    return {};
+  }
+
+  std::pair<WireBytesRef, WireBytesRef> GetImportName(ImportExportKindCode kind,
+                                                      uint32_t index) {
+    base::MutexGuard guard(&mutex_);
+    if (!import_names_) {
+      import_names_ = std::make_unique<
+          std::map<ImportExportKey, std::pair<WireBytesRef, WireBytesRef>>>();
+      for (auto imp : native_module_->module()->import_table) {
+        import_names_->insert(
+            std::make_pair(std::make_pair(imp.kind, imp.index),
+                           std::make_pair(imp.module_name, imp.field_name)));
+      }
+    }
+    auto it = import_names_->find(std::make_pair(kind, index));
+    if (it != import_names_->end()) return it->second;
+    return {};
+  }
+
   WireBytesRef GetLocalName(int func_index, int local_index) {
     base::MutexGuard guard(&mutex_);
     if (!local_names_) {
@@ -621,6 +656,14 @@ class DebugInfoImpl {
   // {mutex_} protects all fields below.
   mutable base::Mutex mutex_;
 
+  // Names of exports, lazily derived from the exports table.
+  std::unique_ptr<std::map<ImportExportKey, wasm::WireBytesRef>> export_names_;
+
+  // Names of imports, lazily derived from the imports table.
+  std::unique_ptr<std::map<ImportExportKey,
+                           std::pair<wasm::WireBytesRef, wasm::WireBytesRef>>>
+      import_names_;
+
   // Names of locals, lazily decoded from the wire bytes.
   std::unique_ptr<LocalNames> local_names_;
 
@@ -651,6 +694,16 @@ const wasm::WasmFunction& DebugInfo::GetFunctionAtAddress(Address pc) {
   return impl_->GetFunctionAtAddress(pc);
 }
 
+WireBytesRef DebugInfo::GetExportName(ImportExportKindCode code,
+                                      uint32_t index) {
+  return impl_->GetExportName(code, index);
+}
+
+std::pair<WireBytesRef, WireBytesRef> DebugInfo::GetImportName(
+    ImportExportKindCode code, uint32_t index) {
+  return impl_->GetImportName(code, index);
+}
+
 WireBytesRef DebugInfo::GetLocalName(int func_index, int local_index) {
   return impl_->GetLocalName(func_index, local_index);
 }

src/wasm/wasm-debug.h

@@ -154,6 +154,17 @@ class V8_EXPORT_PRIVATE DebugInfo {
   WasmValue GetStackValue(int index, Address pc, Address fp,
                           Address debug_break_fp);
 
+  // Returns the name of the entity (with the given |index| and |kind|) derived
+  // from the exports table. If the entity is not exported, an empty reference
+  // will be returned instead.
+  WireBytesRef GetExportName(ImportExportKindCode kind, uint32_t index);
+
+  // Returns the module and field name of the entity (with the given |index|
+  // and |kind|) derived from the imports table. If the entity is not imported,
+  // a pair of empty references will be returned instead.
+  std::pair<WireBytesRef, WireBytesRef> GetImportName(ImportExportKindCode kind,
+                                                      uint32_t index);
+
   WireBytesRef GetLocalName(int func_index, int local_index);
 
   void SetBreakpoint(int func_index, int offset, Isolate* current_isolate);

src/wasm/wasm-module.cc

@@ -46,29 +46,6 @@ WireBytesRef LazilyGeneratedNames::LookupFunctionName(
   return it->second;
 }
 
-std::pair<WireBytesRef, WireBytesRef>
-LazilyGeneratedNames::LookupNameFromImportsAndExports(
-    ImportExportKindCode kind, uint32_t index,
-    Vector<const WasmImport> import_table,
-    Vector<const WasmExport> export_table) const {
-  base::MutexGuard lock(&mutex_);
-  DCHECK(kind == kExternalGlobal || kind == kExternalMemory ||
-         kind == kExternalTable);
-  auto& names = kind == kExternalGlobal
-                    ? global_names_
-                    : kind == kExternalMemory ? memory_names_ : table_names_;
-  if (!names) {
-    names.reset(
-        new std::unordered_map<uint32_t,
-                               std::pair<WireBytesRef, WireBytesRef>>());
-    GenerateNamesFromImportsAndExports(kind, import_table, export_table,
-                                       names.get());
-  }
-  auto it = names->find(index);
-  if (it == names->end()) return {};
-  return it->second;
-}
-
 // static
 int MaxNumExportWrappers(const WasmModule* module) {
   // For each signature there may exist a wrapper, both for imported and

src/wasm/wasm-module.h

@@ -187,30 +187,15 @@ class V8_EXPORT_PRIVATE LazilyGeneratedNames {
                                   uint32_t function_index,
                                   Vector<const WasmExport> export_table) const;
 
-  // For memory and global.
-  std::pair<WireBytesRef, WireBytesRef> LookupNameFromImportsAndExports(
-      ImportExportKindCode kind, uint32_t index,
-      const Vector<const WasmImport> import_table,
-      const Vector<const WasmExport> export_table) const;
-
   void AddForTesting(int function_index, WireBytesRef name);
 
  private:
-  // {function_names_}, {global_names_}, {memory_names_} and {table_names_} are
-  // populated lazily after decoding, and therefore need a mutex to protect
-  // concurrent modifications from multiple {WasmModuleObject}.
+  // {function_names_} are populated lazily after decoding, and
+  // therefore need a mutex to protect concurrent modifications
+  // from multiple {WasmModuleObject}.
   mutable base::Mutex mutex_;
   mutable std::unique_ptr<std::unordered_map<uint32_t, WireBytesRef>>
       function_names_;
-  mutable std::unique_ptr<
-      std::unordered_map<uint32_t, std::pair<WireBytesRef, WireBytesRef>>>
-      global_names_;
-  mutable std::unique_ptr<
-      std::unordered_map<uint32_t, std::pair<WireBytesRef, WireBytesRef>>>
-      memory_names_;
-  mutable std::unique_ptr<
-      std::unordered_map<uint32_t, std::pair<WireBytesRef, WireBytesRef>>>
-      table_names_;
 };
 
 class V8_EXPORT_PRIVATE AsmJsOffsetInformation {

src/wasm/wasm-objects.cc

@@ -1571,64 +1571,6 @@ WasmInstanceObject::GetGlobalBufferAndIndex(Handle<WasmInstanceObject> instance,
   return {handle(instance->tagged_globals_buffer(), isolate), global.offset};
 }
 
-// static
-MaybeHandle<String> WasmInstanceObject::GetGlobalNameOrNull(
-    Isolate* isolate, Handle<WasmInstanceObject> instance,
-    uint32_t global_index) {
-  return WasmInstanceObject::GetNameFromImportsAndExportsOrNull(
-      isolate, instance, wasm::ImportExportKindCode::kExternalGlobal,
-      global_index);
-}
-
-// static
-MaybeHandle<String> WasmInstanceObject::GetMemoryNameOrNull(
-    Isolate* isolate, Handle<WasmInstanceObject> instance,
-    uint32_t memory_index) {
-  return WasmInstanceObject::GetNameFromImportsAndExportsOrNull(
-      isolate, instance, wasm::ImportExportKindCode::kExternalMemory,
-      memory_index);
-}
-
-// static
-MaybeHandle<String> WasmInstanceObject::GetTableNameOrNull(
-    Isolate* isolate, Handle<WasmInstanceObject> instance,
-    uint32_t table_index) {
-  return WasmInstanceObject::GetNameFromImportsAndExportsOrNull(
-      isolate, instance, wasm::ImportExportKindCode::kExternalTable,
-      table_index);
-}
-
-// static
-MaybeHandle<String> WasmInstanceObject::GetNameFromImportsAndExportsOrNull(
-    Isolate* isolate, Handle<WasmInstanceObject> instance,
-    wasm::ImportExportKindCode kind, uint32_t index) {
-  DCHECK(kind == wasm::ImportExportKindCode::kExternalGlobal ||
-         kind == wasm::ImportExportKindCode::kExternalMemory ||
-         kind == wasm::ImportExportKindCode::kExternalTable);
-  wasm::ModuleWireBytes wire_bytes(
-      instance->module_object().native_module()->wire_bytes());
-
-  // This is pair of <module_name, field_name>.
-  // If field_name is not set then we don't generate a name. Else if module_name
-  // is set then it is an imported one. Otherwise it is an exported one.
-  std::pair<wasm::WireBytesRef, wasm::WireBytesRef> name_ref =
-      instance->module()
-          ->lazily_generated_names.LookupNameFromImportsAndExports(
-              kind, index, VectorOf(instance->module()->import_table),
-              VectorOf(instance->module()->export_table));
-  if (!name_ref.second.is_set()) return {};
-  Vector<const char> field_name = wire_bytes.GetNameOrNull(name_ref.second);
-  if (!name_ref.first.is_set()) {
-    return isolate->factory()->NewStringFromUtf8(VectorOf(field_name));
-  }
-  Vector<const char> module_name = wire_bytes.GetNameOrNull(name_ref.first);
-  std::string full_name;
-  full_name.append(module_name.begin(), module_name.end());
-  full_name.append(".");
-  full_name.append(field_name.begin(), field_name.end());
-  return isolate->factory()->NewStringFromUtf8(VectorOf(full_name));
-}
-
 // static
 wasm::WasmValue WasmInstanceObject::GetGlobalValue(
     Handle<WasmInstanceObject> instance, const wasm::WasmGlobal& global) {

src/wasm/wasm-objects.h

@@ -573,29 +573,9 @@ class V8_EXPORT_PRIVATE WasmInstanceObject : public JSObject {
   static wasm::WasmValue GetGlobalValue(Handle<WasmInstanceObject>,
                                         const wasm::WasmGlobal&);
 
-  // Get the name of a global in the given instance by index.
-  static MaybeHandle<String> GetGlobalNameOrNull(Isolate*,
-                                                 Handle<WasmInstanceObject>,
-                                                 uint32_t global_index);
-
-  // Get the name of a memory in the given instance by index.
-  static MaybeHandle<String> GetMemoryNameOrNull(Isolate*,
-                                                 Handle<WasmInstanceObject>,
-                                                 uint32_t memory_index);
-
-  // Get the name of a table in the given instance by index.
-  static MaybeHandle<String> GetTableNameOrNull(Isolate*,
-                                                Handle<WasmInstanceObject>,
-                                                uint32_t table_index);
-
   OBJECT_CONSTRUCTORS(WasmInstanceObject, JSObject);
 
  private:
-  // Get the name in the given instance by index and kind.
-  static MaybeHandle<String> GetNameFromImportsAndExportsOrNull(
-      Isolate*, Handle<WasmInstanceObject>, wasm::ImportExportKindCode kind,
-      uint32_t index);
-
   static void InitDataSegmentArrays(Handle<WasmInstanceObject>,
                                     Handle<WasmModuleObject>);
   static void InitElemSegmentArrays(Handle<WasmInstanceObject>,

test/inspector/debugger/wasm-evaluate-on-call-frame-expected.txt

@@ -54,17 +54,19 @@ Call main.
 Debugger paused in main.
 > functions = Functions
 > typeof functions = "object"
-> Object.keys(functions) = Array(3)
+> Object.keys(functions) = Array(4)
 > functions[0] = function 0() { [native code] }
 > functions[1] = function 1() { [native code] }
 > functions[2] = function 2() { [native code] }
 > functions[3] = function 3() { [native code] }
-> functions["$main"] = function 0() { [native code] }
-> $main = function 0() { [native code] }
-> functions["$func1"] = function 1() { [native code] }
-> $func1 = function 1() { [native code] }
-> functions["$func3"] = function 3() { [native code] }
-> $func3 = function 3() { [native code] }
+> functions[4] = function 4() { [native code] }
+> functions["$foo.bar"] = function 0() { [native code] }
+> functions["$main"] = function 1() { [native code] }
+> $main = function 1() { [native code] }
+> functions["$func2"] = function 2() { [native code] }
+> $func2 = function 2() { [native code] }
+> functions["$func4"] = function 4() { [native code] }
+> $func4 = function 4() { [native code] }
 
 Running test: testLocals
 Compile module.

test/inspector/debugger/wasm-evaluate-on-call-frame.js

@@ -21,9 +21,10 @@ async function compileModule(builder) {
   return [result.result, params.scriptId];
 }
 
-async function instantiateModule({objectId}) {
+async function instantiateModule({objectId}, importObject) {
   const {result: {result}} = await Protocol.Runtime.callFunctionOn({
-    functionDeclaration: 'function() { return new WebAssembly.Instance(this); }',
+    arguments: importObject ? [importObject] : [],
+    functionDeclaration: 'function(importObject) { return new WebAssembly.Instance(this, importObject); }',
     objectId
   });
   return result;
@@ -43,7 +44,7 @@ async function dumpOnCallFrame(callFrameId, expression) {
 async function dumpKeysOnCallFrame(callFrameId, object, keys) {
   for (const key of keys) {
     await dumpOnCallFrame(callFrameId, `${object}[${JSON.stringify(key)}]`);
-    if (typeof key === 'string') {
+    if (typeof key === 'string' && key.indexOf('.') < 0) {
       await dumpOnCallFrame(callFrameId, `${key}`);
     }
   }
@@ -150,23 +151,24 @@ InspectorTest.runAsyncTestSuite([
 
   async function testFunctions() {
     const builder = new WasmModuleBuilder();
+    builder.addImport('foo', 'bar', kSig_v_v);
     const main = builder.addFunction('main', kSig_i_v)
                         .addBody([
                           kExprI32Const, 0,
                         ]).exportFunc();
-    builder.addFunction('func1', kSig_i_v)
+    builder.addFunction('func2', kSig_i_v)
            .addBody([
              kExprI32Const, 1,
            ]);
     builder.addFunction(undefined, kSig_i_v)
            .addBody([
              kExprI32Const, 2,
-           ]).exportAs('func1');
+           ]).exportAs('func2');
     builder.addFunction(undefined, kSig_i_v)
            .addBody([
              kExprI32Const, 3,
            ]);
-    const KEYS = [0, 1, 2, 3, '$main', '$func1', '$func3'];
+    const KEYS = [0, 1, 2, 3, 4, '$foo.bar', '$main', '$func2', '$func4'];
 
     InspectorTest.log('Compile module.');
     const [module, scriptId] = await compileModule(builder);
@@ -177,7 +179,10 @@ InspectorTest.runAsyncTestSuite([
     });
 
     InspectorTest.log('Instantiate module.');
-    const instance = await instantiateModule(module);
+    const {result: { result: importObject }} = await Protocol.Runtime.evaluate({
+      expression: `({foo: {bar() { }}})`
+    });
+    const instance = await instantiateModule(module, importObject);
 
     InspectorTest.log('Call main.');
     const callMainPromise = Protocol.Runtime.callFunctionOn({