1370655 - Debugger steps in only once at while loop

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Daniel Lehmann]

Attachment: example.html

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20170524174609

Steps to reproduce:

1. Open attached example.html in Firefox 53.0 or Firefox Developer Edition 54.0 beta12 (both affected).
2. Open Developer Tools, go to Debugger.
3. Set breakpoints in line 7 and line 12 (while loops)
4. Reload to hit first breakpoint.
5. Click "Step In" once.


Actual results:

- Debugger pauses at line 9 (past while loop) after only one step in.
- Confusing, not clear whether all iterations of loop were executed.


Expected results:

- Debugger should pause again at line 7 (where the loop is), at least once per iteration (that is, three times in this case).
- NOTE: How many steps are necessary to get past loop depends on whitespace/formatting. The while-loop in line 12 can be "stepped in" seven times before execution pauses after the loop. See attached screencapture.
- Possibly related bug with for-loops: https://bugzilla.mozilla.org/show_bug.cgi?id=1363325

Comment 1

[Daniel Lehmann]

Attachment: screencapture.ogv

Comment 2

[Tom Tromey :tromey]

I believe this is the (historically) intended behavior -- single-line loops are treated as a single statement.
There are tests checking this.

That said, I'm not sure why it was done this way, or whether it makes sense.
It seems to me that the new debugger's ability to have column breakpoints
(not sure if there is column stepping or not) plus the (coming?) ability to 
do things like "continue to here" means that there aren't many advantages to
this special case.

Comment 3

[Daniel Lehmann]

Thanks for the quick comment. 

Mhm, interesting that it was intended before, I would have (naively) assumed the number of steps for any construct should never depend on the whitespace/formatting of that construct.

One additional data point for the "expected debugger behavior" from the user's view: Chrome takes the same number of steps in both cases, i.e. it is independent of whitespace/formatting.

Comment 4

[Tom Tromey :tromey]

SpiderMonkey probably should be emitting a source note for the loop body here.
It currently doesn't (see below).

However, even if it did, this wouldn't be visible until column stepping is implemented anyway.
The current stepping is line-based -- step just single-steps instructions until the line number
changes, but since the whole loop is on a single line, this doesn't happen.

Here's the bytecode.
I think there should be a column note at PC=7.

js> function f()  {
      while (i > 0) { i--; } // set breakpoint here, then step in
}
js> dis(f)
flags: CONSTRUCTOR
loc     op
-----   --
main:
00000:  jumptarget                      # 
00001:  goto 31 (+30)                   # 

# from ifne @ 00040
00006:  loophead                        # 
00007:  bindgname "i"                   # GLOBAL
00012:  getgname "i"                    # GLOBAL i
00017:  pos                             # GLOBAL (+ i)
00018:  dup                             # GLOBAL (+ i) (+ i)
00019:  one                             # GLOBAL (+ i) (+ i) 1
00020:  sub                             # GLOBAL (+ i) ((+ i) - 1)
00021:  pick 2                          # (+ i) ((+ i) - 1) GLOBAL
00023:  swap                            # (+ i) GLOBAL ((+ i) - 1)
00024:  setgname "i"                    # (+ i) ((+ i) - 1)
00029:  pop                             # (+ i)
00030:  pop                             # 

# from goto @ 00001
00031:  loopentry 129                   # 
00033:  getgname "i"                    # i
00038:  zero                            # i 0
00039:  gt                              # (i > 0)
00040:  ifne 6 (-34)                    # 
00045:  jumptarget                      # 
00046:  retrval                         # 

Source notes:
 ofs line    pc  delta desc     args
---- ---- ----- ------ -------- ------
  0:    1     0 [   0] newline 
  1:    2     0 [   0] colspan 6
  3:    2     1 [   1] while    offset 39
  5:    2     6 [   5] colspan 16
  7:    2    31 [  25] xdelta  
  8:    2    31 [   0] colspan -9
 13:    2    46 [  15] xdelta  
 14:    2    46 [   0] newline 

Exception table:
kind               stack    start      end
 loop                  0        6       45

Comment 5

[Jason Laster [:jlast]]

Logan, could you try reproducing?

Comment 6

[Logan Smyth [:loganfsmyth]]

This is all set and steps through the loop properly now.