dolphinsmalltalk / Dolphin

Dolphin Smalltalk Core Image
MIT License
301 stars 58 forks source link

Process keeps running after Debugger step, return, etc. if calling frame immediately returns #1176

Closed daniels220 closed 1 year ago

daniels220 commented 1 year ago

Describe the bug When stepping through returning from a method, by explicit "Step Out", "Step Into"/"Over" at the end of the method, or "Return..." from the frame, if the frame being returned to is itself about to return, the debugger fails to adjust the IP so that a breakpoint is hit, and the process just continues execution.

To Reproduce

  1. Evaluate [self halt] value. in a workspace.
  2. Click any of the step operations (into/over/out), or Debug > Return... and supply a value.
  3. (For comparison:) Try the same after evaluating [self halt] value. 1 + 1..

Expected behavior The Debugger halts again in the outer DoIt frame.

Actual behavior In (2), the Debugger immediately vanishes, because the process continues execution until it terminates. Or, specifically with Step Into, in this case it does eventually break in the machinery of a subsequent #trigger:, by way of a step interrupt rather than hitting a breakpoint, but that's still wrong.

I have experienced this when debugging long-running processes, where the Debugger will instead sit frozen on-screen showing "Step (Into/Over/Out)" or "Return..." in the title bar.

(3) works as expected.

Please complete the following information):

Additional context I notice that, when the outer frame is made into a debug frame, there is a *Break bytecode right before the final return, but the mapping from regular to debug IP lands on the return instruction, bypassing the break. I see how this is the correct mapping for the top frame on the stack—you don't ever want the debugger halted with the IP pointing at a *Break, because then it will immediately break again without executing any code when stepping. But all lower frames on the stack, if they are debug frames, I think generally do want to have their IP pointing at a breakpoint, because if they are reached, something has already happened—some higher frame has returned—so we want the opportunity to break.

Normal method:

9   Return From Block
10  Special Send #value
11  Return

Debug method:

12  Return From Block
13  *Break
14  Special Send #value
15  *Break
16  Return 

Normal textMap:

{ ...
9 -> (1 to: 11).
10 -> (1 to: 17).
11 -> (18 to: 17)}

Debug textMap:

{...
12 -> (1 to: 11).
14 -> (1 to: 17).
16 -> (18 to: 17)}

Like I say, I guess this mapping is correct in general, but maybe when modifying a frame that is not the top frame, we should look back one bytecode and, if it is a breakpoint, set the IP there?

blairmcg commented 1 year ago

I believe this is a duplicate of the long-ago reported issue #281, although the additional context is useful. Shout if you disagree, otherwise I will close it as a Dup. I'm afraid the issue has never bothered me enough (or interested me enough) to get to the top of my priority stack.

daniels220 commented 1 year ago

Sorry, meant to respond to this sooner. Ah—I agree this is a duplicate, yes. I might take a stab at fixing it myself...need to do some investigating as to what the cases are. I vaguely recall a related issue where, after encountering a self halt, the next Step Over/Into "doesn't do anything"—it finishes execution of Object>>halt itself, but stops on the next breakpoint in the method that made the call, which looks like nothing has happened. So there may be a need to look ahead and skip an immediately-following breakpoint if one exists.

More generally, one possibility is that the mapping of regular -> debug IP needs to depend in practice on where the frame is in the stack and what debugger operation is being performed, rather than being a constant mapping based on the method. Though this could also be accomplished by after-the-fact manipulation by the debugger, which might involve less refactoring, or for that matter be clearer to read.

(It's worth noting that your suggested workaround of using Step Into still produces significantly incorrect behavior—it still returns at least one level too far in the call stack, and if there are a bunch of tail calls on the stack it may return many levels deep before another message send is encountered which triggers a step interrupt. While it does at least ensure that the debugged process does not simply return to normal execution, it's no less confusing and just as much a violation of expectations.)

blairmcg commented 1 year ago

Duplicate of #281