On Wed, May 6, 2009 at 8:28 AM, Andreas Raab <andreas.raab@gmx.de> wrote:

David T. Lewis wrote:
The main concern seems to be existing bugs in the code base that are
just now being exposed. I don't know of any easy way out; as far as
I know you have to either inspect the code manually, or wait for it
to fail at run time.

Note, it is common in many plugins to pop things off the stack early
in the method, so you cannot just patch the last line of the method.
You need to check senders to determine the expected argument count,
then make sure that the plugin matches this.

SmartSyntaxInterpreterPlugin automates much of this, so it's less
of a concern for plugins that use it. Also, I think that Eliot has
some ideas for making the problem go away entirely.

We changed our VMs to ignore push/pop requests from plugins and rather have the VM do the management of arguments. The VM interprets popXYZ as an access of method argument n-x and pushXYZ as an implicit return from a method and then pops the "right" number of args regardless of what the plugin thinks it requested. We also added more convenient (lef-to-right) argument accessors. This has worked very well for us.


Yes, we support this in the Stack VM.  But this is sloooow, we're not actually using it yet, and I'm not supporting this for Cog.  I'm relying merely on the simple stack balance check which has also worked and is much faster.  This is only an error-checking step and doesnt have much benefit in production, whereas eliminating the bug in the first place by eliminating explicit stack manipulation in primitives is the way to go.

 
One thing we could do is for the next generation of VMs which have a new image format won't be referred to as Squeak 3x etc. to up the interpreter proxy major number and use only that mechanism. Thoughts?

That a smart syntax approach is the way to go.  That it may be much better to duplicate primitives than support varargs (very few primitives need varargs) but that varargs can be supported somehow, e.g.

    myVarArgClass: numArgs receiver: receiver with: firstArg
        <varargs>
        numArgs = 0 ifTrue: [^self fetchClassOf: receiver].
        numArgs = 1 ifTrue: [^self fetchClassOf: firstArg].
        ^self primitiveFailFor: BadNumArgs

which would be both the class primitive and a "thisContext mirrorClassOf: something" primitive as the current one is:
    primitiveClass
| instance |
instance := self stackTop.
self pop: argumentCount+1 thenPush: (self fetchClassOf: instance)
   
That one makes the common case go fast as long as errors are apparent.  So have as streamlined a mechanism that can still spot incorrect argument counts as possible, and hence that the best way is to make it impossible to get the argument count wrong in the first place by eliminating explicit stack manipulation in primitives.     


Cheers,
 - Andreas