[Pharo-project] doesNotUnderstand:/createActualMessageTo: for zero-argument sends appears to imply the need for an extra stack slot in methods
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
Seems to make sense. If they didn't allocate small/big frames but exactly just enough space for the arguments this will indeed fail. You could also change the DNU to reallocate the frame on demand; maybe even only if the argsize is 0? I don't know the implementation details obviously, but it seems like you would waste less space to do it that way; although you will get a slight performance hit for the DNU, but that seems less relevant. cheers, Toon On 05/06/2011 07:39 PM, Eliot Miranda 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?
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
participants (2)
-
Eliot Miranda -
Toon Verwaest