[wasm][diagnostics] Support WasmCode in gdb JIT integration

- Add new enum WASM_CODE to JitCodeEvent::CodeType
- Use AddressRegion instead of AddressRange (remove the latter)
- Change CodeDescription constructor to take an AddressRegion,
  both JIT_CODE and WASM_CODE use this
- Add a simple mjsunit test that sets --gdbjit to check that
  we don't crash.
- Add a api test for adding WASM_CODE

Bug: v8:11908
Change-Id: I6e87fadc2df67978144d78caf9800c3982bc3705
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3067754Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#76271}

include/v8.h

@@ -7848,13 +7848,13 @@ struct JitCodeEvent {
   // statement, and is used to indicate possible break locations.
   enum PositionType { POSITION, STATEMENT_POSITION };
 
-  // There are two different kinds of JitCodeEvents, one for JIT code generated
-  // by the optimizing compiler, and one for byte code generated for the
-  // interpreter.  For JIT_CODE events, the |code_start| member of the event
-  // points to the beginning of jitted assembly code, while for BYTE_CODE
-  // events, |code_start| points to the first bytecode of the interpreted
-  // function.
-  enum CodeType { BYTE_CODE, JIT_CODE };
+  // There are three different kinds of CodeType, one for JIT code generated
+  // by the optimizing compiler, one for byte code generated for the
+  // interpreter, and one for code generated from Wasm. For JIT_CODE and
+  // WASM_CODE, |code_start| points to the beginning of jitted assembly code,
+  // while for BYTE_CODE events, |code_start| points to the first bytecode of
+  // the interpreted function.
+  enum CodeType { BYTE_CODE, JIT_CODE, WASM_CODE };
 
   // Type of event.
   EventType type;

src/diagnostics/gdb-jit.cc

@@ -10,6 +10,7 @@
 
 #include "include/v8.h"
 #include "src/api/api-inl.h"
+#include "src/base/address-region.h"
 #include "src/base/bits.h"
 #include "src/base/hashmap.h"
 #include "src/base/platform/platform.h"
@@ -896,17 +897,20 @@ class CodeDescription {
   };
 #endif
 
-  CodeDescription(const char* name, Code code, SharedFunctionInfo shared,
-                  LineInfo* lineinfo)
-      : name_(name), code_(code), shared_info_(shared), lineinfo_(lineinfo) {}
+  CodeDescription(const char* name, base::AddressRegion region,
+                  SharedFunctionInfo shared, LineInfo* lineinfo,
+                  bool is_function)
+      : name_(name),
+        shared_info_(shared),
+        lineinfo_(lineinfo),
+        is_function_(is_function),
+        code_region_(region) {}
 
   const char* name() const { return name_; }
 
   LineInfo* lineinfo() const { return lineinfo_; }
 
-  bool is_function() const {
-    return CodeKindIsOptimizedJSFunction(code_.kind());
-  }
+  bool is_function() const { return is_function_; }
 
   bool has_scope_info() const { return !shared_info_.is_null(); }
 
@@ -915,15 +919,11 @@ class CodeDescription {
     return shared_info_.scope_info();
   }
 
-  uintptr_t CodeStart() const {
-    return static_cast<uintptr_t>(code_.InstructionStart());
-  }
+  uintptr_t CodeStart() const { return code_region_.begin(); }
 
-  uintptr_t CodeEnd() const {
-    return static_cast<uintptr_t>(code_.InstructionEnd());
-  }
+  uintptr_t CodeEnd() const { return code_region_.end(); }
 
-  uintptr_t CodeSize() const { return CodeEnd() - CodeStart(); }
+  uintptr_t CodeSize() const { return code_region_.size(); }
 
   bool has_script() {
     return !shared_info_.is_null() && shared_info_.script().IsScript();
@@ -933,6 +933,8 @@ class CodeDescription {
 
   bool IsLineInfoAvailable() { return lineinfo_ != nullptr; }
 
+  base::AddressRegion region() { return code_region_; }
+
 #if V8_TARGET_ARCH_X64
   uintptr_t GetStackStateStartAddress(StackState state) const {
     DCHECK(state < STACK_STATE_MAX);
@@ -965,9 +967,10 @@ class CodeDescription {
 
  private:
   const char* name_;
-  Code code_;
   SharedFunctionInfo shared_info_;
   LineInfo* lineinfo_;
+  bool is_function_;
+  base::AddressRegion code_region_;
 #if V8_TARGET_ARCH_X64
   uintptr_t stack_state_start_addresses_[STACK_STATE_MAX];
 #endif
@@ -1816,26 +1819,17 @@ static JITCodeEntry* CreateELFObject(CodeDescription* desc, Isolate* isolate) {
   return CreateCodeEntry(reinterpret_cast<Address>(w.buffer()), w.position());
 }
 
-struct AddressRange {
-  Address start;
-  Address end;
-};
-
-struct AddressRangeLess {
-  bool operator()(const AddressRange& a, const AddressRange& b) const {
-    if (a.start == b.start) return a.end < b.end;
-    return a.start < b.start;
+// Like base::AddressRegion::StartAddressLess but also compares |end| when
+// |begin| is equal.
+struct AddressRegionLess {
+  bool operator()(const base::AddressRegion& a,
+                  const base::AddressRegion& b) const {
+    if (a.begin() == b.begin()) return a.end() < b.end();
+    return a.begin() < b.begin();
   }
 };
 
-struct CodeMapConfig {
-  using Key = AddressRange;
-  using Value = JITCodeEntry*;
-  using Less = AddressRangeLess;
-};
-
-using CodeMap =
-    std::map<CodeMapConfig::Key, CodeMapConfig::Value, CodeMapConfig::Less>;
+using CodeMap = std::map<base::AddressRegion, JITCodeEntry*, AddressRegionLess>;
 
 static CodeMap* GetCodeMap() {
   // TODO(jgruber): Don't leak.
@@ -1909,36 +1903,37 @@ static void AddUnwindInfo(CodeDescription* desc) {
 
 static base::LazyMutex mutex = LAZY_MUTEX_INITIALIZER;
 
-// Remove entries from the map that intersect the given address range,
+// Remove entries from the map that intersect the given address region,
 // and deregister them from GDB.
-static void RemoveJITCodeEntries(CodeMap* map, const AddressRange& range) {
-  DCHECK(range.start < range.end);
+static void RemoveJITCodeEntries(CodeMap* map,
+                                 const base::AddressRegion region) {
+  DCHECK_LT(region.begin(), region.end());
 
   if (map->empty()) return;
 
   // Find the first overlapping entry.
 
-  // If successful, points to the first element not less than `range`. The
+  // If successful, points to the first element not less than `region`. The
   // returned iterator has the key in `first` and the value in `second`.
-  auto it = map->lower_bound(range);
+  auto it = map->lower_bound(region);
   auto start_it = it;
 
   if (it == map->end()) {
     start_it = map->begin();
   } else if (it != map->begin()) {
     for (--it; it != map->begin(); --it) {
-      if ((*it).first.end <= range.start) break;
+      if ((*it).first.end() <= region.begin()) break;
       start_it = it;
     }
   }
 
-  DCHECK(start_it != map->end());
+  DCHECK_NE(start_it, map->end());
 
-  // Find the first non-overlapping entry after `range`.
+  // Find the first non-overlapping entry after `region`.
 
-  const auto end_it = map->lower_bound({range.end, 0});
+  const auto end_it = map->lower_bound({region.end(), 0});
 
-  // Evict intersecting ranges.
+  // Evict intersecting regions.
 
   if (std::distance(start_it, end_it) < 1) return;  // No overlapping entries.
 
@@ -1952,7 +1947,7 @@ static void RemoveJITCodeEntries(CodeMap* map, const AddressRange& range) {
 }
 
 // Insert the entry into the map and register it with GDB.
-static void AddJITCodeEntry(CodeMap* map, const AddressRange& range,
+static void AddJITCodeEntry(CodeMap* map, const base::AddressRegion region,
                             JITCodeEntry* entry, bool dump_if_enabled,
                             const char* name_hint) {
 #if defined(DEBUG) && !V8_OS_WIN
@@ -1969,24 +1964,21 @@ static void AddJITCodeEntry(CodeMap* map, const AddressRange& range,
   }
 #endif
 
-  auto result = map->emplace(range, entry);
+  auto result = map->emplace(region, entry);
   DCHECK(result.second);  // Insertion happened.
   USE(result);
 
   RegisterCodeEntry(entry);
 }
 
-static void AddCode(const char* name, Code code, SharedFunctionInfo shared,
-                    LineInfo* lineinfo) {
+static void AddCode(const char* name, base::AddressRegion region,
+                    SharedFunctionInfo shared, LineInfo* lineinfo,
+                    Isolate* isolate, bool is_function) {
   DisallowGarbageCollection no_gc;
+  CodeDescription code_desc(name, region, shared, lineinfo, is_function);
 
   CodeMap* code_map = GetCodeMap();
-  AddressRange range;
-  range.start = code.address();
-  range.end = code.address() + code.CodeSize();
-  RemoveJITCodeEntries(code_map, range);
-
-  CodeDescription code_desc(name, code, shared, lineinfo);
+  RemoveJITCodeEntries(code_map, region);
 
   if (!FLAG_gdbjit_full && !code_desc.IsLineInfoAvailable()) {
     delete lineinfo;
@@ -1994,7 +1986,6 @@ static void AddCode(const char* name, Code code, SharedFunctionInfo shared,
   }
 
   AddUnwindInfo(&code_desc);
-  Isolate* isolate = code.GetIsolate();
   JITCodeEntry* entry = CreateELFObject(&code_desc, isolate);
 
   delete lineinfo;
@@ -2010,25 +2001,40 @@ static void AddCode(const char* name, Code code, SharedFunctionInfo shared,
       should_dump = (name_hint != nullptr);
     }
   }
-  AddJITCodeEntry(code_map, range, entry, should_dump, name_hint);
+  AddJITCodeEntry(code_map, region, entry, should_dump, name_hint);
 }
 
 void EventHandler(const v8::JitCodeEvent* event) {
   if (!FLAG_gdbjit) return;
-  if (event->code_type != v8::JitCodeEvent::JIT_CODE) return;
+  if ((event->code_type != v8::JitCodeEvent::JIT_CODE) &&
+      (event->code_type != v8::JitCodeEvent::WASM_CODE)) {
+    return;
+  }
   base::MutexGuard lock_guard(mutex.Pointer());
   switch (event->type) {
     case v8::JitCodeEvent::CODE_ADDED: {
       Address addr = reinterpret_cast<Address>(event->code_start);
-      Isolate* isolate = reinterpret_cast<Isolate*>(event->isolate);
-      Code code = isolate->heap()->GcSafeFindCodeForInnerPointer(addr);
       LineInfo* lineinfo = GetLineInfo(addr);
       std::string event_name(event->name.str, event->name.len);
       // It's called UnboundScript in the API but it's a SharedFunctionInfo.
       SharedFunctionInfo shared = event->script.IsEmpty()
                                       ? SharedFunctionInfo()
                                       : *Utils::OpenHandle(*event->script);
-      AddCode(event_name.c_str(), code, shared, lineinfo);
+      Isolate* isolate = reinterpret_cast<Isolate*>(event->isolate);
+      bool is_function = false;
+      // TODO(zhin): See if we can use event->code_type to determine
+      // is_function, the difference currently is that JIT_CODE is SparkPlug,
+      // TurboProp, TurboFan, whereas CodeKindIsOptimizedJSFunction is only
+      // TurboProp and TurboFan. is_function is used for AddUnwindInfo, and the
+      // prologue that SP generates probably matches that of TP/TF, so we can
+      // use event->code_type here instead of finding the Code.
+      // TODO(zhin): Rename is_function to be more accurate.
+      if (event->code_type == v8::JitCodeEvent::JIT_CODE) {
+        Code code = isolate->heap()->GcSafeFindCodeForInnerPointer(addr);
+        is_function = CodeKindIsOptimizedJSFunction(code.kind());
+      }
+      AddCode(event_name.c_str(), {addr, event->code_len}, shared, lineinfo,
+              isolate, is_function);
       break;
     }
     case v8::JitCodeEvent::CODE_MOVED:

src/logging/log.cc

@@ -730,7 +730,7 @@ void JitLogger::LogRecordedBuffer(const wasm::WasmCode* code, const char* name,
                                   int length) {
   JitCodeEvent event = {};
   event.type = JitCodeEvent::CODE_ADDED;
-  event.code_type = JitCodeEvent::JIT_CODE;
+  event.code_type = JitCodeEvent::WASM_CODE;
   event.code_start = code->instructions().begin();
   event.code_len = code->instructions().length();
   event.name.str = name;
@@ -1558,12 +1558,14 @@ void Logger::CodeLinePosInfoRecordEvent(Address code_start,
   CodeLinePosEvent(*jit_logger_, code_start, iter, code_type);
 }
 
-void Logger::CodeLinePosInfoRecordEvent(
+#if V8_ENABLE_WEBASSEMBLY
+void Logger::WasmCodeLinePosInfoRecordEvent(
     Address code_start, base::Vector<const byte> source_position_table) {
   if (!jit_logger_) return;
   SourcePositionTableIterator iter(source_position_table);
-  CodeLinePosEvent(*jit_logger_, code_start, iter, JitCodeEvent::JIT_CODE);
+  CodeLinePosEvent(*jit_logger_, code_start, iter, JitCodeEvent::WASM_CODE);
 }
+#endif  // V8_ENABLE_WEBASSEMBLY
 
 void Logger::CodeNameEvent(Address addr, int pos, const char* code_name) {
   if (code_name == nullptr) return;  // Not a code object.

src/logging/log.h

@@ -241,8 +241,10 @@ class Logger : public CodeEventListener {
   void CodeLinePosInfoRecordEvent(Address code_start,
                                   ByteArray source_position_table,
                                   JitCodeEvent::CodeType code_type);
-  void CodeLinePosInfoRecordEvent(
+#if V8_ENABLE_WEBASSEMBLY
+  void WasmCodeLinePosInfoRecordEvent(
       Address code_start, base::Vector<const byte> source_position_table);
+#endif  // V8_ENABLE_WEBASSEMBLY
 
   void CodeNameEvent(Address addr, int pos, const char* code_name);
 

src/wasm/wasm-code-manager.cc

@@ -267,14 +267,17 @@ void WasmCode::LogCode(Isolate* isolate, const char* source_url,
                  "wasm-function[%d]", index()));
     name = base::VectorOf(name_buffer);
   }
-  int code_offset = module->functions[index_].code.offset();
-  PROFILE(isolate, CodeCreateEvent(CodeEventListener::FUNCTION_TAG, this, name,
-                                   source_url, code_offset, script_id));
 
+  // Record source positions before adding code, otherwise when code is added,
+  // there are no source positions to associate with the added code.
   if (!source_positions().empty()) {
-    LOG_CODE_EVENT(isolate, CodeLinePosInfoRecordEvent(instruction_start(),
-                                                       source_positions()));
+    LOG_CODE_EVENT(isolate, WasmCodeLinePosInfoRecordEvent(instruction_start(),
+                                                           source_positions()));
   }
+
+  int code_offset = module->functions[index_].code.offset();
+  PROFILE(isolate, CodeCreateEvent(CodeEventListener::FUNCTION_TAG, this, name,
+                                   source_url, code_offset, script_id));
 }
 
 void WasmCode::Validate() const {

test/cctest/cctest.status

@@ -580,6 +580,7 @@
   'test-api/TurboAsmDisablesDetach': [SKIP],
   'test-api/WasmI32AtomicWaitCallback': [SKIP],
   'test-api/WasmI64AtomicWaitCallback': [SKIP],
+  'test-api/WasmSetJitCodeEventHandler': [SKIP],
   'test-api-wasm/WasmStreaming*': [SKIP],
   'test-backing-store/Run_WasmModule_Buffer_Externalized_Regression_UseAfterFree': [SKIP],
   'test-c-wasm-entry/*': [SKIP],

test/cctest/test-api.cc

@@ -76,6 +76,7 @@
 
 #if V8_ENABLE_WEBASSEMBLY
 #include "test/cctest/wasm/wasm-run-utils.h"
+#include "test/common/wasm/test-signatures.h"
 #include "test/common/wasm/wasm-macro-gen.h"
 #endif  // V8_ENABLE_WEBASSEMBLY
 
@@ -13738,16 +13739,14 @@ static v8::base::HashMap* jitcode_line_info = nullptr;
 static int saw_bar = 0;
 static int move_events = 0;
 
-
 static bool FunctionNameIs(const char* expected,
                            const v8::JitCodeEvent* event) {
   // Log lines for functions are of the general form:
   // "LazyCompile:<type><function_name>" or Function:<type><function_name>,
   // where the type is one of "*", "~" or "".
-  static const char* kPreamble;
-  if (!i::FLAG_lazy) {
-    kPreamble = "Function:";
-  } else {
+  static const char* kPreamble = "Function:";
+  if (i::FLAG_lazy &&
+      event->code_type != v8::JitCodeEvent::CodeType::WASM_CODE) {
     kPreamble = "LazyCompile:";
   }
   static size_t kPreambleLen = strlen(kPreamble);
@@ -13780,7 +13779,6 @@ static bool FunctionNameIs(const char* expected,
   return strncmp(tail, expected, expected_len) == 0;
 }
 
-
 static void event_handler(const v8::JitCodeEvent* event) {
   CHECK_NOT_NULL(event);
   CHECK_NOT_NULL(code_map);
@@ -13873,7 +13871,6 @@ static void event_handler(const v8::JitCodeEvent* event) {
   }
 }
 
-
 UNINITIALIZED_TEST(SetJitCodeEventHandler) {
   i::FLAG_stress_compaction = true;
   i::FLAG_incremental_marking = false;
@@ -13998,6 +13995,77 @@ UNINITIALIZED_TEST(SetJitCodeEventHandler) {
   isolate->Dispose();
 }
 
+#if V8_ENABLE_WEBASSEMBLY
+static bool saw_wasm_main = false;
+static void wasm_event_handler(const v8::JitCodeEvent* event) {
+  switch (event->type) {
+    case v8::JitCodeEvent::CODE_ADDED: {
+      if (FunctionNameIs("main-0-turbofan", event)) {
+        saw_wasm_main = true;
+        // Make sure main function has line info.
+        auto* entry = jitcode_line_info->Lookup(
+            event->code_start, i::ComputePointerHash(event->code_start));
+        CHECK_NOT_NULL(entry);
+      }
+      break;
+    }
+    case v8::JitCodeEvent::CODE_END_LINE_INFO_RECORDING: {
+      jitcode_line_info->LookupOrInsert(
+          event->code_start, i::ComputePointerHash(event->code_start));
+      break;
+    }
+    case v8::JitCodeEvent::CODE_ADD_LINE_POS_INFO: {
+      break;
+    }
+    default: {
+      // Ignore all other events;
+    }
+  }
+}
+
+namespace v8 {
+namespace internal {
+namespace wasm {
+TEST(WasmSetJitCodeEventHandler) {
+  v8::base::HashMap code;
+  code_map = &code;
+
+  v8::base::HashMap lineinfo;
+  jitcode_line_info = &lineinfo;
+
+  WasmRunner<int32_t, int32_t, int32_t> r(TestExecutionTier::kTurbofan);
+  i::Isolate* isolate = r.main_isolate();
+
+  v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate);
+  v8_isolate->SetJitCodeEventHandler(v8::kJitCodeEventDefault,
+                                     wasm_event_handler);
+
+  TestSignatures sigs;
+  auto& f = r.NewFunction(sigs.i_i(), "f");
+  BUILD(f, WASM_I32_ADD(WASM_LOCAL_GET(0), WASM_LOCAL_GET(0)));
+
+  LocalContext env;
+
+  BUILD(r,
+        WASM_I32_ADD(WASM_LOCAL_GET(0), WASM_CALL_FUNCTION(f.function_index(),
+                                                           WASM_LOCAL_GET(1))));
+
+  Handle<JSFunction> func = r.builder().WrapCode(0);
+  CHECK(env->Global()
+            ->Set(env.local(), v8_str("func"), v8::Utils::ToLocal(func))
+            .FromJust());
+  const char* script = R"(
+    func(1, 2);
+  )";
+  CompileRun(script);
+  CHECK(saw_wasm_main);
+  saw_wasm_main = false;
+}
+}  // namespace wasm
+}  // namespace internal
+}  // namespace v8
+#endif  // V8_ENABLE_WEBASSEMBLY
+
 TEST(ExternalAllocatedMemory) {
   v8::Isolate* isolate = CcTest::isolate();
   v8::HandleScope outer(isolate);

test/mjsunit/wasm/gdbjit.js

+// Copyright 2021 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: --gdbjit
+
+d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
+
+// A simple test to ensure that passing the --gdbjit flag doesn't crash.
+(function testGdbJitFlag() {
+  const builder = new WasmModuleBuilder();
+  builder.addFunction('i32_add', kSig_i_ii)
+      .addBody([kExprLocalGet, 0, kExprLocalGet, 1, kExprI32Add])
+      .exportFunc();
+
+  const module = new WebAssembly.Module(builder.toBuffer());
+  const instance = new WebAssembly.Instance(module);
+
+  assertEquals(instance.exports.i32_add(1, 2), 3);
+}());