[Vm-dev] Push/pop considered harmful

[Igor Stasenko]

2009/3/3 Andreas Raab <andreas.raab at gmx.de>:
>
> 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 ;-)
>
No, i mean we can use the return value of primitive function, which is
currently not used.
This is different from return value of primitive (an oop which is
pushed on stack).


> Cheers,
>  - Andreas
> - Show quoted text -
>
>> The modification for doing this is minimal.
>> In primitiveResponse
>>
>> -       self dispatchFunctionPointerOn: primIdx in: primitiveTable.
>> +       primReturnedValue := self dispatchFunctionPointerOn: primIdx in:
>> primitiveTable.
>>
>>
>> and then add
>>
>> primitiveGetPrimReturnValue
>>   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.
>>>
>>>> Cheers,
>>>>  - Andreas
>>>>
>>>
>>> --
>>> Best regards,
>>> Igor Stasenko AKA sig.
>>>
>>
>>
>>
>



-- 
Best regards,
Igor Stasenko AKA sig.