Push/pop considered harmful

Do you think we could change Slang so that it wouldn't return anything if the procedure returns nothing? Thus avoid all these sqInt foo() {}  **warning does not return value messages

- Show quoted text -

On 2-Mar-09, at 11:24 PM, Andreas Raab wrote:

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]:".

Cheers,
 - Andreas

--
===========================================================================
John M. McIntosh <[hidden email]>
Corporate Smalltalk Consulting Ltd.  http://www.smalltalkconsulting.com
===========================================================================

2009/3/3 Andreas Raab <[hidden email]>:
>

> Igor Stasenko wrote:
>>
>> 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.
>
> I think that is a brilliant idea, right in line with what we were doing.
> Finally you can use left-to-right access to method args ;-)
>
>> 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.
>
> Right. One thing we can do here is to add a check to primArgAt: that just
> fails the primitive if you try to access arguments that aren't provided.
>

yes, this check could be put in #primArgAt:
but i feel little unpleased that it will be performed more than once.
It would be better to have a slang directive, like:
self expectedArguments: 3.  " 1== receiver only, 2 == receiver +
single arg ... "

in primitie code, and put it as first check in primitive entry. Then
code generator could produce a code which checks the number of
arguments, and automatically fail primitive if they are not same as
expected.

>> 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.
>
> Yes. push and pop will still be there for internal use - it is only the
> plugins that will get updated. So anything that wants to munge the stack in
> such a way will have to be builtin VM primitive which seems fair to me.
>
> Cheers,
>  - Andreas
>

--
Best regards,
Igor Stasenko AKA sig.