[wasm-debug-eval] Reland: Implement additional evaluator API

This CL relands the implementation of the __getLocal and __sbrk APIs of
the evaluator interface reverted in efea7407. Update the original
commit to account for a changes to the import function name tracking and
defaulting to debugging with liftoff.

Change-Id: I9674aad419fb1dab0a9ecbb5d3fd4c33186b127a
Bug: chromium:1020120
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2151353
Commit-Queue: Philip Pfaffe <pfaffe@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67178}

src/wasm/wasm-debug-evaluate.h

@@ -16,7 +16,7 @@ namespace wasm {
 
 MaybeHandle<String> V8_EXPORT_PRIVATE DebugEvaluate(
     Vector<const byte> snippet, Handle<WasmInstanceObject> debuggee_instance,
-    WasmInterpreter::FramePtr frame);
+    StandardFrame* frame);
 
 }  // namespace wasm
 }  // namespace internal

src/wasm/wasm-debug.cc

@@ -24,6 +24,7 @@
 #include "src/wasm/wasm-limits.h"
 #include "src/wasm/wasm-module.h"
 #include "src/wasm/wasm-objects-inl.h"
+#include "src/wasm/wasm-value.h"
 #include "src/zone/accounting-allocator.h"
 
 namespace v8 {
@@ -548,6 +549,17 @@ class DebugInfoImpl {
   explicit DebugInfoImpl(NativeModule* native_module)
       : native_module_(native_module) {}
 
+  WasmValue GetLocalValue(int local, Isolate* isolate, Address pc, Address fp,
+                          Address debug_break_fp) {
+    wasm::WasmCodeRefScope wasm_code_ref_scope;
+    wasm::WasmCode* code =
+        isolate->wasm_engine()->code_manager()->LookupCode(pc);
+    auto* debug_side_table = GetDebugSideTable(code, isolate->allocator());
+    int pc_offset = static_cast<int>(pc - code->instruction_start());
+    auto* debug_side_table_entry = debug_side_table->GetEntry(pc_offset);
+    return GetValue(debug_side_table_entry, local, fp, debug_break_fp);
+  }
+
   Handle<JSObject> GetLocalScopeObject(Isolate* isolate, Address pc, Address fp,
                                        Address debug_break_fp) {
     Handle<JSObject> local_scope_object =
@@ -957,6 +969,11 @@ DebugInfo::DebugInfo(NativeModule* native_module)
 
 DebugInfo::~DebugInfo() = default;
 
+WasmValue DebugInfo::GetLocalValue(int local, Isolate* isolate, Address pc,
+                                   Address fp, Address debug_break_fp) {
+  return impl_->GetLocalValue(local, isolate, pc, fp, debug_break_fp);
+}
+
 Handle<JSObject> DebugInfo::GetLocalScopeObject(Isolate* isolate, Address pc,
                                                 Address fp,
                                                 Address debug_break_fp) {

src/wasm/wasm-debug.h

@@ -33,6 +33,7 @@ class LocalNames;
 class NativeModule;
 class WasmCode;
 class WireBytesRef;
+class WasmValue;
 
 // Side table storing information used to inspect Liftoff frames at runtime.
 // This table is only created on demand for debugging, so it is not optimized
@@ -143,6 +144,9 @@ class DebugInfo {
 
   // {fp} is the frame pointer of the Liftoff frame, {debug_break_fp} that of
   // the {WasmDebugBreak} frame (if any).
+  WasmValue GetLocalValue(int local, Isolate*, Address pc, Address fp,
+                          Address debug_break_fp);
+
   Handle<JSObject> GetLocalScopeObject(Isolate*, Address pc, Address fp,
                                        Address debug_break_fp);
 

src/wasm/wasm-module-builder.cc

@@ -2,21 +2,19 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "src/codegen/signature.h"
+#include "src/wasm/wasm-module-builder.h"
 
+#include "src/base/memory.h"
+#include "src/codegen/signature.h"
 #include "src/handles/handles.h"
 #include "src/init/v8.h"
 #include "src/objects/objects-inl.h"
-#include "src/zone/zone-containers.h"
-
 #include "src/wasm/function-body-decoder.h"
 #include "src/wasm/leb-helper.h"
 #include "src/wasm/wasm-constants.h"
-#include "src/wasm/wasm-module-builder.h"
 #include "src/wasm/wasm-module.h"
 #include "src/wasm/wasm-opcodes.h"
-
-#include "src/base/memory.h"
+#include "src/zone/zone-containers.h"
 
 namespace v8 {
 namespace internal {
@@ -336,16 +334,17 @@ uint32_t WasmModuleBuilder::AddTable(ValueType type, uint32_t min_size,
   return static_cast<uint32_t>(tables_.size() - 1);
 }
 
-uint32_t WasmModuleBuilder::AddImport(Vector<const char> name,
-                                      FunctionSig* sig) {
+uint32_t WasmModuleBuilder::AddImport(Vector<const char> name, FunctionSig* sig,
+                                      Vector<const char> module) {
   DCHECK(adding_imports_allowed_);
-  function_imports_.push_back({name, AddSignature(sig)});
+  function_imports_.push_back({module, name, AddSignature(sig)});
   return static_cast<uint32_t>(function_imports_.size() - 1);
 }
 
 uint32_t WasmModuleBuilder::AddGlobalImport(Vector<const char> name,
-                                            ValueType type, bool mutability) {
-  global_imports_.push_back({name, type.value_type_code(), mutability});
+                                            ValueType type, bool mutability,
+                                            Vector<const char> module) {
+  global_imports_.push_back({module, name, type.value_type_code(), mutability});
   return static_cast<uint32_t>(global_imports_.size() - 1);
 }
 
@@ -424,15 +423,15 @@ void WasmModuleBuilder::WriteTo(ZoneBuffer* buffer) const {
     size_t start = EmitSection(kImportSectionCode, buffer);
     buffer->write_size(global_imports_.size() + function_imports_.size());
     for (auto import : global_imports_) {
-      buffer->write_u32v(0);              // module name (length)
-      buffer->write_string(import.name);  // field name
+      buffer->write_string(import.module);  // module name
+      buffer->write_string(import.name);    // field name
       buffer->write_u8(kExternalGlobal);
       buffer->write_u8(import.type_code);
       buffer->write_u8(import.mutability ? 1 : 0);
     }
     for (auto import : function_imports_) {
-      buffer->write_u32v(0);              // module name (length)
-      buffer->write_string(import.name);  // field name
+      buffer->write_string(import.module);  // module name
+      buffer->write_string(import.name);    // field name
       buffer->write_u8(kExternalFunction);
       buffer->write_u32v(import.sig_index);
     }

src/wasm/wasm-module-builder.h

@@ -5,16 +5,15 @@
 #ifndef V8_WASM_WASM_MODULE_BUILDER_H_
 #define V8_WASM_WASM_MODULE_BUILDER_H_
 
-#include "src/codegen/signature.h"
-#include "src/zone/zone-containers.h"
-
 #include "src/base/memory.h"
+#include "src/codegen/signature.h"
 #include "src/utils/vector.h"
 #include "src/wasm/leb-helper.h"
 #include "src/wasm/local-decl-encoder.h"
 #include "src/wasm/wasm-module.h"
 #include "src/wasm/wasm-opcodes.h"
 #include "src/wasm/wasm-result.h"
+#include "src/zone/zone-containers.h"
 
 namespace v8 {
 namespace internal {
@@ -233,12 +232,13 @@ class V8_EXPORT_PRIVATE WasmModuleBuilder : public ZoneObject {
   explicit WasmModuleBuilder(Zone* zone);
 
   // Building methods.
-  uint32_t AddImport(Vector<const char> name, FunctionSig* sig);
+  uint32_t AddImport(Vector<const char> name, FunctionSig* sig,
+                     Vector<const char> module = {});
   WasmFunctionBuilder* AddFunction(FunctionSig* sig = nullptr);
   uint32_t AddGlobal(ValueType type, bool mutability = true,
                      const WasmInitExpr& init = WasmInitExpr());
   uint32_t AddGlobalImport(Vector<const char> name, ValueType type,
-                           bool mutability);
+                           bool mutability, Vector<const char> module = {});
   void AddDataSegment(const byte* data, uint32_t size, uint32_t dest);
   uint32_t AddSignature(FunctionSig* sig);
   // In the current implementation, it's supported to have uninitialized slots
@@ -272,11 +272,13 @@ class V8_EXPORT_PRIVATE WasmModuleBuilder : public ZoneObject {
 
  private:
   struct WasmFunctionImport {
+    Vector<const char> module;
     Vector<const char> name;
     uint32_t sig_index;
   };
 
   struct WasmGlobalImport {
+    Vector<const char> module;
     Vector<const char> name;
     ValueTypeCode type_code;
     bool mutability;

test/cctest/wasm/test-wasm-debug-evaluate.cc

@@ -43,14 +43,20 @@ namespace {
 template <typename... FunctionArgsT>
 class TestCode {
  public:
-  TestCode(WasmRunnerBase* runner, std::initializer_list<byte> code)
-      : compiler_(&runner->NewFunction<FunctionArgsT...>()), code_(code) {
+  TestCode(WasmRunnerBase* runner, std::initializer_list<byte> code,
+           std::initializer_list<ValueType::Kind> locals = {})
+      : compiler_(&runner->NewFunction<FunctionArgsT...>()),
+        code_(code),
+        locals_(static_cast<uint32_t>(locals.size())) {
+    for (ValueType::Kind T : locals) {
+      compiler_->AllocateLocal(ValueType(T));
+    }
     compiler_->Build(code.begin(), code.end());
   }
 
   Handle<BreakPoint> BreakOnReturn(WasmRunnerBase* runner) {
     runner->TierDown();
-    uint32_t return_offset_in_function = FindReturn();
+    uint32_t return_offset_in_function = locals_ + FindReturn();
 
     int function_index = compiler_->function_index();
     int function_offset =
@@ -98,6 +104,7 @@ class TestCode {
 
   WasmFunctionCompiler* compiler_;
   std::vector<byte> code_;
+  uint32_t locals_;
 };
 
 class WasmEvaluatorBuilder {
@@ -108,6 +115,9 @@ class WasmEvaluatorBuilder {
       : zone_(&allocator_, ZONE_NAME), builder_(&zone_) {
     get_memory_function_index = AddImport<void, uint32_t, uint32_t, uint32_t>(
         CStrVector("__getMemory"));
+    get_local_function_index =
+        AddImport<void, uint32_t, uint32_t>(CStrVector("__getLocal"));
+    sbrk_function_index = AddImport<uint32_t, uint32_t>(CStrVector("__sbrk"));
     wasm_format_function =
         builder_.AddFunction(WasmRunnerBase::CreateSig<uint32_t>(&zone_));
     wasm_format_function->SetName(CStrVector("wasm_format"));
@@ -119,7 +129,8 @@ class WasmEvaluatorBuilder {
   template <typename ReturnT, typename... ArgTs>
   uint32_t AddImport(Vector<const char> name) {
     return builder_.AddImport(
-        name, WasmRunnerBase::CreateSig<ReturnT, ArgTs...>(&zone_));
+        name, WasmRunnerBase::CreateSig<ReturnT, ArgTs...>(&zone_),
+        CStrVector("env"));
   }
 
   void push_back(std::initializer_list<byte> code) {
@@ -127,6 +138,16 @@ class WasmEvaluatorBuilder {
                                    static_cast<uint32_t>(code.size()));
   }
 
+  void CallSbrk(std::initializer_list<byte> args) {
+    push_back(args);
+    push_back({WASM_CALL_FUNCTION0(sbrk_function_index)});
+  }
+
+  void CallGetLocal(std::initializer_list<byte> args) {
+    push_back(args);
+    push_back({WASM_CALL_FUNCTION0(get_local_function_index)});
+  }
+
   void CallGetMemory(std::initializer_list<byte> args) {
     push_back(args);
     push_back({WASM_CALL_FUNCTION0(get_memory_function_index)});
@@ -143,6 +164,8 @@ class WasmEvaluatorBuilder {
   Zone zone_;
   WasmModuleBuilder builder_;
   uint32_t get_memory_function_index = 0;
+  uint32_t get_local_function_index = 0;
+  uint32_t sbrk_function_index = 0;
   WasmFunctionBuilder* wasm_format_function = nullptr;
 };
 
@@ -192,12 +215,10 @@ class WasmBreakHandler : public debug::DebugDelegate {
     FrameSummary::WasmInterpretedFrameSummary summary =
         FrameSummary::GetTop(frame_it.frame()).AsWasmInterpreted();
     Handle<WasmInstanceObject> instance = summary.wasm_instance();
-    WasmInterpreter::FramePtr frame =
-        instance->debug_info().GetInterpretedFrame(frame_it.frame()->fp(), 0);
 
     MaybeHandle<String> result_handle = v8::internal::wasm::DebugEvaluate(
         {evaluator_bytes_.begin(), evaluator_bytes_.size()}, instance,
-        std::move(frame));
+        frame_it.frame());
 
     Maybe<std::string> error_message = GetPendingExceptionAsString();
     Maybe<std::string> result_message =
@@ -218,7 +239,7 @@ WASM_COMPILED_EXEC_TEST(WasmDebugEvaluate_CompileFailed) {
   code.BreakOnReturn(&runner);
 
   WasmEvaluatorBuilder evaluator(execution_tier);
-  // Create a module that doesn't compile by missing the END bytecode
+  // Create a module that doesn't compile by missing the END bytecode.
   evaluator.push_back({WASM_RETURN1(WASM_I32V_1(33))});
 
   Isolate* isolate = runner.main_isolate();
@@ -262,11 +283,12 @@ WASM_COMPILED_EXEC_TEST(WasmDebugEvaluate_ExecuteFailed_SEGV) {
 
   TestCode<int> code(&runner, {WASM_RETURN1(WASM_I32V_1(32))});
 
-  // Create a module that doesn't compile by missing the END bytecode
-  WasmEvaluatorBuilder evaluator(execution_tier);
+  // Use a max memory size of 2 here to verify the precondition for the
+  // GrowMemory test below.
+  WasmEvaluatorBuilder evaluator(execution_tier, 1, 2);
   code.BreakOnReturn(&runner);
 
-  // Load 1 byte from an address that's too high
+  // Load 1 byte from an address that's too high.
   evaluator.CallGetMemory(
       {WASM_I32V_1(32), WASM_I32V_1(1), WASM_I32V_3((1 << 16) + 1)});
   evaluator.push_back({WASM_RETURN1(WASM_I32V_1(33)), WASM_END});
@@ -283,6 +305,34 @@ WASM_COMPILED_EXEC_TEST(WasmDebugEvaluate_ExecuteFailed_SEGV) {
       std::string::npos);
 }
 
+WASM_COMPILED_EXEC_TEST(WasmDebugEvaluate_GrowMemory) {
+  WasmRunner<int> runner(execution_tier);
+  runner.builder().AddMemoryElems<int32_t>(64);
+
+  TestCode<int> code(
+      &runner,
+      {WASM_STORE_MEM(MachineType::Int32(), WASM_I32V_1(32), WASM_I32V_2('A')),
+       WASM_RETURN1(WASM_LOAD_MEM(MachineType::Int32(), WASM_I32V_1(32)))});
+  code.BreakOnReturn(&runner);
+
+  WasmEvaluatorBuilder evaluator(execution_tier, 1, 2);
+  // Grow the memory.
+  evaluator.CallSbrk({WASM_I32V_1(1)});
+  // Load 1 byte from an address that's too high for the default memory.
+  evaluator.CallGetMemory(
+      {WASM_I32V_1(32), WASM_I32V_1(1), WASM_I32V_3((1 << 16) + 1)});
+  evaluator.push_back({WASM_RETURN1(WASM_I32V_3((1 << 16) + 1)), WASM_END});
+
+  Isolate* isolate = runner.main_isolate();
+  WasmBreakHandler break_handler(isolate, evaluator.bytes());
+  CHECK(!code.Run(&runner).is_null());
+
+  WasmBreakHandler::EvaluationResult result =
+      break_handler.result().ToChecked();
+  CHECK(result.error.IsNothing());
+  CHECK_EQ(result.result.ToChecked(), "A");
+}
+
 WASM_COMPILED_EXEC_TEST(WasmDebugEvaluate_LinearMemory) {
   WasmRunner<int> runner(execution_tier);
   runner.builder().AddMemoryElems<int32_t>(64);
@@ -309,6 +359,30 @@ WASM_COMPILED_EXEC_TEST(WasmDebugEvaluate_LinearMemory) {
   CHECK_EQ(result.result.ToChecked(), "A");
 }
 
+WASM_COMPILED_EXEC_TEST(WasmDebugEvaluate_Locals) {
+  WasmRunner<int> runner(execution_tier);
+  runner.builder().AddMemoryElems<int32_t>(64);
+
+  TestCode<int> code(
+      &runner,
+      {WASM_SET_LOCAL(0, WASM_I32V_2('A')), WASM_RETURN1(WASM_GET_LOCAL(0))},
+      {ValueType::kI32});
+  code.BreakOnReturn(&runner);
+
+  WasmEvaluatorBuilder evaluator(execution_tier);
+  evaluator.CallGetLocal({WASM_I32V_1(0), WASM_I32V_1(33)});
+  evaluator.push_back({WASM_RETURN1(WASM_I32V_1(33)), WASM_END});
+
+  Isolate* isolate = runner.main_isolate();
+  WasmBreakHandler break_handler(isolate, evaluator.bytes());
+  CHECK(!code.Run(&runner).is_null());
+
+  WasmBreakHandler::EvaluationResult result =
+      break_handler.result().ToChecked();
+  CHECK(result.error.IsNothing());
+  CHECK_EQ(result.result.ToChecked(), "A");
+}
+
 }  // namespace
 }  // namespace wasm
 }  // namespace internal