AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Fix v8windbg Locals pane behavior

Browse Source 
Background:

In order to show custom content in the "Locals" pane in WinDbg, v8windbg
replaces the getter function for a built-in debug model property named
"Debugger.Models.StackFrame.LocalVariables". This is the property that
the debugger fetches when determining what to display in "Locals". The
new implementation of that getter, V8LocalVariables::GetValue, can
either call the original getter (so that WinDbg displays the usual
content for normal C++ frames) or produce a custom result (for builtins
and JIT-compiled JS frames).

The current problem:

In new builds of WinDbg, users of v8windbg no longer see any content in
the Locals pane for stack frames that correspond to builtins or
JIT-compiled code. This is because of a behavior change in WinDbg:
previously, attempting to get Debugger.Models.StackFrame.LocalVariables
would eagerly attempt to find the symbols for the frame and return an
error code if symbols were not found, but now it returns a lazy object
which does not perform symbol lookup until you iterate its properties.
V8LocalVariables::GetValue currently starts with an early-exit path
based on checking whether the original getter succeeded, so the new lazy
implementation causes us to always take that early exit.

Proposed fix:

Rather than relying on the return value from the original getter, which
is not guaranteed to work consistently, we can base our decisions on the
instruction pointer. If it points outside any module, or if it points to
within a function in the module containing V8 whose name starts with
"Builtins_", then we can build a custom result for the Locals pane.

Change-Id: I6644071d5d83a25b964d9f4018265532528cc85c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3759228
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#81856}
This commit is contained in:
Seth Brenith 2022-07-20 08:12:43 -07:00 committed by V8 LUCI CQ
parent aee4f59521
commit 607ad422be
1 changed files with 20 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
tools/v8windbg/src/local-variables.cc
View File

@ -6,6 +6,7 @@
#include <vector>
#include "src/base/logging.h"
#include "tools/v8windbg/base/utilities.h"
#include "tools/v8windbg/src/object-inspection.h"
#include "tools/v8windbg/src/v8-debug-helper-interop.h"
@ -18,12 +19,7 @@ V8LocalVariables::~V8LocalVariables() = default;
IFACEMETHODIMP V8LocalVariables::GetValue(PCWSTR key, IModelObject* context,
IModelObject** value) noexcept {
// See if the frame can fetch locals based on symbols. If so, it's a normal
// C++ frame, so we can be done.
HRESULT original_hr = original_->GetValue(key, context, value);
if (SUCCEEDED(original_hr)) return original_hr;
// Next, try to find out about the instruction pointer. If it is within the V8
// Try to find out about the instruction pointer. If it is within the V8
// module, or points to unknown space outside a module (generated code), then
// we're interested. Otherwise, we have nothing useful to do.
WRL::ComPtr<IModelObject> attributes;
@ -48,13 +44,28 @@ IFACEMETHODIMP V8LocalVariables::GetValue(PCWSTR key, IModelObject* context,
if (v8_module == nullptr) {
// Anything in a module must not be in the V8 module if the V8 module
// doesn't exist.
return original_hr;
return original_->GetValue(key, context, value);
}
Location v8_base;
RETURN_IF_FAIL(v8_module->GetBaseLocation(&v8_base));
if (module_base != v8_base) {
// It's in a module, but not the one that contains V8.
return original_hr;
return original_->GetValue(key, context, value);
}
// Next, determine whether the instruction pointer refers to a builtin.
DCHECK_GE(instruction_offset, v8_base.GetOffset());
ULONG64 rva = instruction_offset - v8_base.GetOffset();
WRL::ComPtr<IDebugHostSymbol> symbol;
_bstr_t symbol_name;
WRL::ComPtr<IDebugHostModule2> module2;
RETURN_IF_FAIL(module.As(&module2));
ULONG64 offset_within_symbol{};
if (FAILED(module2->FindContainingSymbolByRVA(rva, &symbol,
&offset_within_symbol)) ||
FAILED(symbol->GetName(symbol_name.GetAddress())) ||
strncmp("Builtins_", static_cast<const char*>(symbol_name),
strlen("Builtins_"))) {
return original_->GetValue(key, context, value);
}
}
@ -98,7 +109,7 @@ IFACEMETHODIMP V8LocalVariables::GetValue(PCWSTR key, IModelObject* context,
if (object_type == nullptr) {
// There's nothing useful to do if we can't find the symbol for
// v8::internal::Object.
return original_hr;
return original_->GetValue(key, context, value);
}
ULONG64 object_size{};
RETURN_IF_FAIL(object_type->GetSize(&object_size));