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

This reverts commit a3b2c4ec.

Reason for revert: UBSan https://logs.chromium.org/logs/v8/buildbucket/cr-buildbucket/8839060153390139249/+/u/Check/gdbjit

Original change's description:
> [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/+/3067754
> Reviewed-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}

Bug: v8:11908
Change-Id: Ic1a74a9239e8ef6107efd36f61c089ae6bfc5b6c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3093365
Auto-Submit: Zhi An Ng <zhin@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#76274}

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 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 };
+  // 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 };
 
   // Type of event.
   EventType type;

src/diagnostics/gdb-jit.cc

@@ -10,7 +10,6 @@
 
 #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"
@@ -897,20 +896,17 @@ class CodeDescription {
   };
 #endif
 
-  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) {}
+  CodeDescription(const char* name, Code code, SharedFunctionInfo shared,
+                  LineInfo* lineinfo)
+      : name_(name), code_(code), shared_info_(shared), lineinfo_(lineinfo) {}
 
   const char* name() const { return name_; }
 
   LineInfo* lineinfo() const { return lineinfo_; }
 
-  bool is_function() const { return is_function_; }
+  bool is_function() const {
+    return CodeKindIsOptimizedJSFunction(code_.kind());
+  }
 
   bool has_scope_info() const { return !shared_info_.is_null(); }
 
@@ -919,11 +915,15 @@ class CodeDescription {
     return shared_info_.scope_info();
   }
 
-  uintptr_t CodeStart() const { return code_region_.begin(); }
+  uintptr_t CodeStart() const {
+    return static_cast<uintptr_t>(code_.InstructionStart());
+  }
 
-  uintptr_t CodeEnd() const { return code_region_.end(); }
+  uintptr_t CodeEnd() const {
+    return static_cast<uintptr_t>(code_.InstructionEnd());
+  }
 
-  uintptr_t CodeSize() const { return code_region_.size(); }
+  uintptr_t CodeSize() const { return CodeEnd() - CodeStart(); }
 
   bool has_script() {
     return !shared_info_.is_null() && shared_info_.script().IsScript();
@@ -933,8 +933,6 @@ 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);
@@ -967,10 +965,9 @@ 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
@@ -1819,17 +1816,26 @@ static JITCodeEntry* CreateELFObject(CodeDescription* desc, Isolate* isolate) {
   return CreateCodeEntry(reinterpret_cast<Address>(w.buffer()), w.position());
 }
 
-// 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 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;
   }
 };
 
-using CodeMap = std::map<base::AddressRegion, JITCodeEntry*, AddressRegionLess>;
+struct CodeMapConfig {
+  using Key = AddressRange;
+  using Value = JITCodeEntry*;
+  using Less = AddressRangeLess;
+};
+
+using CodeMap =
+    std::map<CodeMapConfig::Key, CodeMapConfig::Value, CodeMapConfig::Less>;
 
 static CodeMap* GetCodeMap() {
   // TODO(jgruber): Don't leak.
@@ -1903,37 +1909,36 @@ static void AddUnwindInfo(CodeDescription* desc) {
 
 static base::LazyMutex mutex = LAZY_MUTEX_INITIALIZER;
 
-// Remove entries from the map that intersect the given address region,
+// Remove entries from the map that intersect the given address range,
 // and deregister them from GDB.
-static void RemoveJITCodeEntries(CodeMap* map,
-                                 const base::AddressRegion region) {
-  DCHECK_LT(region.begin(), region.end());
+static void RemoveJITCodeEntries(CodeMap* map, const AddressRange& range) {
+  DCHECK(range.start < range.end);
 
   if (map->empty()) return;
 
   // Find the first overlapping entry.
 
-  // If successful, points to the first element not less than `region`. The
+  // If successful, points to the first element not less than `range`. The
   // returned iterator has the key in `first` and the value in `second`.
-  auto it = map->lower_bound(region);
+  auto it = map->lower_bound(range);
   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() <= region.begin()) break;
+      if ((*it).first.end <= range.start) break;
       start_it = it;
     }
   }
 
-  DCHECK_NE(start_it, map->end());
+  DCHECK(start_it != map->end());
 
-  // Find the first non-overlapping entry after `region`.
+  // Find the first non-overlapping entry after `range`.
 
-  const auto end_it = map->lower_bound({region.end(), 0});
+  const auto end_it = map->lower_bound({range.end, 0});
 
-  // Evict intersecting regions.
+  // Evict intersecting ranges.
 
   if (std::distance(start_it, end_it) < 1) return;  // No overlapping entries.
 
@@ -1947,7 +1952,7 @@ static void RemoveJITCodeEntries(CodeMap* map,
 }
 
 // Insert the entry into the map and register it with GDB.
-static void AddJITCodeEntry(CodeMap* map, const base::AddressRegion region,
+static void AddJITCodeEntry(CodeMap* map, const AddressRange& range,
                             JITCodeEntry* entry, bool dump_if_enabled,
                             const char* name_hint) {
 #if defined(DEBUG) && !V8_OS_WIN
@@ -1964,21 +1969,24 @@ static void AddJITCodeEntry(CodeMap* map, const base::AddressRegion region,
   }
 #endif
 
-  auto result = map->emplace(region, entry);
+  auto result = map->emplace(range, entry);
   DCHECK(result.second);  // Insertion happened.
   USE(result);
 
   RegisterCodeEntry(entry);
 }
 
-static void AddCode(const char* name, base::AddressRegion region,
-                    SharedFunctionInfo shared, LineInfo* lineinfo,
-                    Isolate* isolate, bool is_function) {
+static void AddCode(const char* name, Code code, SharedFunctionInfo shared,
+                    LineInfo* lineinfo) {
   DisallowGarbageCollection no_gc;
-  CodeDescription code_desc(name, region, shared, lineinfo, is_function);
 
   CodeMap* code_map = GetCodeMap();
-  RemoveJITCodeEntries(code_map, region);
+  AddressRange range;
+  range.start = code.address();
+  range.end = code.address() + code.CodeSize();
+  RemoveJITCodeEntries(code_map, range);
+
+  CodeDescription code_desc(name, code, shared, lineinfo);
 
   if (!FLAG_gdbjit_full && !code_desc.IsLineInfoAvailable()) {
     delete lineinfo;
@@ -1986,6 +1994,7 @@ static void AddCode(const char* name, base::AddressRegion region,
   }
 
   AddUnwindInfo(&code_desc);
+  Isolate* isolate = code.GetIsolate();
   JITCodeEntry* entry = CreateELFObject(&code_desc, isolate);
 
   delete lineinfo;
@@ -2001,40 +2010,25 @@ static void AddCode(const char* name, base::AddressRegion region,
       should_dump = (name_hint != nullptr);
     }
   }
-  AddJITCodeEntry(code_map, region, entry, should_dump, name_hint);
+  AddJITCodeEntry(code_map, range, entry, should_dump, name_hint);
 }
 
 void EventHandler(const v8::JitCodeEvent* event) {
   if (!FLAG_gdbjit) return;
-  if ((event->code_type != v8::JitCodeEvent::JIT_CODE) &&
-      (event->code_type != v8::JitCodeEvent::WASM_CODE)) {
-    return;
-  }
+  if (event->code_type != v8::JitCodeEvent::JIT_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);
-      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);
+      AddCode(event_name.c_str(), code, shared, lineinfo);
       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::WASM_CODE;
+  event.code_type = JitCodeEvent::JIT_CODE;
   event.code_start = code->instructions().begin();
   event.code_len = code->instructions().length();
   event.name.str = name;
@@ -1558,14 +1558,12 @@ void Logger::CodeLinePosInfoRecordEvent(Address code_start,
   CodeLinePosEvent(*jit_logger_, code_start, iter, code_type);
 }
 
-#if V8_ENABLE_WEBASSEMBLY
-void Logger::WasmCodeLinePosInfoRecordEvent(
+void Logger::CodeLinePosInfoRecordEvent(
     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::WASM_CODE);
+  CodeLinePosEvent(*jit_logger_, code_start, iter, JitCodeEvent::JIT_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,10 +241,8 @@ class Logger : public CodeEventListener {
   void CodeLinePosInfoRecordEvent(Address code_start,
                                   ByteArray source_position_table,
                                   JitCodeEvent::CodeType code_type);
-#if V8_ENABLE_WEBASSEMBLY
-  void WasmCodeLinePosInfoRecordEvent(
+  void CodeLinePosInfoRecordEvent(
       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,17 +267,14 @@ void WasmCode::LogCode(Isolate* isolate, const char* source_url,
                  "wasm-function[%d]", index()));
     name = base::VectorOf(name_buffer);
   }
-
-  // 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, 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));
+
+  if (!source_positions().empty()) {
+    LOG_CODE_EVENT(isolate, CodeLinePosInfoRecordEvent(instruction_start(),
+                                                       source_positions()));
+  }
 }
 
 void WasmCode::Validate() const {

test/cctest/cctest.status

@@ -580,7 +580,6 @@
   '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,7 +76,6 @@
 
 #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
 
@@ -13739,14 +13738,16 @@ 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 = "Function:";
-  if (i::FLAG_lazy &&
-      event->code_type != v8::JitCodeEvent::CodeType::WASM_CODE) {
+  static const char* kPreamble;
+  if (!i::FLAG_lazy) {
+    kPreamble = "Function:";
+  } else {
     kPreamble = "LazyCompile:";
   }
   static size_t kPreambleLen = strlen(kPreamble);
@@ -13779,6 +13780,7 @@ 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);
@@ -13871,6 +13873,7 @@ static void event_handler(const v8::JitCodeEvent* event) {
   }
 }
 
+
 UNINITIALIZED_TEST(SetJitCodeEventHandler) {
   i::FLAG_stress_compaction = true;
   i::FLAG_incremental_marking = false;
@@ -13995,77 +13998,6 @@ 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);
-}());