Closed
Bug 1296441
Opened 8 years ago
Closed 8 years ago
debugger steps entirely over an "if"
Categories
(DevTools :: Debugger, defect, P2)
DevTools
Debugger
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 1263355
People
(Reporter: tromey, Assigned: tromey)
Details
Attachments
(2 files)
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.
Comment 1•8 years ago
|
||
(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
Assignee | ||
Comment 2•8 years ago
|
||
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.
Assignee | ||
Comment 3•8 years ago
|
||
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
Assignee | ||
Comment 4•8 years ago
|
||
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)
Comment 5•8 years ago
|
||
(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 | ||
Updated•8 years ago
|
Assignee: nobody → ttromey
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•8 years ago
|
||
It turns out this was fixed by bug 1263355.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•