Smalltalk › Squeak › Squeak - Dev

stepping over non local return in a protected block

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

16 messages Options

Nicolas Cellier

stepping over non local return in a protected block

When debugging things like this:

[^2] ensure: [Transcript cr; show: 'done'].

if we step into the protected block [^2], then step over ^2, we
incorrectly get a BlockCannotReturn.
In deeper stacks, I saw some weirder behaviour
(when debugging test03EventHandler I put a halt into
m on: #mouseDown send: #value: to:[:evt|
self halt.
evt redButtonPressed ifTrue:[m color: Color red].
evt yellowButtonPressed ifTrue:[m color: Color yellow].
evt blueButtonPressed ifTrue:[m color: Color blue]].
and stepped over such a [^...] ensure: - result is very hard to
understand/trace)

I'm pretty sure this has been thoroughly investigated and discussed
during the last year or so (I remember of some infinite debugger
thread and some hard work of Christoph).

After the integration of many fixes in exception handling, process
termination, unwinding, and simulation machinery, thanks to Jaromir,
Marcel, Christoph, ... (apologies to whoever I forgot), maybe it's
time to revisit this problem.

Christoph, if you still have brain cells unburnt by this problem ;)

Jaromir Matas

Re: stepping over non local return in a protected block

Hi Nicolas,

Nicolas Cellier wrote
> When debugging things like this:
>
> [^2] ensure: [Transcript cr; show: 'done'].
>
> if we step into the protected block [^2], then step over ^2, we
> incorrectly get a BlockCannotReturn.

Hmm, that's the bug that plagued #terminate and caused unwind errors... I
didn't fully realize then; I simply avoided it by eliminating the simulation
code from #terminate :) This behavior happens when the VM calls
#aboutToReturn:through: and the Debugger executes #stepOver it (or over
#return:through: or #resume:through:) - sorry if I'm stating the obvious;
unfortunately I have negligible knowledge of the Debugger. I'm really
looking forward to seeing a solution!
Thanks,
best,

-----
^[^ Jaromir
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

^[^ Jaromir

marcel.taeumel

Re: stepping over non local return in a protected block

Hmm... at least you get a debugger. In Squeak 5.3, you are not able to put a "self halt" into that event handler in test03EventHandler. ^__^

Yet, I noticed that when stepping over that "^ self dispatchEvent:..." in #dispatchMouseDown:with:, a second debugger appears. That's a bug. The expected behavior would be that the same debugger shows the "halt." I think.

Best,

Marcel

Am 17.04.2021 10:58:06 schrieb Jaromir Matas <[hidden email]>:
Hi Nicolas,

Nicolas Cellier wrote
> When debugging things like this:
>
> [^2] ensure: [Transcript cr; show: 'done'].
>
> if we step into the protected block [^2], then step over ^2, we
> incorrectly get a BlockCannotReturn.

Hmm, that's the bug that plagued #terminate and caused unwind errors... I
didn't fully realize then; I simply avoided it by eliminating the simulation
code from #terminate :) This behavior happens when the VM calls
#aboutToReturn:through: and the Debugger executes #stepOver it (or over
#return:through: or #resume:through:) - sorry if I'm stating the obvious;
unfortunately I have negligible knowledge of the Debugger. I'm really
looking forward to seeing a solution!
Thanks,
best,

-----
^[^ Jaromir
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

Christoph Thiede

Re: stepping over non local return in a protected block

Hi all,

thanks for reporting. I think that we see two overlapping effects here:

First, #runUntilErrorOrReturnFrom: (which is called eventually by Debugger >> #stepOver) does not support Context >> #aboutToReturn:through: correctly, still. I'd like to refer to this thread again: http://forum.world.st/BUG-REGRESSION-while-debugging-Generator-gt-gt-nextPut-td5108125.html#a5127567

But the fact that the guard contexts are not removed again should not lead to a cannot return situation. Instead, I would rather have expected that the UI process would be terminated. But somehow, the termination in BlockClosure >> #newProcess fails. I observed that primitive 88 in Process >> #suspend fails so the execution continues right from the bottom context of the process. I'm not sure why primitiveSuspend fails here. Might this be related to the changes in termination logic? Jaromir? :-)

Best,

Christoph

Von: Squeak-dev <[hidden email]> im Auftrag von Taeumel, Marcel
Gesendet: Montag, 19. April 2021 09:54:44
An: squeak-dev
Betreff: Re: [squeak-dev] stepping over non local return in a protected block

Hmm... at least you get a debugger. In Squeak 5.3, you are not able to put a "self halt" into that event handler in test03EventHandler. ^__^

Best,

Marcel

Am 17.04.2021 10:58:06 schrieb Jaromir Matas <[hidden email]>:

Hi Nicolas,

Nicolas Cellier wrote
> When debugging things like this:
>
> [^2] ensure: [Transcript cr; show: 'done'].
>
> if we step into the protected block [^2], then step over ^2, we
> incorrectly get a BlockCannotReturn.

Hmm, that's the bug that plagued #terminate and caused unwind errors... I
didn't fully realize then; I simply avoided it by eliminating the simulation
code from #terminate :) This behavior happens when the VM calls
#aboutToReturn:through: and the Debugger executes #stepOver it (or over
#return:through: or #resume:through:) - sorry if I'm stating the obvious;
unfortunately I have negligible knowledge of the Debugger. I'm really
looking forward to seeing a solution!
Thanks,
best,

-----
^[^ Jaromir
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

Carpe Squeak!

Jaromir Matas

Re: stepping over non local return in a protected block

Hi Christoph,

Christoph Thiede wrote
> But the fact that the guard contexts are not removed again should not lead
> to a cannot return situation. Instead, I would rather have expected that
> the UI process would be terminated. But somehow, the termination in
> BlockClosure >> #newProcess fails. I observed that primitive 88 in Process
> >> #suspend fails so the execution continues right from the bottom context
> of the process. I'm not sure why primitiveSuspend fails here. Might this
> be related to the changes in termination logic? Jaromir? :-)

In my opinion the problem is that when the Debugger invokes #stepOver over a
non-local return, it invokes #evaluate:onBehalfOf: down the chain of sends
to execute the non-local return and that's where the problem lies:
#evaluate:onBehalfOf: incorrectly executes the non-local return on the wrong
stack - the debugger process uses its own stack to have
#evaluate:onBehalfOf: executed the non-local return ([^2]) which fails to
find its home context which is indeed on a different stack (original
process's stack). The cannot return is just a consequence of the failure to
find the home context...

At least this is my working hypothesis :)

Compared to #evaluate:onBehalfOf:, the #runUntilErrorOrReturnFrom: method
itself jumps right into the debugged process's stack - which results in
flawless execution of the non-local return! It finds it's home context and
jumps into it and continues execution.

I, however, haven't explored the behavior of the combination of
#evaluate:onBehalfOf: and #runUntilErrorOrReturnFrom: :o As I said my
Debugger knowledge is still negligible, unfortunately.

Prior to your change from active to genuine process the `[^2] ensure: []`
example caused the infinite debugger loop... after that it only causes the
cannot return error. So I guess it's unlikely the change in #terminate
somehow affected the bug's behavior; the bug was there before and behaves
similarly.

The change in #teminate only eliminated the use #evaluate:onBehalfOf: in the
termination procedure...

I'm looking at it from a different angle now, I'll keep you posted...
Thanks,
best,

-----
^[^ Jaromir
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

^[^ Jaromir

Christoph Thiede

Re: stepping over non local return in a protected block

Thanks Jaromir,

this exception makes very much sense. So to fix this bug, we "only" need to fix #runUntilErrorOrReturnFrom: for stack-manipulating code. I'll need to clean up my inbox a bit to find out whether there have been any recent posts on this issue ... :-)

Best,

Christoph

Von: Squeak-dev <[hidden email]> im Auftrag von Jaromir Matas <[hidden email]>
Gesendet: Montag, 19. April 2021 15:08:55
An: [hidden email]
Betreff: Re: [squeak-dev] stepping over non local return in a protected block

Carpe Squeak!

Jaromir Matas

Re: stepping over non local return in a protected block

In reply to this post by Nicolas Cellier

Hi Nicolas, Christoph, all

> When debugging things like this:
>
> [^2] ensure: [Transcript cr; show: 'done'].
>
> if we step into the protected block [^2], then step over ^2, we
> incorrectly get a BlockCannotReturn.

Found a bug inside the debugger causing this problem: #stepOver uses
#runUntilErrorOrReturnFrom: which inserts an ensure context when started.
The problem happens when the debugger simulates #aboutToReturn:through: --
before it starts executing it via #runUntilErrorOrReturnFrom this method
inserts its ensure context between #aboutToReturn:through:firstUnwindContext
and its firstUnwindContext argument! This means the inserted context will be
skipped during execution and that's the whole problem :)

The situation is captured on the enclosed screenshot.
Screenshot_2021-05-12_202833.png
<http://forum.world.st/file/t372955/Screenshot_2021-05-12_202833.png>

To prove this try changing #aboutToReturn and the BlockCannotReturn error is
gone:

#aboutToReturn: result through: firstUnwindContext
"Called from VM when an unwindBlock is found between self and its home.
Return to home's sender, executing unwind blocks on the way."

self methodReturnContext return: result

I'm not sure how to fix it but a debugger pro should figure it out way
faster than me :)

-----
^[^ Jaromir
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

^[^ Jaromir

Nicolas Cellier

Re: stepping over non local return in a protected block

Hi Jaromir,
good find!
See https://source.squeak.org/inbox/Kernel-nice.1407.diff
I based it on your latest inbox contribution, because I don't know if
this could be decoupled...

Le mer. 12 mai 2021 à 20:48, Jaromir Matas <[hidden email]> a écrit :

>
> Hi Nicolas, Christoph, all
>
> > When debugging things like this:
> >
> > [^2] ensure: [Transcript cr; show: 'done'].
> >
> > if we step into the protected block [^2], then step over ^2, we
> > incorrectly get a BlockCannotReturn.
>
> Found a bug inside the debugger causing this problem: #stepOver uses
> #runUntilErrorOrReturnFrom: which inserts an ensure context when started.
> The problem happens when the debugger simulates #aboutToReturn:through: --
> before it starts executing it via #runUntilErrorOrReturnFrom this method
> inserts its ensure context between #aboutToReturn:through:firstUnwindContext
> and its firstUnwindContext argument! This means the inserted context will be
> skipped during execution and that's the whole problem :)
>
> The situation is captured on the enclosed screenshot.
> Screenshot_2021-05-12_202833.png
> <http://forum.world.st/file/t372955/Screenshot_2021-05-12_202833.png>
>
> To prove this try changing #aboutToReturn and the BlockCannotReturn error is
> gone:
>
> #aboutToReturn: result through: firstUnwindContext
> "Called from VM when an unwindBlock is found between self and its home.
> Return to home's sender, executing unwind blocks on the way."
>
> self methodReturnContext return: result
>
> I'm not sure how to fix it but a debugger pro should figure it out way
> faster than me :)
>
>
>
>
>
> -----
> ^[^ Jaromir
> --
> Sent from: http://forum.world.st/Squeak-Dev-f45488.html
>

Jaromir Matas

Re: stepping over non local return in a protected block

Hi Nicolas,

thanks for your quick response; stepping over #aboutToReturn:through: now
works perfectly, however the same problem remains on lower levels, i.e. for
stepping over #return:through: and #resume:through: - same example, same
incorrect behavior:

[^2] ensure: [Transcript showln: 'done']

step into ^2 then repeat step over until you step over the return:through:
and cannot return is back. Same on the level below: step into ^2 then into
return:through: and then step over #resume:through: - and again, you get the
cannot return error. Once you're inside #resume:through: it seems safe.

So I'm wondering whether it's possible to invoke a simulated version of
#resume:through: somehow instead of #aboutToReturn:through: - I guess you'd
know the answer :)

BTW: I tried your fix in Cuis and it works perfectly there as well :)

During testing I realized I'd missed one more detail in my #terminate fix so
here's the latest version in the Inbox: Kernel-jar.1408 (including your fix
Kernel-nice.1407)

The point is the Debugger actually resumes after finding an error and only
updates its title ( which I never fully noticed :o ) but during termination
each nested error opens a new debugger - this means I can't override
#runUntilErrorOrReturnFrom: as I suggested previously so I created its copy
named #runUnwindUntilErrorOrReturnFrom: with the modified functionality
required by #terminate. (It's a code duplication, I know, but I'd like to
get it working now)

On top of that I extracted a repeating part of #terminate's code to a new
method #complete:to: to improve readability and avoid further code
duplication.

So you can now debug safely (and correctly) even examples like:

[self error] ensure: [^2]

Thanks again,

-----
^[^ Jaromir
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

^[^ Jaromir

Christoph Thiede

Re: stepping over non local return in a protected block

Hi Jaromir, Hi Nicolas,

> however the same problem remains on lower levels, i.e. for stepping over
> #return:through: and #resume:through: - same example, same incorrect
> behavior:

I fully agree with this. While Nicolas's fix makes this particular situation
easier to debug, IMHO it is only fighting the symptoms. In my opinion, we
should instead fix the underlying problem of dangling guard contexts in
#runUntilErrorOrReturnFrom:.

I have just updated runUntilErrorOrReturnFrom.cs as proposed in [1], which
notifies the debugger about risky context manipulations such as #jump,
#swapSender:, and now also #resume:through:. I think that this is a more
holistic approach than #simulatedAboutToReturn:through:.

I'm still wondering whether this approach has its limits, but if there are
any, I did not yet find them. Probably Jaromir you might feel like giving
this a closer look? Looking forward to your feedback! :-)

Best,
Christoph

[1]
http://forum.world.st/BUG-REGRESSION-while-debugging-Generator-nextPut-tp5108125p5129721.html

-----
Carpe Squeak!
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

Carpe Squeak!

Jaromir Matas

Re: stepping over non local return in a protected block

Hi Christoph,

> I have just updated runUntilErrorOrReturnFrom.cs as proposed in [1], which
> notifies the debugger about risky context manipulations such as #jump,
> #swapSender:, and now also #resume:through:. I think that this is a more
> holistic approach than #simulatedAboutToReturn:through:.

Debugger implementation is still out of my league so at least a few
observations:

\1. You inserted `here push: nil` line into #runUntilErrorOrReturnFrom: I
guess it's because you wanted to make it a top context but I thought #jump
already takes care of that with its first line:

`thisContext sender push: nil`

Please correct me if I'm wrong, I'm very interested :)

\2 Your #nextHandlerContext predates and thus removes Nicolas's changes made
recently - is it just accidental or intentional?

> > however the same problem remains on lower levels, i.e. for stepping over
> > #return:through: and #resume:through: - same example, same incorrect
> > behavior:
>
> I fully agree with this. While Nicolas's fix makes this particular
> situation
> easier to debug, IMHO it is only fighting the symptoms. In my opinion, we
> should instead fix the underlying problem of dangling guard contexts in
> #runUntilErrorOrReturnFrom:.

I've tested your changeset against the famous example:

`[self error] ensure: [^2]`

If you step through until you get to ^2 and then step over the ^2 - you get
the cannot return error again (without Nicolas's fix).

My opinion is let's get this working using whichever approach (I was
wondering if it would help to create a simulated version of #return:through:
instead of #aboutToReturn:through:) and build on that. At least we'll have a
better understanding of the mechanism :)

Thanks and regards,

-----
^[^ Jaromir
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

^[^ Jaromir

Christoph Thiede

Re: stepping over non local return in a protected block

Hi Jaromir,

thanks for the feedback. \2 and \3 were both minor slips which I have
corrected in version 7 of the changeset, see:
http://forum.world.st/BUG-REGRESSION-while-debugging-Generator-nextPut-tp5108125p5129807.html

\1: Fair question. I think I stumbled upon some situation where stepping
*over* #runUntilErrorOrReturnFrom: has not worked for me without these
lines. I just had inserted the "push: nil" without thinking about it in
detail, just because it had also worked for me in Context class >>
#contextEnsure: and #contextOn:do: in the last year (this was also a very
interesting bug, you can read the full story in the mailing archives if you
are interested). But yes, that is suspicious, I need to recheck this because
I cannot reproduce the need for this patch.

\2: This was indeed a slip because I forgot to update the image. I have
moved my patch to #findNextHandlerContext - it makes the method robust
against bottom-contexts that do not have a sender (i.e., sender is nil).

\3: Ah, in this case, the unwind context was already marked as complete. :-)
Since we appear to need the debugger information anyway, I have moved the
#informDebuggerAboutContextSwitchTo: in #resume:through: before the check so
your example now should work, too.

I'm curious whether you can find any other regressions in the changeset! :-)

Best,
Christoph

-----
Carpe Squeak!
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

Carpe Squeak!

Jaromir Matas

Re: stepping over non local return in a protected block

Hi Christoph,

Christoph Thiede wrote

> Hi Jaromir,
>
> thanks for the feedback. \2 and \3 were both minor slips which I have
> corrected in version 7 of the changeset, see:
> http://forum.world.st/BUG-REGRESSION-while-debugging-Generator-nextPut-tp5108125p5129807.html
>
> \1: Fair question. I think I stumbled upon some situation where stepping
> *over* #runUntilErrorOrReturnFrom: has not worked for me without these
> lines. I just had inserted the "push: nil" without thinking about it in
> detail, just because it had also worked for me in Context class >>
> #contextEnsure: and #contextOn:do: in the last year (this was also a very
> interesting bug, you can read the full story in the mailing archives if
> you
> are interested). But yes, that is suspicious, I need to recheck this
> because
> I cannot reproduce the need for this patch.

I've updated
http://forum.world.st/BUG-s-in-Context-control-jump-runUntilErrorOrReturnFrom-td5107263.html
with my explanation why I /think/ #runUntilErrorOrReturnFrom: works
correctly.

Christoph Thiede wrote
> \2: This was indeed a slip because I forgot to update the image. I have
> moved my patch to #findNextHandlerContext - it makes the method robust
> against bottom-contexts that do not have a sender (i.e., sender is nil).

The changeset still seems to have the old version of
#runUntilErrorOrReturnFrom: and #nextHandlerContext nixing Nicolas's changes
made in the meantime...

Christoph Thiede wrote
> \3: Ah, in this case, the unwind context was already marked as complete.
> :-)
> Since we appear to need the debugger information anyway, I have moved the
> #informDebuggerAboutContextSwitchTo: in #resume:through: before the check
> so
> your example now should work, too.

Yes!, my non-local examples work OK now, your scenarios as well; I'll keep
it in my images to give it a ride :) As for the general approach - I look
forward to learning from experts ;)

later,

-----
^[^ Jaromir
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

^[^ Jaromir

Jaromir Matas

Re: stepping over non local return in a protected block

In reply to this post by Nicolas Cellier

Hi Nicolas, Christoph,

Nicolas Cellier wrote

> Simulating #aboutToReturn:through: did jump to first unwind context. But
> this first unwind context was determined BEFORE the simulation #ensure:
> has been inserted. This had the effect of skipping the simulation
> machinery protection, and did result in a BlockCannotReturn
> (cannotReturn:) error...
>
> This did prevent the debugger to correctly debug a protected block with
> non local return like this:
>
> [^2] ensure: [Transcript cr; show: 'done'].

What would you think about this approach: because #return:from: supplies the
first unwind context for #aboutToReturn:through: prematurely, how about to
supply nil instead of the first unwind context and let #resume:through: find
the first unwind context at precisely the right time? I.e.:

resume: value through: firstUnwindCtxt
"Unwind thisContext to self and resume with value as result of last send.
Execute any unwind blocks while unwinding.
ASSUMES self is a sender of thisContext."

| ctxt unwindBlock |
self isDead ifTrue: [self cannotReturn: value to: self].
----> ctxt := firstUnwindCtxt ifNil: [thisContext findNextUnwindContextUpTo:
self].
[ctxt isNil] whileFalse:
[(ctxt tempAt: 2) ifNil:
[ctxt tempAt: 2 put: true.
unwindBlock := ctxt tempAt: 1.
thisContext terminateTo: ctxt.
unwindBlock value].
ctxt := ctxt findNextUnwindContextUpTo: self].
thisContext terminateTo: self.
^value

The change is without any adverse effects and deals with all similar
simulated non-local returns.

Here's the modified #return:from:

return: value from: aSender
"For simulation. Roll back self to aSender and return value from it.
Execute any unwind blocks on the way. ASSUMES aSender is a sender of self"

| newTop |
aSender isDead ifTrue:
[^self send: #cannotReturn: to: self with: {value}].
newTop := aSender sender.
(self findNextUnwindContextUpTo: newTop) ifNotNil:
-----> [^self send: #aboutToReturn:through: to: self with: {value. nil}].
self releaseTo: newTop.
newTop ifNotNil: [newTop push: value].
^newTop

What do you think? Would this be clean?

-----
^[^ Jaromir
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

^[^ Jaromir

Christoph Thiede

Re: stepping over non local return in a protected block

Hi Jaromir,

> > \2: This was indeed a slip because I forgot to update the image. I have moved my patch to #findNextHandlerContext - it makes the method robust against bottom-contexts that do not have a sender (i.e., sender is nil).
>
> The changeset still seems to have the old version of #runUntilErrorOrReturnFrom: and #nextHandlerContext nixing Nicolas's changes made in the meantime...

Version 8 removes roerf finally. :-) But I could not find any trace of #nextHandlerContext in the current changeset, did you maybe forget to revert the previous version before loading v7?

> What would you think about this approach: because #return:from: supplies the first unwind context for #aboutToReturn:through: prematurely, how about to supply nil instead of the first unwind context and let #resume:through: find the first unwind context at precisely the right time?

Correct me if I'm wrong, but this only would move the problem again, wouldn't it? If you press over too late, we would have the same problem again? I'd still prefer a holistic approach such as my #informDebuggerAboutContextSwitchTo: proposal. Or did miss anything different with your proposal? :-)

Best,
Christoph

Carpe Squeak!

Jaromir Matas

Re: stepping over non local return in a protected block

Christoph Thiede wrote

> Hi Jaromir,
>
>> > \2: This was indeed a slip because I forgot to update the image. I have
>> moved my patch to #findNextHandlerContext - it makes the method robust
>> against bottom-contexts that do not have a sender (i.e., sender is nil).
>>
>> The changeset still seems to have the old version of
>> #runUntilErrorOrReturnFrom: and #nextHandlerContext nixing Nicolas's
>> changes made in the meantime...
>
> Version 8 removes roerf finally. :-) But I could not find any trace of
> #nextHandlerContext in the current changeset, did you maybe forget to
> revert the previous version before loading v7?

yes indeed, I forgot to remove the previous version, sorry for confusion,
everything's fine :)

Christoph Thiede wrote

>> What would you think about this approach: because #return:from: supplies
>> the first unwind context for #aboutToReturn:through: prematurely, how
>> about to supply nil instead of the first unwind context and let
>> #resume:through: find the first unwind context at precisely the right
>> time?
>
> Correct me if I'm wrong, but this only would move the problem again,
> wouldn't it? If you press over too late, we would have the same problem
> again? I'd still prefer a holistic approach such as my
> #informDebuggerAboutContextSwitchTo: proposal. Or did miss anything
> different with your proposal? :-)

Well, I thought the fix really solved the bug, not just pushed it further
away :) I couldn't reproduce the incorrect stepOver behavior any longer but
I may have missed some example - do you have something in mind? I'll comment
further in [1]; I hope I'm not wrong here - please send a counterexample if
you find one :)

best,
Jaromir

Christoph Thiede wrote
> Best,
> Christoph

[1]
http://forum.world.st/BUG-REGRESSION-while-debugging-Generator-gt-gt-nextPut-td5108125i20.html

-----
^[^ Jaromir
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

^[^ Jaromir