[Vm-dev] Push/pop considered harmful
andreas.raab at gmx.de
Tue Mar 3 16:32:46 UTC 2009
Igor Stasenko wrote:
> I'm just thinking , what if simpy store return value in some var, and
> add simple primitive which returns it.
> So, then even if primitive is not failed, you still can use its return
> value (could be useful in some situations).
Do you mean you wouldn't return when the prim is successful? Otherwise
you are already returning the result from the primitive method. Also,
keep in mind that this needs to be thread-safe. There may be a context
switch in the near future ;-)
> The modification for doing this is minimal.
> In primitiveResponse
> - self dispatchFunctionPointerOn: primIdx in: primitiveTable.
> + primReturnedValue := self dispatchFunctionPointerOn: primIdx in:
> and then add
> self export: true.
> ^ self pop:1 thenPush:( self signed32BitIntegerFor: primReturnedValue )
> 2009/3/3 Igor Stasenko <siguctua at gmail.com>:
>> 2009/3/3 Andreas Raab <andreas.raab at gmx.de>:
>> - Show quoted text -
>>> Folks -
>>> Ever since we started to use the Stack VM we had a series of subtle crashes,
>>> many of which we could trace to stack imbalances (i.e., mismatching numbers
>>> of push/pops in primitives). Since the Stack VM is much more affected by
>>> this we have been looking to fix these issues once and for all.
>>> One thing that occurred to us is that practically all primitives do the
>>> same: They pop the number of arguments and they push an optional return
>>> value. There are many ways in which this can get wrong, sometimes it's a
>>> subtle early return, sometimes the fact that the primitive has actually
>>> failed before the prim returns etc.
>>> One thing that all of the uses (in plugins) have in common that all the
>>> plugin *really* needs to do, is to provide a return value and leave the
>>> push/pop to be done by the VM. In this case the stack invariants wouldn't
>>> even be exposed to the plugin.
>>> What we are doing right now is along those lines - basically we are
>>> replacing push/pop in the interpreter proxy by variants that don't actually
>>> do push and pop. Rather than that, pop is ignored (it only remembers how
>>> many times you popped so we can track inconsistencies) and push remembers
>>> the return value.
>>> This should definitely be replaced at some point by a proper
>>> #primitiveReturnValue: call. I was wondering what people think about the
>>> possibility of automatically rewriting plugins to fix those uses? It should
>>> be reasonably easy to find all the uses of "interpreterProxy pop: foo" and
>>> just remove them and replace "interpreterProxy push[Float|Int]:" with
>>> "interpreterProxy primitiveReturn[Float|Int]:".
>> I've seen some code which doing stack balance check after prim returns:
>> in #primitiveResponse ..
>> DoBalanceChecks ifTrue:[
>> (self balancedStack: delta afterPrimitive: primIdx withArgs: nArgs)
>> ifFalse:[self printUnbalancedStack: primIdx].
>> But it seems that it disabled by default.
>> Yes it would be nice to replace push/pops with single method, like
>> #primitiveArgumentAt: index
>> where index = 0 is receiver
>> 1 .. n - rest of arguments.
>> Another thing, that most primitives never check a number of arguments
>> on stack. This is also a potential risk, when you calling primitive
>> with wrong number of arguments from a language side.
>> Apart from this, what you suppose to do with primitives who switching
>> the active context (entering block closure,signaling semaphore,
>> scheduling etc)?
>> In this situation you shouldn't try to validate the stack pointer, as
>> well as primitive return value is useless.
>>> - Andreas
>> Best regards,
>> Igor Stasenko AKA sig.
More information about the Vm-dev