Created attachment 8559648 [details] Test case The debugger stops twice at a breakpoint on a while loop when the script execution is continued. STR: 1. Open the DevTools Debugger (Ctrl+Shift+S) on attached test case 2. Set a breakpoint at the while loop (line 3) 3. Reload the page => The debugger stops at line 3. 4. Continue the script execution (F8) => The debugger stops again at line 3, which should not happen. Stepping after the first hit works as expected, though. Sebastian
I think this is probably a dupe of bug 1003554, where we're tackling a bunch of these issues.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1003554
Hmm, I'm not sure this is really the same as it just happens when you continue the script execution. It does not occur when single-stepping through the code. Though I'll check again as soon as bug 1003554 is fixed. Sebastian
I'm un-duping this since it isn't exactly like the problems being addressed in bug 1003554. The deal here is that the entry point computation seems to make perfect sense; it is just weird for users :( Here's the disassembly: loc op ----- -- main: 00000: goto 6 (+6) 00005: loophead 00006: loopentry 129 00008: false 00009: ifne 5 (-4) 00014: getgname "console" 00019: dup 00020: callprop "log" 00025: swap 00026: string "hi" 00031: call 1 00034: pop 00035: retrval Source notes: ofs line pc delta desc args ---- ---- ----- ------ -------- ------ 0: 1 0 [ 0] while offset 9 2: 1 5 [ 5] newline 3: 2 5 [ 0] colspan 4 5: 2 6 [ 1] setline lineno 1 7: 1 6 [ 0] colspan 6 9: 1 14 [ 8] xdelta 10: 1 14 [ 0] newline 11: 2 14 [ 0] colspan 5 13: 2 14 [ 0] newline 14: 3 35 [ 21] xdelta 15: 3 35 [ 0] colspan 18 What happens here is that line 1 gets two entry points: PC=0, the loopentry instruction; and PC=6, the start of the condition computation. Maybe one fix would be to change EmitWhile to *not* give a line number to that initial "goto". Then only the condition computation would show up as an entry point.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
> Maybe one fix would be to change EmitWhile to *not* give a line number to > that initial "goto". Then only the condition computation would show up > as an entry point. There doesn't seem to be a clean way to do this. The line note is emitted by EmitTree, not EmitWhile, so this plan would involve a special case there. I think bug also happens for: for (; false; ) ;
One more thing I realized today is that a fix to somehow ignore this leading "goto" is going to run into: https://bugzilla.mozilla.org/show_bug.cgi?id=1003554#c38
Created attachment 8563650 [details] [diff] [review] special-case "while" and "for" line notes Here's what the resulting patch looks like. This works for all the cases I've tried, and passes the "debug" jit-tests. I'm not as sure as I was yesterday that this approach is so very ugly. Well, it is ugly, I just think alternatives will be equivalent. My reasoning is this: The most principled way to deal with this bug and the associated bugs would be to have the bytecode emitter generate a special table of entry points. However, I think this would effectively mean putting a call to construct this table at all the spots that currently update the line table. And then, to deal with the "while" and "for" issue, it would need a hack just like the hack in this patch.
Created attachment 8588046 [details] [diff] [review] special-case "while" and "for" line notes Rebased and updated for parser changes.
Attachment #8563650 - Attachment is obsolete: true
Created attachment 8668027 [details] [diff] [review] special-case "while" and "for" line notes Rebased.
Created attachment 8669047 [details] [diff] [review] special-case "while" and "for" line notes Move operator to end of line.
Attachment #8668027 - Attachment is obsolete: true
Created attachment 8677554 [details] [diff] [review] special-case "while" and "for" line notes Rebased & updated.
Attachment #8669047 - Attachment is obsolete: true
Created attachment 8684942 [details] [diff] [review] special-case "while" and "for" line notes Rebased.
Attachment #8677554 - Attachment is obsolete: true
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago → 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Works for me, thank you! Sebastian
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.