Skip to content

Commit

Permalink
Make column match fuzzy
Browse files Browse the repository at this point in the history
Summary:
We are currently very strict about breakpoint location matching. This
diff allows some fuzz in the column, but not in the line.

Changelog: [Internal] Setting Hermes breakpoints no longer requires exact column match

Reviewed By: avp

Differential Revision: D21343198

fbshipit-source-id: a59786a9d63f9fe1ed576835ed660ba3343affe1
  • Loading branch information
willholen authored and facebook-github-bot committed May 5, 2020
1 parent 292a2b1 commit 4e1a14b
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 6 deletions.
31 changes: 25 additions & 6 deletions lib/BCGen/HBC/DebugInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,22 +169,41 @@ OptValue<DebugSearchResult> DebugInfo::getAddressForLocation(

unsigned offset = start;

// We consider the best match for a location to be the first of:
// 1. Exact match
// 2. Exact line, largest column before the target
// 3. Exact line, any column
// If multiple, the lowest bytecode address wins in each category.
DebugSearchResult best(0, DebugOffsets::NO_OFFSET, 0, 0);

while (offset < end) {
FunctionDebugInfoDeserializer fdid(data_.getData(), offset);
while (auto loc = fdid.next()) {
uint32_t line = loc->line;
uint32_t column = loc->column;
if (line == targetLine &&
(!targetColumn.hasValue() || column == *targetColumn)) {
// Short-circuit on a precise match.
return DebugSearchResult(
fdid.getFunctionIndex(), loc->address, line, column);
if (line == targetLine) {
if (!targetColumn.hasValue() || column == *targetColumn) {
// Short-circuit on a precise match.
return DebugSearchResult(
fdid.getFunctionIndex(), loc->address, line, column);
}

if (best.bytecodeOffset == DebugOffsets::NO_OFFSET ||
(column <= *targetColumn &&
(best.column > *targetColumn || column > best.column))) {
best = DebugSearchResult(
fdid.getFunctionIndex(), loc->address, line, column);
}
}
}
offset = fdid.getOffset();
}

return llvm::None;
if (best.bytecodeOffset == DebugOffsets::NO_OFFSET) {
return llvm::None;
}

return best;
}

/// Read \p count variable names from \p offset into the variable name section
Expand Down
45 changes: 45 additions & 0 deletions test/debugger/fuzzy-break.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

// RUN: %hdb --break-at-start %s < %s.debug | %FileCheck --match-full-lines %s
// REQUIRES: debugger

function foo() {
print('first');
print('second'); print('third'); print("fourth");
print('fifth'); print('sixth');
}

function bar() {
foo();
}

bar();
// CHECK-LABEL: Break on script load {{.+}}

// column=1 is before all statements. Match the first.
// CHECK-NEXT: Set breakpoint 1 at {{.+}}:12:3

// column=22 is the middle of the 'third' print. Match that.
// CHECK-NEXT: Set breakpoint 2 at {{.+}}:13:20

// column=200 is after all statements. Match the last.
// CHECK-NEXT: Set breakpoint 3 at {{.+}}:14:24

// CHECK-NEXT: Continuing execution
// CHECK-NEXT: Break on breakpoint 1 in foo: {{.+}}:12:3
// CHECK-NEXT: Continuing execution
// CHECK-NEXT: first
// CHECK-NEXT: second
// CHECK-NEXT: Break on breakpoint 2 in foo: {{.+}}:13:20
// CHECK-NEXT: Continuing execution
// CHECK-NEXT: third
// CHECK-NEXT: fourth
// CHECK-NEXT: fifth
// CHECK-NEXT: Break on breakpoint 3 in foo: {{.+}}:14:24
// CHECK-NEXT: Continuing execution
// CHECK-NEXT: sixth
7 changes: 7 additions & 0 deletions test/debugger/fuzzy-break.js.debug
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
break 12 1
break 13 22
break 14 200
continue
continue
continue
continue
37 changes: 37 additions & 0 deletions test/debugger/fuzzy-lazy-oneline-break.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

// RUN: %hdb --lazy %s < %s.debug | %FileCheck --match-full-lines %s
// REQUIRES: debugger

// @nolint
function foo() { print('foo called'); /** * Some text to pad out the function so that it won't be eagerly compiled * for being too short. Lorem ipsum dolor sit amet, consectetur adipiscing * elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. */ function bar() { print('bar called'); /** * Some text to pad out the function so that it won't be eagerly compiled * for being too short. Lorem ipsum dolor sit amet, consectetur adipiscing * elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. */ } function baz() { print('baz called'); /** * Some text to pad out the function so that it won't be eagerly compiled * for being too short. Lorem ipsum dolor sit amet, consectetur adipiscing * elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. */ }; bar(); baz(); }

debugger;
foo();

/* This test verifies that you can set breakpoints in uncompiled
* functions, without fuzzy column matching accidentally resolving the
* breakpoint to the closest already-compiled instruction it can find.
* This comes up when debugging minified source with source maps.
*/

// CHECK: Break on 'debugger' statement in global: {{.*}}:14:1

// Breakpoint is in the middle of the 'print' for 'bar called'
// CHECK-NEXT: Set breakpoint 1 at {{.*}}:12:286
// Breakpoint is in the middle of the 'print' for 'baz called'
// CHECK-NEXT: Set breakpoint 2 at {{.*}}:12:556

// CHECK-NEXT: Continuing execution
// CHECK-NEXT: foo called
// CHECK-NEXT: Break on breakpoint 1 in bar: {{.*}}:12:286
// CHECK-NEXT: Continuing execution
// CHECK-NEXT: bar called
// CHECK-NEXT: Break on breakpoint 2 in baz: {{.*}}:12:556
// CHECK-NEXT: Continuing execution
// CHECK-NEXT: baz called
5 changes: 5 additions & 0 deletions test/debugger/fuzzy-lazy-oneline-break.js.debug
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
break 12 290
break 12 560
continue
continue
continue

0 comments on commit 4e1a14b

Please sign in to comment.