Add more index spaces to the WebAssembly JS debug proxy

This CL adds the globals index space to the JS debug proxy as well as the
stack object. It also adds few small helpers to simplify the proxy setup
a little, since all index spaces work exaclty the same.

Bug: chromium:1127914
Change-Id: I707292ab7f44aafb73751c17fdacfef976316f39
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2448468
Commit-Queue: Philip Pfaffe <pfaffe@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70332}

src/wasm/wasm-js.cc

@@ -2379,6 +2379,21 @@ std::vector<Handle<String>> GetLocalNames(Handle<WasmInstanceObject> instance,
   return names;
 }
 
+// Generate names for the globals. Names either come from the name table,
+// otherwise the default $globalX is used.
+std::vector<Handle<String>> GetGlobalNames(
+    Handle<WasmInstanceObject> instance) {
+  Isolate* isolate = instance->GetIsolate();
+  auto& globals = instance->module()->globals;
+  std::vector<Handle<String>> names;
+  for (uint32_t i = 0; i < globals.size(); ++i) {
+    names.emplace_back(GetNameOrDefault(
+        isolate, WasmInstanceObject::GetGlobalNameOrNull(isolate, instance, i),
+        "$global", i));
+  }
+  return names;
+}
+
 Handle<WasmInstanceObject> GetInstance(Isolate* isolate,
                                        Handle<JSObject> handler) {
   Handle<Object> instance =
@@ -2444,32 +2459,21 @@ static Handle<Object> WasmValueToObject(Isolate* isolate,
   return factory->undefined_value();
 }
 
-bool HasLocalImpl(Isolate* isolate, Handle<Name> property,
-                  Handle<JSObject> handler, bool enable_index_lookup) {
+base::Optional<int> HasLocalImpl(Isolate* isolate, Handle<Name> property,
+                                 Handle<JSObject> handler,
+                                 bool enable_index_lookup) {
   Handle<WasmInstanceObject> instance = GetInstance(isolate, handler);
 
   base::Optional<int> index =
       ResolveValueSelector(isolate, property, handler, enable_index_lookup);
-  if (!index) return false;
+  if (!index) return index;
   Address pc = GetPC(isolate, handler);
 
   wasm::DebugInfo* debug_info =
       instance->module_object().native_module()->GetDebugInfo();
   int num_locals = debug_info->GetNumLocals(pc);
-  return 0 <= index && index < num_locals;
-}
-
-// Has trap callback for the locals index space proxy.
-void HasLocalCallback(const v8::FunctionCallbackInfo<v8::Value>& args) {
-  if (args.Length() < 2) return;
-  Isolate* isolate = reinterpret_cast<Isolate*>(args.GetIsolate());
-  DCHECK(args.This()->IsObject());
-  Handle<JSObject> handler =
-      Handle<JSObject>::cast(Utils::OpenHandle(*args.This()));
-
-  DCHECK(args[1]->IsName());
-  Handle<Name> property = Handle<Name>::cast(Utils::OpenHandle(*args[1]));
-  args.GetReturnValue().Set(HasLocalImpl(isolate, property, handler, true));
+  if (0 <= index && index < num_locals) return index;
+  return {};
 }
 
 Handle<Object> GetLocalImpl(Isolate* isolate, Handle<Name> property,
@@ -2479,7 +2483,7 @@ Handle<Object> GetLocalImpl(Isolate* isolate, Handle<Name> property,
   Handle<WasmInstanceObject> instance = GetInstance(isolate, handler);
 
   base::Optional<int> index =
-      ResolveValueSelector(isolate, property, handler, enable_index_lookup);
+      HasLocalImpl(isolate, property, handler, enable_index_lookup);
   if (!index) return factory->undefined_value();
   Address pc = GetPC(isolate, handler);
   Address fp = GetFP(isolate, handler);
@@ -2487,14 +2491,57 @@ Handle<Object> GetLocalImpl(Isolate* isolate, Handle<Name> property,
 
   wasm::DebugInfo* debug_info =
       instance->module_object().native_module()->GetDebugInfo();
-  int num_locals = debug_info->GetNumLocals(pc);
-  if (0 > index || index >= num_locals) return factory->undefined_value();
   wasm::WasmValue value = debug_info->GetLocalValue(*index, pc, fp, callee_fp);
   return WasmValueToObject(isolate, value);
 }
 
-// Get trap callback for the locals index space proxy.
-void GetLocalCallback(const v8::FunctionCallbackInfo<v8::Value>& args) {
+base::Optional<int> HasGlobalImpl(Isolate* isolate, Handle<Name> property,
+                                  Handle<JSObject> handler,
+                                  bool enable_index_lookup) {
+  Handle<WasmInstanceObject> instance = GetInstance(isolate, handler);
+  base::Optional<int> index =
+      ResolveValueSelector(isolate, property, handler, enable_index_lookup);
+  if (!index) return index;
+
+  const std::vector<wasm::WasmGlobal>& globals = instance->module()->globals;
+  if (globals.size() <= kMaxInt && 0 <= *index &&
+      *index < static_cast<int>(globals.size())) {
+    return index;
+  }
+  return {};
+}
+
+Handle<Object> GetGlobalImpl(Isolate* isolate, Handle<Name> property,
+                             Handle<JSObject> handler,
+                             bool enable_index_lookup) {
+  Handle<WasmInstanceObject> instance = GetInstance(isolate, handler);
+  base::Optional<int> index =
+      HasGlobalImpl(isolate, property, handler, enable_index_lookup);
+  if (!index) return isolate->factory()->undefined_value();
+
+  const std::vector<wasm::WasmGlobal>& globals = instance->module()->globals;
+  return WasmValueToObject(
+      isolate, WasmInstanceObject::GetGlobalValue(instance, globals[*index]));
+}
+
+// Generic has trap callback for the index space proxies.
+template <base::Optional<int> Impl(Isolate*, Handle<Name>, Handle<JSObject>,
+                                   bool)>
+void HasTrapCallback(const v8::FunctionCallbackInfo<v8::Value>& args) {
+  DCHECK_GE(args.Length(), 2);
+  Isolate* isolate = reinterpret_cast<Isolate*>(args.GetIsolate());
+  DCHECK(args.This()->IsObject());
+  Handle<JSObject> handler =
+      Handle<JSObject>::cast(Utils::OpenHandle(*args.This()));
+
+  DCHECK(args[1]->IsName());
+  Handle<Name> property = Handle<Name>::cast(Utils::OpenHandle(*args[1]));
+  args.GetReturnValue().Set(Impl(isolate, property, handler, true).has_value());
+}
+
+// Generic get trap callback for the index space proxies.
+template <Handle<Object> Impl(Isolate*, Handle<Name>, Handle<JSObject>, bool)>
+void GetTrapCallback(const v8::FunctionCallbackInfo<v8::Value>& args) {
   DCHECK_GE(args.Length(), 2);
   Isolate* isolate = reinterpret_cast<Isolate*>(args.GetIsolate());
   DCHECK(args.This()->IsObject());
@@ -2504,11 +2551,25 @@ void GetLocalCallback(const v8::FunctionCallbackInfo<v8::Value>& args) {
   DCHECK(args[1]->IsName());
   Handle<Name> property = Handle<Name>::cast(Utils::OpenHandle(*args[1]));
   args.GetReturnValue().Set(
-      Utils::ToLocal(GetLocalImpl(isolate, property, handler, true)));
+      Utils::ToLocal(Impl(isolate, property, handler, true)));
+}
+
+template <typename ReturnT>
+ReturnT DelegateToplevelCall(Isolate* isolate, Handle<JSObject> target,
+                             Handle<Name> property, const char* index_space,
+                             ReturnT (*impl)(Isolate*, Handle<Name>,
+                                             Handle<JSObject>, bool)) {
+  Handle<Object> namespace_proxy =
+      JSObject::GetProperty(isolate, target, index_space).ToHandleChecked();
+  DCHECK(namespace_proxy->IsJSProxy());
+  Handle<JSObject> namespace_handler(
+      JSObject::cast(Handle<JSProxy>::cast(namespace_proxy)->handler()),
+      isolate);
+  return impl(isolate, property, namespace_handler, false);
 }
 
 // Has trap callback for the top-level proxy.
-void HasToplevelCallback(const v8::FunctionCallbackInfo<v8::Value>& args) {
+void ToplevelHasTrapCallback(const v8::FunctionCallbackInfo<v8::Value>& args) {
   DCHECK_GE(args.Length(), 2);
   Isolate* isolate = reinterpret_cast<Isolate*>(args.GetIsolate());
   DCHECK(args[0]->IsObject());
@@ -2524,13 +2585,12 @@ void HasToplevelCallback(const v8::FunctionCallbackInfo<v8::Value>& args) {
   }
 
   // Now check the index space proxies in order if they know the property.
-  Handle<Object> namespace_proxy =
-      JSObject::GetProperty(isolate, target, "locals").ToHandleChecked();
-  DCHECK(namespace_proxy->IsJSProxy());
-  Handle<JSObject> namespace_handler(
-      JSObject::cast(Handle<JSProxy>::cast(namespace_proxy)->handler()),
-      isolate);
-  if (HasLocalImpl(isolate, property, namespace_handler, false)) {
+  if (DelegateToplevelCall(isolate, target, property, "locals", HasLocalImpl)) {
+    args.GetReturnValue().Set(true);
+    return;
+  }
+  if (DelegateToplevelCall(isolate, target, property, "globals",
+                           HasGlobalImpl)) {
     args.GetReturnValue().Set(true);
     return;
   }
@@ -2538,8 +2598,8 @@ void HasToplevelCallback(const v8::FunctionCallbackInfo<v8::Value>& args) {
 }
 
 // Get trap callback for the top-level proxy.
-void GetToplevelCallback(const v8::FunctionCallbackInfo<v8::Value>& args) {
-  if (args.Length() < 2) return;
+void ToplevelGetTrapCallback(const v8::FunctionCallbackInfo<v8::Value>& args) {
+  DCHECK_GE(args.Length(), 2);
   Isolate* isolate = reinterpret_cast<Isolate*>(args.GetIsolate());
   DCHECK(args[0]->IsObject());
   Handle<JSObject> target = Handle<JSObject>::cast(Utils::OpenHandle(*args[0]));
@@ -2557,14 +2617,18 @@ void GetToplevelCallback(const v8::FunctionCallbackInfo<v8::Value>& args) {
   }
 
   // Try the index space proxies in the correct disambiguation order.
-  Handle<Object> namespace_proxy =
-      JSObject::GetProperty(isolate, target, "locals").ToHandleChecked();
-  DCHECK(!namespace_proxy->IsUndefined() && namespace_proxy->IsJSProxy());
-  Handle<JSObject> namespace_handler(
-      JSObject::cast(Handle<JSProxy>::cast(namespace_proxy)->handler()),
-      isolate);
-  value = GetLocalImpl(isolate, property, namespace_handler, false);
-  if (!value->IsUndefined()) args.GetReturnValue().Set(Utils::ToLocal(value));
+  value =
+      DelegateToplevelCall(isolate, target, property, "locals", GetLocalImpl);
+  if (!value->IsUndefined()) {
+    args.GetReturnValue().Set(Utils::ToLocal(value));
+    return;
+  }
+  value =
+      DelegateToplevelCall(isolate, target, property, "globals", GetGlobalImpl);
+  if (!value->IsUndefined()) {
+    args.GetReturnValue().Set(Utils::ToLocal(value));
+    return;
+  }
 }
 
 // Populate a JSMap with name->index mappings from an ordered list of names.
@@ -2606,6 +2670,21 @@ Handle<JSProxy> GetJSProxy(
 
   return factory->NewJSProxy(target, handler);
 }
+
+Handle<JSObject> GetStackObject(WasmFrame* frame) {
+  Isolate* isolate = frame->isolate();
+  Handle<JSObject> object = isolate->factory()->NewJSObjectWithNullProto();
+  wasm::DebugInfo* debug_info =
+      frame->wasm_instance().module_object().native_module()->GetDebugInfo();
+  int num_values = debug_info->GetStackDepth(frame->pc());
+  for (int i = 0; i < num_values; ++i) {
+    wasm::WasmValue value = debug_info->GetStackValue(
+        i, frame->pc(), frame->fp(), frame->callee_fp());
+    JSObject::AddDataElement(object, i, WasmValueToObject(isolate, value),
+                             NONE);
+  }
+  return object;
+}
 }  // namespace
 
 // This function generates the JS debug proxy for a given Wasm frame. The debug
@@ -2650,21 +2729,32 @@ Handle<JSProxy> WasmJs::GetJSDebugProxy(WasmFrame* frame) {
 
   // The top level proxy delegates lookups to the index space proxies.
   Handle<JSObject> handler = factory->NewJSObjectWithNullProto();
-  InstallFunc(isolate, handler, "get", GetToplevelCallback, 3, false,
+  InstallFunc(isolate, handler, "get", ToplevelGetTrapCallback, 3, false,
               READ_ONLY);
-  InstallFunc(isolate, handler, "has", HasToplevelCallback, 2, false,
+  InstallFunc(isolate, handler, "has", ToplevelHasTrapCallback, 2, false,
               READ_ONLY);
 
   Handle<JSObject> target = factory->NewJSObjectWithNullProto();
 
   // Generate JSMaps per index space for name->index lookup. Every index space
   // proxy is associated with its table for local name lookup.
+
   auto local_name_table =
       GetNameTable(isolate, GetLocalNames(instance, frame->pc()));
   auto locals =
-      GetJSProxy(frame, local_name_table, GetLocalCallback, HasLocalCallback);
+      GetJSProxy(frame, local_name_table, GetTrapCallback<GetLocalImpl>,
+                 HasTrapCallback<HasLocalImpl>);
   JSObject::AddProperty(isolate, target, "locals", locals, READ_ONLY);
 
+  auto global_name_table = GetNameTable(isolate, GetGlobalNames(instance));
+  auto globals =
+      GetJSProxy(frame, global_name_table, GetTrapCallback<GetGlobalImpl>,
+                 HasTrapCallback<HasGlobalImpl>);
+  JSObject::AddProperty(isolate, target, "globals", globals, READ_ONLY);
+
+  auto stack = GetStackObject(frame);
+  JSObject::AddProperty(isolate, target, "stack", stack, READ_ONLY);
+
   return factory->NewJSProxy(target, handler);
 }
 

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

@@ -536,12 +536,12 @@ WASM_COMPILED_EXEC_TEST(WasmDebugEvaluate_JavaScript) {
   Handle<String> snippet =
       V8String(isolate,
                "JSON.stringify(["
-               //"$global0, "
+               "$global0, "
                //"$table0, "
                "$var0, "
                //"$main, "
                //"$memory0, "
-               //"globals[0], "
+               "globals[0], "
                //"tables[0], "
                "locals[0], "
                //"functions[0], "
@@ -551,7 +551,7 @@ WASM_COMPILED_EXEC_TEST(WasmDebugEvaluate_JavaScript) {
                //"stack, "
                //"imports, "
                //"exports, "
-               //"globals, "
+               "globals, "
                "locals, "
                //"functions, "
                "], (k, v) => k === 'at' || typeof v === 'undefined' || typeof "
@@ -563,7 +563,7 @@ WASM_COMPILED_EXEC_TEST(WasmDebugEvaluate_JavaScript) {
   WasmJSBreakHandler::EvaluationResult result =
       break_handler.result().ToChecked();
   CHECK_WITH_MSG(result.error.IsNothing(), result.error.ToChecked().c_str());
-  CHECK_EQ(result.result.ToChecked(), "[\"65\",\"65\",{}]");
+  CHECK_EQ(result.result.ToChecked(), "[\"66\",\"65\",\"66\",\"65\",{},{}]");
   //"[\"66\",{},\"65\",\"function 0() { [native code] }\",{},"
   //"\"66\",{},\"65\",\"function 0() { [native code] }\",{},"
   //"{},{},{\"0\":\"65\"},{},{},{},{},{}]");