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

[inspector] Avoid sliding breakpoints for same scripts

Browse Source 
We change the breakpoint hint logic to check if the script has not
locally changed (with a hash of the source text between the requested
breakpoint location and the actual breakpoint location). If the
text did not change, we set the breakpoint at the same
location as before.

Bug: chromium:1404643
Change-Id: I6ceecf9924e699aaf37518680d1cb79d3eb00959
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4138260
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#85131}
This commit is contained in:
Jaroslav Sevcik 2023-01-05 17:30:33 +01:00 committed by V8 LUCI CQ
parent ebd933037e
commit ff2b5a6729
7 changed files with 242 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
BUILD.bazel
View File

@ -3255,6 +3255,8 @@ filegroup(
filegroup(
name = "v8_inspector_files",
srcs = [
"src/inspector/crc32.cc",
"src/inspector/crc32.h",
"src/inspector/custom-preview.cc",
"src/inspector/custom-preview.h",
"src/inspector/injected-script.cc",

2
src/inspector/BUILD.gn
View File

@ -102,6 +102,8 @@ v8_source_set("inspector") {
"../../include/v8-inspector.h",
]
sources += [
"crc32.cc",
"crc32.h",
"custom-preview.cc",
"custom-preview.h",
"injected-script.cc",

85
src/inspector/crc32.cc Normal file
View File

@ -0,0 +1,85 @@
// Copyright 2023 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.
#include "src/inspector/crc32.h"
#include "src/base/macros.h"
namespace v8_inspector {
// Generated from the polynomial 0xedb88320 using the following script:
// for i in range(0, 256):
// c = i ^ 0xff
// for j in range(0, 8):
// l = 0 if c & 1 else 0xedb88320
// c = (c >> 1) ^ l
// print("0x%x" % (c))
static uint32_t kCrcTable[256] = {
0x0L, 0x77073096L, 0xee0e612cL, 0x990951baL, 0x76dc419L,
0x706af48fL, 0xe963a535L, 0x9e6495a3L, 0xedb8832L, 0x79dcb8a4L,
0xe0d5e91eL, 0x97d2d988L, 0x9b64c2bL, 0x7eb17cbdL, 0xe7b82d07L,
0x90bf1d91L, 0x1db71064L, 0x6ab020f2L, 0xf3b97148L, 0x84be41deL,
0x1adad47dL, 0x6ddde4ebL, 0xf4d4b551L, 0x83d385c7L, 0x136c9856L,
0x646ba8c0L, 0xfd62f97aL, 0x8a65c9ecL, 0x14015c4fL, 0x63066cd9L,
0xfa0f3d63L, 0x8d080df5L, 0x3b6e20c8L, 0x4c69105eL, 0xd56041e4L,
0xa2677172L, 0x3c03e4d1L, 0x4b04d447L, 0xd20d85fdL, 0xa50ab56bL,
0x35b5a8faL, 0x42b2986cL, 0xdbbbc9d6L, 0xacbcf940L, 0x32d86ce3L,
0x45df5c75L, 0xdcd60dcfL, 0xabd13d59L, 0x26d930acL, 0x51de003aL,
0xc8d75180L, 0xbfd06116L, 0x21b4f4b5L, 0x56b3c423L, 0xcfba9599L,
0xb8bda50fL, 0x2802b89eL, 0x5f058808L, 0xc60cd9b2L, 0xb10be924L,
0x2f6f7c87L, 0x58684c11L, 0xc1611dabL, 0xb6662d3dL, 0x76dc4190L,
0x1db7106L, 0x98d220bcL, 0xefd5102aL, 0x71b18589L, 0x6b6b51fL,
0x9fbfe4a5L, 0xe8b8d433L, 0x7807c9a2L, 0xf00f934L, 0x9609a88eL,
0xe10e9818L, 0x7f6a0dbbL, 0x86d3d2dL, 0x91646c97L, 0xe6635c01L,
0x6b6b51f4L, 0x1c6c6162L, 0x856530d8L, 0xf262004eL, 0x6c0695edL,
0x1b01a57bL, 0x8208f4c1L, 0xf50fc457L, 0x65b0d9c6L, 0x12b7e950L,
0x8bbeb8eaL, 0xfcb9887cL, 0x62dd1ddfL, 0x15da2d49L, 0x8cd37cf3L,
0xfbd44c65L, 0x4db26158L, 0x3ab551ceL, 0xa3bc0074L, 0xd4bb30e2L,
0x4adfa541L, 0x3dd895d7L, 0xa4d1c46dL, 0xd3d6f4fbL, 0x4369e96aL,
0x346ed9fcL, 0xad678846L, 0xda60b8d0L, 0x44042d73L, 0x33031de5L,
0xaa0a4c5fL, 0xdd0d7cc9L, 0x5005713cL, 0x270241aaL, 0xbe0b1010L,
0xc90c2086L, 0x5768b525L, 0x206f85b3L, 0xb966d409L, 0xce61e49fL,
0x5edef90eL, 0x29d9c998L, 0xb0d09822L, 0xc7d7a8b4L, 0x59b33d17L,
0x2eb40d81L, 0xb7bd5c3bL, 0xc0ba6cadL, 0xedb88320L, 0x9abfb3b6L,
0x3b6e20cL, 0x74b1d29aL, 0xead54739L, 0x9dd277afL, 0x4db2615L,
0x73dc1683L, 0xe3630b12L, 0x94643b84L, 0xd6d6a3eL, 0x7a6a5aa8L,
0xe40ecf0bL, 0x9309ff9dL, 0xa00ae27L, 0x7d079eb1L, 0xf00f9344L,
0x8708a3d2L, 0x1e01f268L, 0x6906c2feL, 0xf762575dL, 0x806567cbL,
0x196c3671L, 0x6e6b06e7L, 0xfed41b76L, 0x89d32be0L, 0x10da7a5aL,
0x67dd4accL, 0xf9b9df6fL, 0x8ebeeff9L, 0x17b7be43L, 0x60b08ed5L,
0xd6d6a3e8L, 0xa1d1937eL, 0x38d8c2c4L, 0x4fdff252L, 0xd1bb67f1L,
0xa6bc5767L, 0x3fb506ddL, 0x48b2364bL, 0xd80d2bdaL, 0xaf0a1b4cL,
0x36034af6L, 0x41047a60L, 0xdf60efc3L, 0xa867df55L, 0x316e8eefL,
0x4669be79L, 0xcb61b38cL, 0xbc66831aL, 0x256fd2a0L, 0x5268e236L,
0xcc0c7795L, 0xbb0b4703L, 0x220216b9L, 0x5505262fL, 0xc5ba3bbeL,
0xb2bd0b28L, 0x2bb45a92L, 0x5cb36a04L, 0xc2d7ffa7L, 0xb5d0cf31L,
0x2cd99e8bL, 0x5bdeae1dL, 0x9b64c2b0L, 0xec63f226L, 0x756aa39cL,
0x26d930aL, 0x9c0906a9L, 0xeb0e363fL, 0x72076785L, 0x5005713L,
0x95bf4a82L, 0xe2b87a14L, 0x7bb12baeL, 0xcb61b38L, 0x92d28e9bL,
0xe5d5be0dL, 0x7cdcefb7L, 0xbdbdf21L, 0x86d3d2d4L, 0xf1d4e242L,
0x68ddb3f8L, 0x1fda836eL, 0x81be16cdL, 0xf6b9265bL, 0x6fb077e1L,
0x18b74777L, 0x88085ae6L, 0xff0f6a70L, 0x66063bcaL, 0x11010b5cL,
0x8f659effL, 0xf862ae69L, 0x616bffd3L, 0x166ccf45L, 0xa00ae278L,
0xd70dd2eeL, 0x4e048354L, 0x3903b3c2L, 0xa7672661L, 0xd06016f7L,
0x4969474dL, 0x3e6e77dbL, 0xaed16a4aL, 0xd9d65adcL, 0x40df0b66L,
0x37d83bf0L, 0xa9bcae53L, 0xdebb9ec5L, 0x47b2cf7fL, 0x30b5ffe9L,
0xbdbdf21cL, 0xcabac28aL, 0x53b39330L, 0x24b4a3a6L, 0xbad03605L,
0xcdd70693L, 0x54de5729L, 0x23d967bfL, 0xb3667a2eL, 0xc4614ab8L,
0x5d681b02L, 0x2a6f2b94L, 0xb40bbe37L, 0xc30c8ea1L, 0x5a05df1bL,
0x2d02ef8dL};
int32_t computeCrc32(const String16& text) {
const uint8_t* bytes = reinterpret_cast<const uint8_t*>(text.characters16());
size_t byteLength = sizeof(UChar) * text.length();
uint32_t checksum = 0;
for (size_t i = 0; i < byteLength; ++i) {
uint32_t index = (checksum ^ bytes[i]) & 0xff;
checksum = (checksum >> 8) ^ kCrcTable[index];
}
return v8::base::bit_cast<int32_t>(checksum);
}
} // namespace v8_inspector

16
src/inspector/crc32.h Normal file
View File

@ -0,0 +1,16 @@
// Copyright 2023 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.
#ifndef V8_INSPECTOR_CRC32_H_
#define V8_INSPECTOR_CRC32_H_
#include "src/inspector/string-16.h"
namespace v8_inspector {
int32_t computeCrc32(const String16&);
}
#endif // V8_INSPECTOR_CRC32_H_

111
src/inspector/v8-debugger-agent-impl.cc
View File

@ -14,6 +14,7 @@
#include "include/v8-microtask-queue.h"
#include "src/base/safe_conversions.h"
#include "src/debug/debug-interface.h"
#include "src/inspector/crc32.h"
#include "src/inspector/injected-script.h"
#include "src/inspector/inspected-context.h"
#include "src/inspector/protocol/Debugger.h"
@ -55,6 +56,9 @@ static const char breakpointsByRegex[] = "breakpointsByRegex";
static const char breakpointsByUrl[] = "breakpointsByUrl";
static const char breakpointsByScriptHash[] = "breakpointsByScriptHash";
static const char breakpointHints[] = "breakpointHints";
static const char breakpointHintText[] = "text";
static const char breakpointHintPrefixHash[] = "prefixHash";
static const char breakpointHintPrefixLength[] = "prefixLen";
static const char instrumentationBreakpoints[] = "instrumentationBreakpoints";
} // namespace DebuggerAgentState
@ -179,23 +183,46 @@ bool positionComparator(const std::pair<int, int>& a,
return a.second < b.second;
}
String16 breakpointHint(const V8DebuggerScript& script, int lineNumber,
int columnNumber) {
int offset;
if (!script.offset(lineNumber, columnNumber).To(&offset)) return String16();
std::unique_ptr<protocol::DictionaryValue> breakpointHint(
const V8DebuggerScript& script, int breakpointLineNumber,
int breakpointColumnNumber, int actualLineNumber, int actualColumnNumber) {
int actualOffset;
int breakpointOffset;
if (!script.offset(actualLineNumber, actualColumnNumber).To(&actualOffset) ||
!script.offset(breakpointLineNumber, breakpointColumnNumber)
.To(&breakpointOffset)) {
return {};
}
auto hintObject = protocol::DictionaryValue::create();
String16 hint =
script.source(offset, kBreakpointHintMaxLength).stripWhiteSpace();
script.source(actualOffset, kBreakpointHintMaxLength).stripWhiteSpace();
for (size_t i = 0; i < hint.length(); ++i) {
if (hint[i] == '\r' || hint[i] == '\n' || hint[i] == ';') {
return hint.substring(0, i);
hint = hint.substring(0, i);
break;
}
}
return hint;
hintObject->setString(DebuggerAgentState::breakpointHintText, hint);
// Also store the hash of the text between the requested breakpoint location
// and the actual breakpoint location. If we see the same prefix text next
// time, we will keep the breakpoint at the same location (so that
// breakpoints do not slide around on reloads without any edits).
if (breakpointOffset <= actualOffset) {
size_t length = actualOffset - breakpointOffset;
String16 prefix = script.source(breakpointOffset, length);
int crc32 = computeCrc32(prefix);
hintObject->setInteger(DebuggerAgentState::breakpointHintPrefixHash, crc32);
hintObject->setInteger(DebuggerAgentState::breakpointHintPrefixLength,
v8::base::checked_cast<int32_t>(length));
}
return hintObject;
}
void adjustBreakpointLocation(const V8DebuggerScript& script,
const String16& hint, int* lineNumber,
int* columnNumber) {
const protocol::DictionaryValue* hintObject,
int* lineNumber, int* columnNumber) {
if (*lineNumber < script.startLine() || *lineNumber > script.endLine())
return;
if (*lineNumber == script.startLine() &&
@ -206,15 +233,41 @@ void adjustBreakpointLocation(const V8DebuggerScript& script,
return;
}
if (hint.isEmpty()) return;
int sourceOffset;
if (!script.offset(*lineNumber, *columnNumber).To(&sourceOffset)) return;
int prefixLength = 0;
hintObject->getInteger(DebuggerAgentState::breakpointHintPrefixLength,
&prefixLength);
String16 hint;
if (!hintObject->getString(DebuggerAgentState::breakpointHintText, &hint) ||
hint.isEmpty())
return;
intptr_t searchRegionOffset = std::max(
sourceOffset - kBreakpointHintMaxSearchOffset, static_cast<intptr_t>(0));
size_t offset = sourceOffset - searchRegionOffset;
String16 searchArea = script.source(searchRegionOffset,
offset + kBreakpointHintMaxSearchOffset);
size_t searchRegionSize =
offset + std::max(kBreakpointHintMaxSearchOffset,
static_cast<intptr_t>(prefixLength + hint.length()));
String16 searchArea = script.source(searchRegionOffset, searchRegionSize);
// Let us see if the breakpoint hint text appears at the same location
// as before, with the same prefix text in between. If yes, then we just use
// that position.
int prefixHash;
if (hintObject->getInteger(DebuggerAgentState::breakpointHintPrefixHash,
&prefixHash) &&
offset + prefixLength + hint.length() <= searchArea.length() &&
searchArea.substring(offset + prefixLength, hint.length()) == hint &&
computeCrc32(searchArea.substring(offset, prefixLength)) == prefixHash) {
v8::debug::Location hintPosition =
script.location(static_cast<int>(offset + prefixLength));
*lineNumber = hintPosition.GetLineNumber();
*columnNumber = hintPosition.GetColumnNumber();
return;
}
size_t nextMatch = searchArea.find(hint, offset);
size_t prevMatch = searchArea.reverseFind(hint, offset);
@ -222,7 +275,8 @@ void adjustBreakpointLocation(const V8DebuggerScript& script,
return;
}
size_t bestMatch;
if (nextMatch == String16::kNotFound) {
if (nextMatch == String16::kNotFound ||
nextMatch > kBreakpointHintMaxSearchOffset) {
bestMatch = prevMatch;
} else if (prevMatch == String16::kNotFound) {
bestMatch = nextMatch;
@ -588,26 +642,30 @@ Response V8DebuggerAgentImpl::setBreakpointByUrl(
"Breakpoint at specified location already exists.");
}
String16 hint;
std::unique_ptr<protocol::DictionaryValue> hint;
for (const auto& script : m_scripts) {
if (!matches(m_inspector, *script.second, type, selector)) continue;
if (!hint.isEmpty()) {
adjustBreakpointLocation(*script.second, hint, &lineNumber,
&columnNumber);
int adjustedLineNumber = lineNumber;
int adjustedColumnNumber = columnNumber;
if (hint) {
adjustBreakpointLocation(*script.second, hint.get(), &adjustedLineNumber,
&adjustedColumnNumber);
}
std::unique_ptr<protocol::Debugger::Location> location = setBreakpointImpl(
breakpointId, script.first, condition, lineNumber, columnNumber);
std::unique_ptr<protocol::Debugger::Location> location =
setBreakpointImpl(breakpointId, script.first, condition,
adjustedLineNumber, adjustedColumnNumber);
if (location && type != BreakpointType::kByUrlRegex) {
hint = breakpointHint(*script.second, location->getLineNumber(),
location->getColumnNumber(columnNumber));
hint = breakpointHint(*script.second, lineNumber, columnNumber,
location->getLineNumber(),
location->getColumnNumber(adjustedColumnNumber));
}
if (location) (*locations)->emplace_back(std::move(location));
}
breakpoints->setString(breakpointId, condition);
if (!hint.isEmpty()) {
if (hint) {
protocol::DictionaryValue* breakpointHints =
getOrCreateObject(m_state, DebuggerAgentState::breakpointHints);
breakpointHints->setString(breakpointId, hint);
breakpointHints->setObject(breakpointId, std::move(hint));
}
*outBreakpointId = breakpointId;
return Response::Success();
@ -1892,10 +1950,9 @@ void V8DebuggerAgentImpl::didParseSource(
if (!matches(m_inspector, *scriptRef, type, selector)) continue;
String16 condition;
breakpointWithCondition.second->asString(&condition);
String16 hint;
bool hasHint =
breakpointHints && breakpointHints->getString(breakpointId, &hint);
if (hasHint) {
protocol::DictionaryValue* hint =
breakpointHints ? breakpointHints->getObject(breakpointId) : nullptr;
if (hint) {
adjustBreakpointLocation(*scriptRef, hint, &lineNumber, &columnNumber);
}
std::unique_ptr<protocol::Debugger::Location> location =

36
test/inspector/debugger/restore-breakpoint-expected.txt
View File

@ -8,6 +8,42 @@ function foo() {
#boo();
}
Running test: testSameSourceDuplicateLines
function foo() {
boo();
// something
#boo();
}
function foo() {
boo();
// something
#boo();
}
Running test: testSameSourceDuplicateLinesLongLineBetween
function foo() {
boo();
/////////////////////////////////////////////////////////////////////////////...
#boo();
}
function foo() {
boo();
/////////////////////////////////////////////////////////////////////////////...
#boo();
}
Running test: testSameSourceDuplicateLinesDifferentPrefix
function foo() {
boo();
// something
#boo();
}
function foo() {
#boo();
// somethinX
boo();
}
Running test: testOneLineOffset
function foo() {
#boo();

17
test/inspector/debugger/restore-breakpoint.js
View File

@ -12,6 +12,23 @@ InspectorTest.runTestSuite([
test(source, source, { lineNumber: 1, columnNumber: 0 }, next);
},
function testSameSourceDuplicateLines(next) {
var source = 'function foo() {\nboo();\n// something\nboo();\n}';
test(source, source, { lineNumber: 2, columnNumber: 0 }, next);
},
function testSameSourceDuplicateLinesLongLineBetween(next) {
var longComment = '/'.repeat(1e4);
var source = `function foo() {\nboo();\n${longComment}\nboo();\n}`;
test(source, source, { lineNumber: 2, columnNumber: 0 }, next);
},
function testSameSourceDuplicateLinesDifferentPrefix(next) {
var source = 'function foo() {\nboo();\n// something\nboo();\n}';
var newSource = 'function foo() {\nboo();\n// somethinX\nboo();\n}';
test(source, newSource, { lineNumber: 2, columnNumber: 0 }, next);
},
function testOneLineOffset(next) {
var source = 'function foo() {\nboo();\n}';
var newSource = 'function foo() {\nboo();\nboo();\n}';