[Vm-dev] Push/pop considered harmful

Igor Stasenko siguctua at gmail.com
Tue Mar 3 14:16:14 UTC 2009


2009/3/3 Andreas Raab <andreas.raab at gmx.de>:
>
> 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.

> Cheers,
>  - Andreas
>


-- 
Best regards,
Igor Stasenko AKA sig.


More information about the Vm-dev mailing list