[prev in list] [next in list] [prev in thread] [next in thread] 

List:       squeak-vm-dev
Subject:    [Vm-dev] Re: [squeak-dev] Bug whith contextOn:do:
From:       Eliot Miranda <eliot.miranda () gmail ! com>
Date:       2015-03-31 21:10:53
Message-ID: CAC20JE2D+VoFtn5Y8fWJZiLXTc032iDfrcXnOJ+78LDMEOOPkA () mail ! gmail ! com
[Download RAW message or body]

[Attachment #3 (multipart/alternative)]


Hi Tobias,

    first let me sympathize.  This is such horrible code :-(.  Ovr the
years, getting this code to work with Cog's context-to-stack-mapping
machinery has given me headaches :-).


On Tue, Mar 31, 2015 at 9:01 AM, Tobias Pape <Das.Linux@gmx.de> wrote:

> Hey fellow Squeakers
>
> [Attention: low level lengthy stuff]
>
> I today encountered a strange behavior.
> When running the Startup code, somewhen ContextPart>>#complete is called,
> which, in the process issues #contextOn:do:, which subsequently somewhere
> interanlly
> does a jump:
>
> contextOn: exceptionClass do: block
>         "Create an #on:do: context that is ready to return from executing
> its receiver"
>
>         | ctxt chain |
>         ctxt := thisContext.
>         [chain := thisContext sender cut: ctxt. ctxt jump] on:
> exceptionClass do: block.
>         "jump above will resume here without unwinding chain"
>         ^ chain
>
> The idea is that we end up right before '^ chain' as the comment indicates.
> so what happens is that ctxt is the context of #contextOn:do: itself and
> it gets
> send #jump in the closure.
>
> Jump now does the following:
>
> jump
>         "Abandon thisContext and resume self instead (using the same
> current process).  You may want to save thisContext's sender before calling
> this so you can jump back to it.
>         Self MUST BE a top context (ie. a suspended context or a abandoned
> context that was jumped out of).  A top context already has its return
> value on its stack (see Interpreter>>primitiveSuspend and other suspending
> primitives).
>         thisContext's sender is converted to a top context (by pushing a
> nil return value on its stack) so it can be jump back to."
>
>         | top |
>         "Make abandoned context a top context (has return value (nil)) so
> it can be jumped back to"
>         thisContext sender push: nil.
>
>         "Pop self return value then return it to self (since we jump to
> self by returning to it)"
>         stackp = 0 ifTrue: [self stepToSendOrReturn].
>         stackp = 0 ifTrue: [self push: nil].  "must be quick return
> self/constant"
>         top := self pop.
>         thisContext privSender: self.
>         ^ top
>
> So. bytecode for #contextOn:do: is:
>
> 29 <8A 01> push: (Array new: 1)
> 31 <6B> popIntoTemp: 3
> 32 <89> pushThisContext:
> 33 <6A> popIntoTemp: 2
> 34 <12> pushTemp: 2
> 35 <13> pushTemp: 3
> 36 <8F 20 00 0A> closureNumCopied: 2 numArgs: 0 bytes 40 to 49
> 40      <89> pushThisContext:
> 41      <D2> send: sender
> 42      <10> pushTemp: 0
> 43      <E1> send: cut:
> 44      <8E 00 01> popIntoTemp: 0 inVectorAt: 1
> 47      <10> pushTemp: 0
> 48      <D3> send: jump
> 49      <7D> blockReturn
> 50 <10> pushTemp: 0
> 51 <11> pushTemp: 1
> 52 <F0> send: on:do:
> 53 <87> pop
> 54 <8C 00 03> pushTemp: 0 inVectorAt: 3
> 57 <7C> returnTop
>
>
> The jump lands right at 53 and does a pop.
> HOWEVER, at this point the stack of this context is empty and the pop
> actually pops the 3rd temp
> from the temps that 'just happens' to be right under the stack. This
> should be fatal.
> HOWEVER again, Squeak actually does not pop but only decrement the SP so
> the temp access still
> works(this _could_ be fine but some  implementations (Eg, RSqueak) tried
> to separate temps and
> stack; which is not possible currently).
>
> What could be the problem here?
> - are the 'stackp = 0'-checks in #jump wrong and they actually should
> check for the actual stack depth _after_ temps?
>

It does look like it.  I would have expected this to be more correct:

jump
"Abandon thisContext and resume self instead (using the same current
process).
 You may want to save thisContext's sender before calling this so you can
jump back to it.
Self MUST BE a top context (ie. a suspended context or a abandoned context
that was jumped
 out of).  A top context already has its return value on its stack (see
Interpreter>>primitiveSuspend
 and other suspending primitives). thisContext's sender is converted to a
top context (by pushing a
 nil return value on its stack) so it can be jump back to."

| top |
"Make abandoned context a top context (has return value (nil)) so it can be
jumped back to"
thisContext sender push: nil.

"Pop self return value then return it to self (since we jump to self by
returning to it)"
stackp <= self numTemps ifTrue: [self stepToSendOrReturn].
(stackp <= self numTemps
 and: [self willJustPop]) ifTrue: [self push: nil].  "must be quick return
self/constant"

top := self pop.
thisContext privSender: self.
^top


> - should we put in a "sacrificial anode" in #contextOn:do: so that the pop
> does not pop the empty stack? (like this:
>
> contextOn: exceptionClass do: block
>         "Create an #on:do: context that is ready to return from executing
> its receiver"
>
>         | ctxt chain |
>         ctxt := thisContext.
>         [chain := thisContext sender cut: ctxt.
>          ctxt push: nil. "sacrifical anode"
>          ctxt jump
>         ] on: exceptionClass do: block.
>         "jump above will resume here without unwinding chain"
>         ^ chain
>

That looks right.  Once the on:do: is sent the thisContext of
contextOn:do:'s stack contains only exceptionClass, block, context and the
indirection vector containing chain.  So it is in the stack = self numTemps
case.


>
> - Or is there an even better way?
>

I'm not sure the other ways are any better.  The way to transfer to a
context without disturbing its stack is to do a process switch.  So you do
something equivalent to

jump
"Abandon thisContext and resume self instead (using the same current
process)."

| process semaphore |
process := Processor activeProcess.
semaphore := Semaphore new.

[process suspendedContext unwindTo: self.
 process suspendedContext: self.
 semaphore signal] fork.

semaphore wait

This way no bizarre stack manipulations are going on, and no return value
is pushed, because there isn't a return.  One may get away with:

jump
"Abandon thisContext and resume self instead (using the same current
process)."

[| process |
 process := Processor activeProcess.
 process suspendedContext unwindTo: self.
 process suspendedContext: self] fork.

Processor yield

I'd be interested in your experience using either of these.  One of the
advantages the process switch versions have is in not updating the
receiving context sp there's a chance the context-to-stack mapping
machinery won't flush the context to the heap.  In the end it might
actually be faster.
-- 
best,
Eliot

[Attachment #6 (text/html)]

<div dir="ltr">Hi Tobias,<div><br></div><div>      first let me sympathize.   This is \
such horrible code :-(.   Ovr the years, getting this code to work with Cog&#39;s \
context-to-stack-mapping machinery has given me headaches \
:-).</div><div><br></div><div class="gmail_extra"><br><div class="gmail_quote">On \
Tue, Mar 31, 2015 at 9:01 AM, Tobias Pape <span dir="ltr">&lt;<a \
href="mailto:Das.Linux@gmx.de" target="_blank">Das.Linux@gmx.de</a>&gt;</span> \
wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hey \
fellow Squeakers<br> <br>
[Attention: low level lengthy stuff]<br>
<br>
I today encountered a strange behavior.<br>
When running the Startup code, somewhen ContextPart&gt;&gt;#complete is called,<br>
which, in the process issues #contextOn:do:, which subsequently somewhere \
interanlly<br> does a jump:<br>
<br>
contextOn: exceptionClass do: block<br>
            &quot;Create an #on:do: context that is ready to return from executing \
its receiver&quot;<br> <br>
            | ctxt chain |<br>
            ctxt := thisContext.<br>
            [chain := thisContext sender cut: ctxt. ctxt jump] on: exceptionClass do: \
                block.<br>
            &quot;jump above will resume here without unwinding chain&quot;<br>
            ^ chain<br>
<br>
The idea is that we end up right before &#39;^ chain&#39; as the comment \
indicates.<br> so what happens is that ctxt is the context of #contextOn:do: itself \
and it gets<br> send #jump in the closure.<br>
<br>
Jump now does the following:<br>
<br>
jump<br>
            &quot;Abandon thisContext and resume self instead (using the same current \
process).   You may want to save thisContext&#39;s sender before calling this so you \
can jump back to it.<br>  Self MUST BE a top context (ie. a suspended context or a \
abandoned context that was jumped out of).   A top context already has its return \
value on its stack (see Interpreter&gt;&gt;primitiveSuspend and other suspending \
                primitives).<br>
            thisContext&#39;s sender is converted to a top context (by pushing a nil \
return value on its stack) so it can be jump back to.&quot;<br> <br>
            | top |<br>
            &quot;Make abandoned context a top context (has return value (nil)) so it \
can be jumped back to&quot;<br>  thisContext sender push: nil.<br>
<br>
            &quot;Pop self return value then return it to self (since we jump to self \
by returning to it)&quot;<br>  stackp = 0 ifTrue: [self stepToSendOrReturn].<br>
            stackp = 0 ifTrue: [self push: nil].   &quot;must be quick return \
self/constant&quot;<br>  top := self pop.<br>
            thisContext privSender: self.<br>
            ^ top<br>
<br>
So. bytecode for #contextOn:do: is:<br>
<br>
29 &lt;8A 01&gt; push: (Array new: 1)<br>
31 &lt;6B&gt; popIntoTemp: 3<br>
32 &lt;89&gt; pushThisContext:<br>
33 &lt;6A&gt; popIntoTemp: 2<br>
34 &lt;12&gt; pushTemp: 2<br>
35 &lt;13&gt; pushTemp: 3<br>
36 &lt;8F 20 00 0A&gt; closureNumCopied: 2 numArgs: 0 bytes 40 to 49<br>
40         &lt;89&gt; pushThisContext:<br>
41         &lt;D2&gt; send: sender<br>
42         &lt;10&gt; pushTemp: 0<br>
43         &lt;E1&gt; send: cut:<br>
44         &lt;8E 00 01&gt; popIntoTemp: 0 inVectorAt: 1<br>
47         &lt;10&gt; pushTemp: 0<br>
48         &lt;D3&gt; send: jump<br>
49         &lt;7D&gt; blockReturn<br>
50 &lt;10&gt; pushTemp: 0<br>
51 &lt;11&gt; pushTemp: 1<br>
52 &lt;F0&gt; send: on:do:<br>
53 &lt;87&gt; pop<br>
54 &lt;8C 00 03&gt; pushTemp: 0 inVectorAt: 3<br>
57 &lt;7C&gt; returnTop<br>
<br>
<br>
The jump lands right at 53 and does a pop.<br>
HOWEVER, at this point the stack of this context is empty and the pop actually pops \
the 3rd temp<br> from the temps that &#39;just happens&#39; to be right under the \
stack. This should be fatal.<br> HOWEVER again, Squeak actually does not pop but only \
decrement the SP so the temp access still<br> works(this _could_ be fine but some   \
implementations (Eg, RSqueak) tried to separate temps and<br> stack; which is not \
possible currently).<br> <br>
What could be the problem here?<br>
- are the &#39;stackp = 0&#39;-checks in #jump wrong and they actually should check \
for the actual stack depth _after_ temps?<br></blockquote><div><br></div><div>It does \
look like it.   I would have expected this to be more \
correct:</div><div><br></div><div>jump</div><div><span class="" \
style="white-space:pre">	</span>&quot;Abandon thisContext and resume self instead \
(using the same current process).</div><div><span style="white-space:pre">	</span>  \
You may want to save thisContext&#39;s sender before calling this so you can jump \
back to it.</div><div><span class="" style="white-space:pre">	</span>Self MUST BE a \
top context (ie. a suspended context or a abandoned context that was \
jumped</div><div><span style="white-space:pre">	</span>  out of).   A top context \
already has its return value on its stack (see \
Interpreter&gt;&gt;primitiveSuspend</div><div><span style="white-space:pre">	</span>  \
and other suspending primitives). thisContext&#39;s sender is converted to a top \
context (by pushing a</div><div><span style="white-space:pre">	</span>  nil return \
value on its stack) so it can be jump back to.&quot;</div><div><br></div><div><span \
class="" style="white-space:pre">	</span>| top |</div><div><span class="" \
style="white-space:pre">	</span>&quot;Make abandoned context a top context (has \
return value (nil)) so it can be jumped back to&quot;</div><div><span class="" \
style="white-space:pre">	</span>thisContext sender push: \
nil.</div><div><br></div><div><span class="" \
style="white-space:pre">	</span>&quot;Pop self return value then return it to self \
(since we jump to self by returning to it)&quot;</div><div><span class="" \
style="white-space:pre">	</span>stackp &lt;= self numTemps ifTrue: [self \
stepToSendOrReturn].</div><div><span class="" style="white-space:pre">	</span>(stackp \
&lt;= self numTemps</div><div><span class="" style="white-space:pre">	</span> and: \
[self willJustPop]) ifTrue: [self push: nil].   &quot;must be quick return \
self/constant&quot;</div><div><br></div><div><span class="" \
style="white-space:pre">	</span>top := self pop.</div><div><span class="" \
style="white-space:pre">	</span>thisContext privSender: self.</div><div><span \
class="" style="white-space:pre">	</span>^top</div><div>  </div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
                
- should we put in a &quot;sacrificial anode&quot; in #contextOn:do: so that the pop \
does not pop the empty stack? (like this:<br> <br>
contextOn: exceptionClass do: block<br>
            &quot;Create an #on:do: context that is ready to return from executing \
its receiver&quot;<br> <br>
            | ctxt chain |<br>
            ctxt := thisContext.<br>
            [chain := thisContext sender cut: ctxt.<br>
              ctxt push: nil. &quot;sacrifical anode&quot;<br>
              ctxt jump<br>
            ] on: exceptionClass do: block.<br>
            &quot;jump above will resume here without unwinding chain&quot;<br>
            ^ chain<br></blockquote><div><br></div><div>That looks right.   Once the \
on:do: is sent the thisContext of contextOn:do:&#39;s stack contains only \
exceptionClass, block, context and the indirection vector containing chain.   So it \
is in the stack = self numTemps case.</div><div>  </div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
 <br>
- Or is there an even better way?<br></blockquote><div><br></div><div>I&#39;m not \
sure the other ways are any better.   The way to transfer to a context without \
disturbing its stack is to do a process switch.   So you do something equivalent \
to</div><div><br></div><div><div>jump</div><div><span class="" \
style="white-space:pre">	</span>&quot;Abandon thisContext and resume self instead \
(using the same current process).&quot;</div><div><br></div><div><span class="" \
style="white-space:pre">	</span>| process semaphore |</div><div><span class="" \
style="white-space:pre">	</span>process := Processor activeProcess.</div><div><span \
class="" style="white-space:pre">	</span>semaphore := Semaphore \
new.</div><div><br></div><div><span class="" style="white-space:pre">	</span>[process \
suspendedContext unwindTo: self.</div><div><span class="" \
style="white-space:pre">	</span> process suspendedContext: self.</div><div><span \
class="" style="white-space:pre">	</span> semaphore signal] \
fork.</div><div><br></div><div><span class="" \
style="white-space:pre">	</span>semaphore wait</div></div><div><br></div><div>This \
way no bizarre stack manipulations are going on, and no return value is pushed, \
because there isn&#39;t a return.   One may get away \
with:</div><div><br></div><div><div>jump</div><div><span class="" \
style="white-space:pre">	</span>&quot;Abandon thisContext and resume self instead \
(using the same current process).&quot;</div><div><br></div><div><span class="" \
style="white-space:pre">	</span>[| process |</div><div><span class="" \
style="white-space:pre">	</span> process := Processor activeProcess.</div><div><span \
class="" style="white-space:pre">	</span> process suspendedContext unwindTo: \
self.</div><div><span class="" style="white-space:pre">	</span> process \
suspendedContext: self] fork.</div><div><br></div><div><span class="" \
style="white-space:pre">	</span>Processor \
yield</div></div><div><br></div><div>I&#39;d be interested in your experience using \
either of these.   One of the advantages the process switch versions have is in not \
updating the receiving context sp there&#39;s a chance the context-to-stack mapping \
machinery won&#39;t flush the context to the heap.   In the end it might actually be \
faster.</div></div>-- <br><div class="gmail_signature">best,<div>Eliot</div></div> \
</div></div>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic