doesNotUnderstand:/createActualMessageTo: for zero-argument sends appears to imply the need for an extra stack slot in methods

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

doesNotUnderstand:/createActualMessageTo: for zero-argument sends appears to imply the need for an extra stack slot in methods

Eliot Miranda-2
 
Hi All,

    could you check my reasoning here.  I think I've found a serious bug in the VM with doesNotUnderstand: processing for zero-argument sends and the calculation of the necessary stack size in MethodNode>>generate:* (senders of CompiledMethod>>needsFrameSize:).  The bug causes a push beyond the end of a context, overwriting the header of the following object, when an MNU occurs for a zero-argument send when the stack is full.  If you look at Interpreter>>createActualMessageTo: it does the following to push the message created as the argument of the doesNotUnderstand:

        self pop: argumentCount thenPush: message

If argumentCount is zero this pushes an item on the stack.  If the stack is full at this point (the receiver of the not-understood message is the last item in the context) then the push will be out of bound of the activeContext's stack and overwrite the first word of the following object's header.

We observe this with a Newspeak application at Cadence and see a subsequent VM crash.

1. am I right?

2. is this an old bug?

3. did this, or did this not, affect Smalltalk-80 v2 and e.g. the Dorado implementation, and if so why (e.g. was the message of an MNU not pushed onto the stack, and instead MNUs activated by special code?, or was the activeContext not on the heap? or...)

4. what's the right fix?  It seems to me that the right fix is simply to add one to the needed stack depth, with an explanatory comment, but in senders of CompiledMethod>>needsFrameSize: or in CompiledMethod>>needsFrameSize:?  Trying to avoid pushing the argument of an MNU is going to involve lots of changes to the VM and will be tricky and likely have a bug tail.


best,
Eliot
Reply | Threaded
Open this post in threaded view
|

Re: doesNotUnderstand:/createActualMessageTo: for zero-argument sends appears to imply the need for an extra stack slot in methods

Igor Stasenko

On 6 May 2011 19:39, Eliot Miranda <[hidden email]> wrote:
>
> Hi All,
>     could you check my reasoning here.  I think I've found a serious bug in the VM with doesNotUnderstand: processing for zero-argument sends and the calculation of the necessary stack size in MethodNode>>generate:* (senders of CompiledMethod>>needsFrameSize:).  The bug causes a push beyond the end of a context, overwriting the header of the following object, when an MNU occurs for a zero-argument send when the stack is full.  If you look at Interpreter>>createActualMessageTo: it does the following to push the message created as the argument of the doesNotUnderstand:
>         self pop: argumentCount thenPush: message
> If argumentCount is zero this pushes an item on the stack.  If the stack is full at this point (the receiver of the not-understood message is the last item in the context) then the push will be out of bound of the activeContext's stack and overwrite the first word of the following object's header.
> We observe this with a Newspeak application at Cadence and see a subsequent VM crash.
> 1. am I right?

Sounds like you do.

> 2. is this an old bug?

never heard about it.
Probably because its really obscure.

> 3. did this, or did this not, affect Smalltalk-80 v2 and e.g. the Dorado implementation, and if so why (e.g. was the message of an MNU not pushed onto the stack, and instead MNUs activated by special code?, or was the activeContext not on the heap? or...)
> 4. what's the right fix?  It seems to me that the right fix is simply to add one to the needed stack depth, with an explanatory comment, but in senders of CompiledMethod>>needsFrameSize: or in CompiledMethod>>needsFrameSize:?  Trying to avoid pushing the argument of an MNU is going to involve lots of changes to the VM and will be tricky and likely have a bug tail.
>

So, your question actually can be interpreted,
- we can change an image-size to take into account that there's always
should be at least 1 extra slot on stack
reserved for MNU (and other messages like it btw). And based on this
criteria, calculate the minimum frame size required for method

- or we should change teh VM to check if we can push extra things on
stack. I think for that we don't have to use conventional
#pop:thenPush:, but do extra method , like #pop:thenPushChecked:
or
checkIfCanPushAfterPopping: x

But i fear its not just for MNU in particular (cannotInterpret: too,
and there could be other sends, no? just don't remember) .

Same for #perform: primitive - it is using the context's stack , and
you cannot predict how many arguments it may push, so you also need to
check for enough stack space there, but #perform: could also lead to
DNU (and therefore you should also take it into account there).

But StackInterpreter are not affected by this, i am right?

--
Best regards,
Igor Stasenko AKA sig.