From afd2692564a6da24f8cabd304fdda803b1e9f1b2 Mon Sep 17 00:00:00 2001 From: Philip Pfaffe Date: Tue, 6 Oct 2020 10:11:47 +0200 Subject: [PATCH] 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 Reviewed-by: Clemens Backes Cr-Commit-Position: refs/heads/master@{#70332} --- src/wasm/wasm-js.cc | 178 ++++++++++++++----- test/cctest/wasm/test-wasm-debug-evaluate.cc | 8 +- 2 files changed, 138 insertions(+), 48 deletions(-) diff --git a/src/wasm/wasm-js.cc b/src/wasm/wasm-js.cc index 440426454d..1187dc0182 100644 --- a/src/wasm/wasm-js.cc +++ b/src/wasm/wasm-js.cc @@ -2379,6 +2379,21 @@ std::vector> GetLocalNames(Handle instance, return names; } +// Generate names for the globals. Names either come from the name table, +// otherwise the default $globalX is used. +std::vector> GetGlobalNames( + Handle instance) { + Isolate* isolate = instance->GetIsolate(); + auto& globals = instance->module()->globals; + std::vector> 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 GetInstance(Isolate* isolate, Handle handler) { Handle instance = @@ -2444,32 +2459,21 @@ static Handle WasmValueToObject(Isolate* isolate, return factory->undefined_value(); } -bool HasLocalImpl(Isolate* isolate, Handle property, - Handle handler, bool enable_index_lookup) { +base::Optional HasLocalImpl(Isolate* isolate, Handle property, + Handle handler, + bool enable_index_lookup) { Handle instance = GetInstance(isolate, handler); base::Optional 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& args) { - if (args.Length() < 2) return; - Isolate* isolate = reinterpret_cast(args.GetIsolate()); - DCHECK(args.This()->IsObject()); - Handle handler = - Handle::cast(Utils::OpenHandle(*args.This())); - - DCHECK(args[1]->IsName()); - Handle property = Handle::cast(Utils::OpenHandle(*args[1])); - args.GetReturnValue().Set(HasLocalImpl(isolate, property, handler, true)); + if (0 <= index && index < num_locals) return index; + return {}; } Handle GetLocalImpl(Isolate* isolate, Handle property, @@ -2479,7 +2483,7 @@ Handle GetLocalImpl(Isolate* isolate, Handle property, Handle instance = GetInstance(isolate, handler); base::Optional 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 GetLocalImpl(Isolate* isolate, Handle 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& args) { +base::Optional HasGlobalImpl(Isolate* isolate, Handle property, + Handle handler, + bool enable_index_lookup) { + Handle instance = GetInstance(isolate, handler); + base::Optional index = + ResolveValueSelector(isolate, property, handler, enable_index_lookup); + if (!index) return index; + + const std::vector& globals = instance->module()->globals; + if (globals.size() <= kMaxInt && 0 <= *index && + *index < static_cast(globals.size())) { + return index; + } + return {}; +} + +Handle GetGlobalImpl(Isolate* isolate, Handle property, + Handle handler, + bool enable_index_lookup) { + Handle instance = GetInstance(isolate, handler); + base::Optional index = + HasGlobalImpl(isolate, property, handler, enable_index_lookup); + if (!index) return isolate->factory()->undefined_value(); + + const std::vector& globals = instance->module()->globals; + return WasmValueToObject( + isolate, WasmInstanceObject::GetGlobalValue(instance, globals[*index])); +} + +// Generic has trap callback for the index space proxies. +template Impl(Isolate*, Handle, Handle, + bool)> +void HasTrapCallback(const v8::FunctionCallbackInfo& args) { + DCHECK_GE(args.Length(), 2); + Isolate* isolate = reinterpret_cast(args.GetIsolate()); + DCHECK(args.This()->IsObject()); + Handle handler = + Handle::cast(Utils::OpenHandle(*args.This())); + + DCHECK(args[1]->IsName()); + Handle property = Handle::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 Impl(Isolate*, Handle, Handle, bool)> +void GetTrapCallback(const v8::FunctionCallbackInfo& args) { DCHECK_GE(args.Length(), 2); Isolate* isolate = reinterpret_cast(args.GetIsolate()); DCHECK(args.This()->IsObject()); @@ -2504,11 +2551,25 @@ void GetLocalCallback(const v8::FunctionCallbackInfo& args) { DCHECK(args[1]->IsName()); Handle property = Handle::cast(Utils::OpenHandle(*args[1])); args.GetReturnValue().Set( - Utils::ToLocal(GetLocalImpl(isolate, property, handler, true))); + Utils::ToLocal(Impl(isolate, property, handler, true))); +} + +template +ReturnT DelegateToplevelCall(Isolate* isolate, Handle target, + Handle property, const char* index_space, + ReturnT (*impl)(Isolate*, Handle, + Handle, bool)) { + Handle namespace_proxy = + JSObject::GetProperty(isolate, target, index_space).ToHandleChecked(); + DCHECK(namespace_proxy->IsJSProxy()); + Handle namespace_handler( + JSObject::cast(Handle::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& args) { +void ToplevelHasTrapCallback(const v8::FunctionCallbackInfo& args) { DCHECK_GE(args.Length(), 2); Isolate* isolate = reinterpret_cast(args.GetIsolate()); DCHECK(args[0]->IsObject()); @@ -2524,13 +2585,12 @@ void HasToplevelCallback(const v8::FunctionCallbackInfo& args) { } // Now check the index space proxies in order if they know the property. - Handle namespace_proxy = - JSObject::GetProperty(isolate, target, "locals").ToHandleChecked(); - DCHECK(namespace_proxy->IsJSProxy()); - Handle namespace_handler( - JSObject::cast(Handle::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& args) { } // Get trap callback for the top-level proxy. -void GetToplevelCallback(const v8::FunctionCallbackInfo& args) { - if (args.Length() < 2) return; +void ToplevelGetTrapCallback(const v8::FunctionCallbackInfo& args) { + DCHECK_GE(args.Length(), 2); Isolate* isolate = reinterpret_cast(args.GetIsolate()); DCHECK(args[0]->IsObject()); Handle target = Handle::cast(Utils::OpenHandle(*args[0])); @@ -2557,14 +2617,18 @@ void GetToplevelCallback(const v8::FunctionCallbackInfo& args) { } // Try the index space proxies in the correct disambiguation order. - Handle namespace_proxy = - JSObject::GetProperty(isolate, target, "locals").ToHandleChecked(); - DCHECK(!namespace_proxy->IsUndefined() && namespace_proxy->IsJSProxy()); - Handle namespace_handler( - JSObject::cast(Handle::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 GetJSProxy( return factory->NewJSProxy(target, handler); } + +Handle GetStackObject(WasmFrame* frame) { + Isolate* isolate = frame->isolate(); + Handle 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 WasmJs::GetJSDebugProxy(WasmFrame* frame) { // The top level proxy delegates lookups to the index space proxies. Handle 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 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, + HasTrapCallback); JSObject::AddProperty(isolate, target, "locals", locals, READ_ONLY); + auto global_name_table = GetNameTable(isolate, GetGlobalNames(instance)); + auto globals = + GetJSProxy(frame, global_name_table, GetTrapCallback, + HasTrapCallback); + 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); } diff --git a/test/cctest/wasm/test-wasm-debug-evaluate.cc b/test/cctest/wasm/test-wasm-debug-evaluate.cc index 8e6d56f6a5..83ef664346 100644 --- a/test/cctest/wasm/test-wasm-debug-evaluate.cc +++ b/test/cctest/wasm/test-wasm-debug-evaluate.cc @@ -536,12 +536,12 @@ WASM_COMPILED_EXEC_TEST(WasmDebugEvaluate_JavaScript) { Handle 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\"},{},{},{},{},{}]");