Commit f1ddb2fa authored by Benedikt Meurer's avatar Benedikt Meurer Committed by V8 LUCI CQ

[inspector] Account for dynamic nature of the `fn.name` property.

With https://crrev.com/c/3272577 we introduced a `StackFrame` cache for
the inspector, which is keyed on the script ID, line and column number,
so the syntactic properties of the function. However, the name that we
report for functions is dynamic and can change (per closure) by
explicitly reconfiguring the "name" property via

```js
var f = function() { /* ... */ }
Object.defineProperty(f, "name", {value: "super duper function"});
```

for example, so we need to take that into account as well, and only use
the cached `StackFrame` instance if the dynamic names still match up.
Otherwise we just overwrite the cached entry with a new instance (the
assumption here is that "name" isn't reconfigured often).

Fixed: chromium:1274529
Bug: chromium:1268436
Change-Id: I519017c762aed5b4f93b9dc4553fa81d5979f1a1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3306376
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78127}
parent 97b89b6a
...@@ -1146,18 +1146,18 @@ std::shared_ptr<StackFrame> V8Debugger::symbolize( ...@@ -1146,18 +1146,18 @@ std::shared_ptr<StackFrame> V8Debugger::symbolize(
int lineNumber = v8Frame->GetLineNumber() - 1; int lineNumber = v8Frame->GetLineNumber() - 1;
int columnNumber = v8Frame->GetColumn() - 1; int columnNumber = v8Frame->GetColumn() - 1;
CachedStackFrameKey key{scriptId, lineNumber, columnNumber}; CachedStackFrameKey key{scriptId, lineNumber, columnNumber};
auto functionName =
toProtocolString(isolate(), v8::debug::GetFunctionDebugName(v8Frame));
auto it = m_cachedStackFrames.find(key); auto it = m_cachedStackFrames.find(key);
if (it != m_cachedStackFrames.end() && !it->second.expired()) { if (it != m_cachedStackFrames.end() && !it->second.expired()) {
auto stackFrame = it->second.lock(); auto stackFrame = it->second.lock();
if (stackFrame->functionName() == functionName) {
DCHECK_EQ( DCHECK_EQ(
stackFrame->functionName(), stackFrame->sourceURL(),
toProtocolString(isolate(), v8::debug::GetFunctionDebugName(v8Frame)));
DCHECK_EQ(stackFrame->sourceURL(),
toProtocolString(isolate(), v8Frame->GetScriptNameOrSourceURL())); toProtocolString(isolate(), v8Frame->GetScriptNameOrSourceURL()));
return stackFrame; return stackFrame;
} }
auto functionName = }
toProtocolString(isolate(), v8::debug::GetFunctionDebugName(v8Frame));
auto sourceURL = auto sourceURL =
toProtocolString(isolate(), v8Frame->GetScriptNameOrSourceURL()); toProtocolString(isolate(), v8Frame->GetScriptNameOrSourceURL());
auto hasSourceURLComment = auto hasSourceURLComment =
......
Regression test for crbug.com/1274529
Running test: test
Calling foo() with dynamic name "foo"
foo (foo.js:0:23)
(anonymous) (:0:0)
Calling foo() with dynamic name "bar"
bar (foo.js:0:23)
(anonymous) (:0:0)
// 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.
const {contextGroup, Protocol, session} =
InspectorTest.start('Regression test for crbug.com/1274529');
const url = 'foo.js';
contextGroup.addScript('function foo(){console.trace();}', 0, 0, url);
session.setupScriptMap();
InspectorTest.runAsyncTestSuite([async function test() {
await Promise.all([Protocol.Runtime.enable(), Protocol.Debugger.enable()]);
Protocol.Runtime.onConsoleAPICalled(
({params: {stackTrace: {callFrames}}}) => {
session.logCallFrames(callFrames);
});
InspectorTest.logMessage('Calling foo() with dynamic name "foo"');
await Protocol.Runtime.evaluate({expression: 'foo()'});
await Protocol.Runtime.evaluate(
{expression: 'Object.defineProperty(foo, "name", {value: "bar"})'});
InspectorTest.logMessage('Calling foo() with dynamic name "bar"');
await Protocol.Runtime.evaluate({expression: 'foo()'});
await Promise.all([Protocol.Runtime.disable(), Protocol.Debugger.disable()]);
}]);
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment