Push/pop considered harmful

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

Push/pop considered harmful

Andreas.Raab
 
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
Reply | Threaded
Open this post in threaded view
|

Re: Push/pop considered harmful

David T. Lewis
 
On Mon, Mar 02, 2009 at 11:24:07PM -0800, Andreas Raab wrote:
>
> 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.

Excellent idea, especially since this can be done without requiring
modifications to the existing plugins.

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

This would be helpful, although not essential. It might have problems
with things like this:
  interpreterProxy pop: 1; push: foo

But if it does 90% of the conversions automatically, that would still be
a big help.

Dave

Reply | Threaded
Open this post in threaded view
|

Re: Push/pop considered harmful

Igor Stasenko
In reply to this post by Andreas.Raab

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

>
> 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.
Reply | Threaded
Open this post in threaded view
|

Re: Push/pop considered harmful

Igor Stasenko

One more thing about use of primitive return value.
Do you plan to put this in use in Cog?

I remember, Eliot proposed to use context temporary slot to fill with
return value.
Then, when primitive fails , this value could be checked in method.

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).

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 <[hidden email]>:

> 2009/3/3 Andreas Raab <[hidden email]>:
> - 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.
Reply | Threaded
Open this post in threaded view
|

Re: Push/pop considered harmful

johnmci
In reply to this post by Andreas.Raab
 
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

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
=
=
=
========================================================================



Reply | Threaded
Open this post in threaded view
|

Re: Push/pop considered harmful

Andreas.Raab
In reply to this post by Igor Stasenko
 
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 ;-)

Cheers,
   - Andreas

> 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 <[hidden email]>:
>> 2009/3/3 Andreas Raab <[hidden email]>:
>> - 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.
>>
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Push/pop considered harmful

Eliot Miranda-2
In reply to this post by johnmci
 


On Tue, Mar 3, 2009 at 7:09 AM, John M McIntosh <[hidden email]> wrote:

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

Already done.  I have a slew of Slang changes that will get released with Cog.  I'll try and write-up a list sometime soon.

 
- 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
===========================================================================




Reply | Threaded
Open this post in threaded view
|

Re: Push/pop considered harmful

Andreas.Raab
In reply to this post by Igor Stasenko
 
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.

> 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
Reply | Threaded
Open this post in threaded view
|

Re: Push/pop considered harmful

Igor Stasenko

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.
Reply | Threaded
Open this post in threaded view
|

Re: Push/pop considered harmful

Igor Stasenko
In reply to this post by Andreas.Raab

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

>
> 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 <[hidden email]>:
>>>
>>> 2009/3/3 Andreas Raab <[hidden email]>:
>>> - 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.
Reply | Threaded
Open this post in threaded view
|

Re: Push/pop considered harmful

Eliot Miranda-2
In reply to this post by Igor Stasenko
 


On Tue, Mar 3, 2009 at 10:52 AM, Igor Stasenko <[hidden email]> wrote:

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 ... "
 
self expectedNumArgs: 0
    => self only
self expectedNumArgs: 1
    => self plus one argument
self expectedNumArgsBetween: m and: n
    for var-args primitives like perform:[with:*] and value[:[value:*]]

Perhaps checkNumArgs: or failUnlessNumArgs: are better names?



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.

Reply | Threaded
Open this post in threaded view
|

Re: Push/pop considered harmful

Igor Stasenko

2009/3/3 Eliot Miranda <[hidden email]>:

>
>
>
> On Tue, Mar 3, 2009 at 10:52 AM, Igor Stasenko <[hidden email]> wrote:
>>
>> 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 ... "
>
>
> self expectedNumArgs: 0
>     => self only
> self expectedNumArgs: 1
>     => self plus one argument
> self expectedNumArgsBetween: m and: n
>     for var-args primitives like perform:[with:*] and value[:[value:*]]
> Perhaps checkNumArgs: or failUnlessNumArgs: are better names?

as you like.. i bother only about concept :)

>>
>>
>> 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.
>
>
>



--
Best regards,
Igor Stasenko AKA sig.
Reply | Threaded
Open this post in threaded view
|

Re: Push/pop considered harmful

johnmci
In reply to this post by Eliot Miranda-2
 
well we also have

        self primitive: 'primitiveCreateHostWindow'
                parameters: #(SmallInteger SmallInteger SmallInteger SmallInteger  
ByteArray).

So I'm wondering here how many ways do we want to achieve the same  
thing.
Most people wanting to build a plugin want something that doesn't  
require too much thinking,
then again very few people build plugins, but in the code about, well  
we know it should have 5 parms
However the construct doesn't handle the case of  
expectedNumArgsBetween: 0 and: 3


On 3-Mar-09, at 10:59 AM, Eliot Miranda wrote:

>
>
> self expectedNumArgs: 0
>     => self only
> self expectedNumArgs: 1
>     => self plus one argument
> self expectedNumArgsBetween: m and: n
>     for var-args primitives like perform:[with:*] and value[:
> [value:*]]
>
> Perhaps checkNumArgs: or failUnlessNumArgs: are better names?
>

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



Reply | Threaded
Open this post in threaded view
|

Re: Push/pop considered harmful

Igor Stasenko

I was thinking about making an argument count check mandatory,
so VMMaker could spit in your face when you forget adding it.
One of the ways could be to make
self expectedNumArgs: 0
as a slang directive, which extracts the number and builds a primitive
table with entries of 2 elements:
- function pointer
- expected arguments.

Then we could check the number of arguments before calling a primitive
function and fail even without calling it. Moreover we don't need to
worry that someone forget adding #expectedNumArgs: in his code,
because its mandatory.

2009/3/3 John M McIntosh <[hidden email]>:

>
> well we also have
>
>        self primitive: 'primitiveCreateHostWindow'
>                parameters: #(SmallInteger SmallInteger SmallInteger
> SmallInteger ByteArray).
>
> So I'm wondering here how many ways do we want to achieve the same thing.
> Most people wanting to build a plugin want something that doesn't require
> too much thinking,
> then again very few people build plugins, but in the code about, well we
> know it should have 5 parms
> However the construct doesn't handle the case of expectedNumArgsBetween: 0
> and: 3
>
>
> On 3-Mar-09, at 10:59 AM, Eliot Miranda wrote:
>
>>
>>
>> self expectedNumArgs: 0
>>    => self only
>> self expectedNumArgs: 1
>>    => self plus one argument
>> self expectedNumArgsBetween: m and: n
>>    for var-args primitives like perform:[with:*] and value[:[value:*]]
>>
>> Perhaps checkNumArgs: or failUnlessNumArgs: are better names?
>>
>
> - Show quoted text -
> --
> ===========================================================================
> John M. McIntosh <[hidden email]>
> Corporate Smalltalk Consulting Ltd.  http://www.smalltalkconsulting.com
> ===========================================================================
>
>
>
>



--
Best regards,
Igor Stasenko AKA sig.