Re: [Vm-dev] [squeak-dev] Bug whith contextOn:do:

Hi all

Picking up an old discussion, I'd like to include both of Eliot fixes regarding contextOn:do: and ensure into trunk before release. Any vetoes?

Best regards -Tobias

On 31.03.2015, at 23:10, Eliot Miranda eliot.miranda@gmail.com wrote:

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