Closed Bug 1296441 Opened 8 years ago Closed 8 years ago

debugger steps entirely over an "if"

Categories

(DevTools :: Debugger, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1263355

People

(Reporter: tromey, Assigned: tromey)

Details

Attachments

(2 files)

Attached file step-bug.html (deleted) —
Consider the attached file. Set a breakpoint on the first line of "C". Then in the console invoke "bug()". Now single step (step in or step over, doesn't matter). When stepping through the loop, the debugger never stops on the "if". However, it ought to.
(In reply to Tom Tromey :tromey from comment #0) > Created attachment 8782649 [details] > step-bug.html > > Consider the attached file. > > Set a breakpoint on the first line of "C". > Then in the console invoke "bug()". > > Now single step (step in or step over, doesn't matter). > When stepping through the loop, the debugger never stops on the "if". > However, it ought to. Any idea why this happens Tom? My guess would be a bug in how we compute the entry points for the given line, but perhaps I'm wrong.
Priority: -- → P2
Attached file step.js (deleted) —
If you run this test case using 'js' (you need a debug build for "dis"), you'll see from the dump at the end that line 7 (the "if") doesn't have an entry point: 4 => 14 5 => 15 6 => 26,130 8 => 95 10 => 113 12 => 162 13 => 167 So probably a bug in the entry point computation in SpiderMonkey.
I'll append the disassembly. The bug happens because when we see the loophead instruction at offset 64, we enter this: https://dxr.mozilla.org/mozilla-central/source/js/src/vm/Debugger.cpp#5811 However, this computes lineno=-1, oops. I think this confuses later processing, specifically for offset=66, which is the "if" in question. flags: NEEDS_CALLOBJECT CONSTRUCTOR loc op ----- -- 00000: arguments 00001: setaliasedvar "arguments" (hops = 0, slot = 3) 00006: pop 00007: functionthis 00008: setaliasedvar ".this" (hops = 0, slot = 4) 00013: pop main: 00014: debugger 00015: getgname "val" 00020: initaliasedlexical "parent" (hops = 0, slot = 5) 00025: pop 00026: pushblockscope depth 0 {name: 0} 00031: uninitialized 00032: initaliasedlexical "name" (hops = 0, slot = 2) 00037: pop 00038: undefined 00039: initaliasedlexical "name" (hops = 0, slot = 2) 00044: pop 00045: getaliasedvar "nl" (hops = 1, slot = 2) 00050: dup 00051: symbol 1 00053: callelem 00054: swap 00055: calliter 0 00058: undefined 00059: goto 130 (+71) 00064: loophead 00065: dup 00066: getprop "value" 00071: setaliasedvar "name" (hops = 0, slot = 2) 00076: pop 00077: getaliasedvar "parent" (hops = 1, slot = 5) 00082: getaliasedvar "name" (hops = 0, slot = 2) 00087: getelem 00088: not 00089: ifeq 112 (+23) 00094: jumptarget 00095: getaliasedvar "parent" (hops = 1, slot = 5) 00100: getaliasedvar "name" (hops = 0, slot = 2) 00105: string "done" 00110: setelem 00111: pop 00112: jumptarget 00113: getaliasedvar "parent" (hops = 1, slot = 5) 00118: getaliasedvar "name" (hops = 0, slot = 2) 00123: getelem 00124: setaliasedvar "parent" (hops = 1, slot = 5) 00129: pop 00130: loopentry 129 00132: pop 00133: dup 00134: dup 00135: callprop "next" 00140: swap 00141: call 0 00144: checkisobj 0 00146: dup 00147: getprop "done" 00152: ifeq 64 (-88) 00157: jumptarget 00158: popn 2 00161: popblockscope 00162: getaliasedvar "parent" (hops = 0, slot = 5) 00167: return 00168: retrval Source notes: ofs line pc delta desc args ---- ---- ----- ------ -------- ------ 0: 3 7 [ 7] xdelta 1: 3 14 [ 7] newline 2: 4 14 [ 0] colspan 2 4: 4 15 [ 1] newline 5: 5 15 [ 0] colspan 6 7: 5 26 [ 11] xdelta 8: 5 26 [ 0] newline 9: 6 38 [ 12] xdelta 10: 6 38 [ 0] colspan 11 12: 6 59 [ 21] xdelta 13: 6 59 [ 0] for-of closingjump 93 15: 6 77 [ 18] xdelta 16: 6 77 [ 0] newline 17: 7 77 [ 0] colspan 8 19: 7 89 [ 12] xdelta 20: 7 89 [ 0] if 21: 7 95 [ 6] newline 22: 8 95 [ 0] colspan 6 24: 8 113 [ 18] xdelta 25: 8 113 [ 0] setline lineno 10 27: 10 113 [ 0] colspan 4 29: 10 130 [ 17] xdelta 30: 10 130 [ 0] setline lineno 6 32: 6 130 [ 0] colspan 19 34: 6 141 [ 11] xdelta 35: 6 141 [ 0] colspan -17 40: 6 162 [ 21] xdelta 41: 6 162 [ 0] setline lineno 12 43: 12 162 [ 0] colspan 2 45: 12 167 [ 5] newline Exception table: kind stack start end for-of 2 64 157 Block table: index parent start end 0 (none) 31 162
This patch fixes the problem for me. I can write a test case, etc, but I wanted to ask :nbp about it first, since blame says he wrote the lines in question. diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index f5985d1..a3a5f47 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -5791,7 +5791,7 @@ class FlowGraphSummary { if (FlowsIntoNext(prevOp)) addEdge(prevLineno, prevColumn, r.frontOffset()); - if (BytecodeIsJumpTarget(op)) { + if (BytecodeIsJumpTarget(op) && !entries_[r.frontOffset()].hasNoEdges()) { lineno = entries_[r.frontOffset()].lineno(); column = entries_[r.frontOffset()].column(); }
Flags: needinfo?(nicolas.b.pierron)
(In reply to Tom Tromey :tromey from comment #4) > This patch fixes the problem for me. > I can write a test case, etc, but I wanted to ask :nbp about it first, > since blame says he wrote the lines in question. > > diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp > index f5985d1..a3a5f47 100644 > --- a/js/src/vm/Debugger.cpp > +++ b/js/src/vm/Debugger.cpp > @@ -5791,7 +5791,7 @@ class FlowGraphSummary { > if (FlowsIntoNext(prevOp)) > addEdge(prevLineno, prevColumn, r.frontOffset()); > > - if (BytecodeIsJumpTarget(op)) { > + if (BytecodeIsJumpTarget(op) && > !entries_[r.frontOffset()].hasNoEdges()) { > lineno = entries_[r.frontOffset()].lineno(); > column = entries_[r.frontOffset()].column(); > } Sorry for the delay. This patch looks good to me. (= r+) I forgot the case where the loop-head is the first instruction, and the loop entry is later in the bytecode, as this is here. Thus we have no yet registered any incoming edges for the JSOP_LOOPHEAD, but we should record that later. Adding this condition will continue the linear iteration over the bytecode taking the lineno & column from the predecessor, even if there is a discontinuity. I would have preferred to assert that we have incoming edges, but this would not work unless we switch to a different system such as using IonBuilder control flow graph creation.
Flags: needinfo?(nicolas.b.pierron)
Assignee: nobody → ttromey
Status: NEW → ASSIGNED
It turns out this was fixed by bug 1263355.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: