[Vm-dev] SmallInteger as Methods [WAS] Re: [Pharo-project] Ideas for CompiledMethod proxies?

[David T. Lewis]

On Mon, Nov 22, 2010 at 08:27:53PM +0100, Mariano Martinez Peck wrote:
>  
> On Mon, Nov 22, 2010 at 8:20 PM, David T. Lewis <lewis at mail.msen.com> wrote:
> >
> > On Mon, Nov 22, 2010 at 08:05:02PM +0100, Mariano Martinez Peck wrote:
> > >
> > > On Mon, Nov 22, 2010 at 7:58 PM, David T. Lewis <lewis at mail.msen.com>
> > wrote:
> > > >
> > > > On Sun, Nov 21, 2010 at 05:06:01PM -0800, Eliot Miranda wrote:
> > > > >
> > > > > On Sun, Nov 21, 2010 at 3:48 PM, Mariano Martinez Peck <
> > > > > marianopeck at gmail.com> wrote:
> > > > > >
> > > > > > On Sun, Nov 21, 2010 at 8:18 PM, David T. Lewis <
> > lewis at mail.msen.com
> > > > >wrote:
> > > > > >>
> > > > > >> On Sat, Nov 20, 2010 at 08:41:44PM +0100, Mariano Martinez Peck
> > wrote:
> > > > > >> >
> > > > > >> > On Sat, Nov 20, 2010 at 7:50 PM, Eliot Miranda <
> > > > eliot.miranda at gmail.com
> > > > > >> >wrote:
> > > > > >> > >
> > > > > >> > > On Sat, Nov 20, 2010 at 10:28 AM, Mariano Martinez Peck <
> > > > > >> > > marianopeck at gmail.com> wrote:
> > > > > >> > >
> > > > > >> > >> It doesn't work here. It seems I cannot use SmallInteger like
> > > > > >> methods. The
> > > > > >> > >> #run:with:in: is never call and the image crashes.
> > > > > >> > >>
> > > > > >> > >
> > > > > >> > > That's simply a bug and we should fix it.  What crashes, the
> > > > > >> Interpreter,
> > > > > >> > > Cog or both?  Prepare a reproducible test case (image, change
> > set
> > > > etc)
> > > > > >> and
> > > > > >> > > we can try and fix this.
> > > > > >> > >
> > > > > >> > >
> > > > > >> > To reproduce just file in the attached cs. Then evaluate the
> > > > following:
> > > > > >> >
> > > > > >> > TestCase methodDict at: #assert: put: 4.
> > > > > >> > TestCase new assert: 'zaraza'.
> > > > > >> >
> > > > > >> > If works, you should have a debugger in SmallInteger >>
> > > > #run:with:in:
> > > > > >> > otherwise, a crash.
> > > > > >> >
> > > > > >> > I've tested in mac vms like:
> > > > > >> >
> > > > > >> > Squeak 4.2.5beta1U
> > > > > >> > Squeak 5.8b4
> > > > > >> >
> > > > > >> > and it crash.
> > > > > >> > In Squeak 64-32 5.7b1   it just hangs forever (I guess at some
> > point
> > > > it
> > > > > >> will
> > > > > >> > crash).
> > > > > >> >
> > > > > >> > And in Cog it works perfect!  The problem is that I am using my
> > own
> > > > VM
> > > > > >> and I
> > > > > >> > cannot use COG. So, if Cog has fixed this, I would like to know
> > the
> > > > > >> changes
> > > > > >> > so that to apply it to my VM.
> > > > > >> >
> > > > > >> > Maybe it is already fixed in latest VMMaker, but I couldn't
> > compile
> > > > > >> > it....(other story)
> > > > > >> >
> > > > > >>
> > > > > >> It is definitely not fixed in the latest VMMaker (interpreter).
> > > > > >>
> > > > > >>
> > > > > >
> > > > > > Thanks Dave. Then I guess it was integrated in Cog by Eliot, or
> > maybe
> > > > just
> > > > > > a side effect in Eliot changes that automagically fixed this?
> > > > > >
> > > > >
> > > > > Yes.  When I did run:as:in: support for Cog I fixed this.  I
> > introduced
> > > > >
> > > > > isOopCompiledMethod: oop
> > > > >     "Answer whether the oop is an object of compiled method format"
> > > > > <api>
> > > > >     ^(self isNonIntegerObject: oop) and: [(self formatOf: oop) >= 12]
> > > > >
> > > > > and used it in the relevant places.  perhaps you could look at the
> > usage
> > > > in
> > > > > Cog and apply the same to the interpreter?
> > > > >
> > > > > The existing method is
> > > > >
> > > > > isCompiledMethod: oop
> > > > >     "Answer whether the argument object is of compiled method format"
> > > > >     ^(self formatOf: oop) >= 12
> > > > >
> > > > > and may crash if given a SmallInteger (alas on x86 it won't always
> > crash
> > > > :)
> > > > > ).
> > > >
> > > > Thanks Eliot,
> > > >
> > > > I put the check only into Interpreter>>lookupMethodInDictionary (not
> > > > because
> > > > I wanted to do it differently, but because I'm behind in my email and
> > did
> > > > not
> > > > notice your message until now). This seems sufficient for the moment,
> > and
> > > > I'll
> > > > make a note to apply your more general fixes in the next update.
> > > >
> > >
> > > Yes, because with only that change there are still problems. For example,
> > if
> > > you put a SmallInteger as method and then you send #perform:
> > > Example:
> > >
> > > TestCase methodDict at: #assert: put: 5.
> > > (TestCase >> #assert:) perform: #literals.
> >
> > I get MessageNotUnderstood: SmallInteger>>literals with your example.
> > Is that not expected?
> >
> 
> Yes, that's perfect....grrrr maybe it a another thing, I cannot remember :(
> maybe TestCase new perform: #assert:   ?
> 
> Anyway, if I can reproduce it again, I'll let you know.

Mariano,

Indeed you were right, the necessary checks were still missing from
#primitivePerform and #primitivePerformAt: so the following would still
crash the VM:

  TestCase methodDict at: #assert: put: 5.
  (TestCase >> #assert:) perform: #literals.

I added the updates to VMMaker-dtl.205 (this time adopting Eliots's
changes directly) so hopefully this will now cover all cases.
The update that I did for Interpreter>>lookupMethodInDictionary:
should be added back to the oscog branch, but it does not effect
the Cog VM itself, so at this point I expect that an interpreter VM
built from VMMaker-dtl.205 will behave identically to a Cog VM with
respect to handling objects as methods.

Dave