squeak-smalltalk / squeak-object-memory

Issues and assets related to the Squeak object memory.
https://bugs.squeak.org
MIT License
13 stars 1 forks source link

Debugger returnValue through unwind context unwinds too far #119

Open LinqLover opened 7 months ago

LinqLover commented 7 months ago

Steps to reproduce

Do it/print it:

[self error] ensure: [self halt]

In the debugger, yellow-click UndefinedObject>>DoIt (home), choose "return entered value", and enter any value.

Expected: Should land in UndefinedObject(Object)>>halt.
Actual: Land in Compiler>>evaluateCue:ifFail:logged: (but the process suspendedContext is not actually yet there).
Patch:

  Debugger>>returnValue
    "Force a return of a given value to the previous context!"

-   | previous selectedContext expression value |
+   | previous selectedContext expression value newContext |
    contextStackIndex = 0 ifTrue: [^Beeper beep].
    selectedContext := self selectedContext.
    expression := UIManager default request: 'Enter expression for return value:'.
    value := Compiler new 
                evaluate: expression
                in: selectedContext
                to: selectedContext receiver.
    previous := selectedContext sender.
-   self resetContext: previous.
-   interruptedProcess popTo: previous value: value
+   newContext := self
+       handleLabelUpdatesIn: [interruptedProcess popTo: previous value: value]
+       whenExecuting: previous.
+   self resetContext: newContext. "might differ from previous in case of error during unwinding"

With that change, let's continue and step out of [] in UndefinedObject>>DoIt. Whether you use step over or step through from UndefinedObject>>DoIt or step into all the details of the unwind stack, in any case the process ends up completely terminated before the debugger halts in Compiler>>evaluateCue:ifFail:logged: as expected, and you get anything like disabled stepping buttons in the debugger, a cannotReturn: error, or a nil DNU from the debugger.

Something seems to be wrong with the unwinding logic here. I checked: This is not a regression and already failed in 6.0 and 5.3.

@isCzech FYIO, just in case you're up for another round of unwinding fun. Otherwise, I might myself address this one day. :-)