[Vm-dev] Re: [squeak-dev] Re: Separate TeaTime (was Re: Re: Teleplace is hiring...)

[Eliot Miranda]

Hi All,

    Andreas, vm-devs this is a question of y'all.  Recently I integrated
some of the merge of Alien callback support into ReentrantFFIPlugin.  This
broke OpenGL example in images not containing Alien.  Here's the breakage:

As part of Alien marshalling the FFI plugin needs to know if an argument is
an Alien and hence dereference it correctly to pass its address.  Here's the
code in ReentrantFFIPlugin >>ffiAtomicArgByReference:Class:in:

isAlien := (isString := interpreterProxy
includesBehavior: oopClass
ThatOf: interpreterProxy classString)
ifTrue: [false]
ifFalse:
[interpreterProxy
includesBehavior: oopClass
ThatOf: interpreterProxy classAlien].

This is erroneously setting isAlien to true when oopClass is Bitmap (i.e.
when uploadFont: tries to glBitmap:with:with:with:with:with:with: a Bitmap).
 That causes the marshaller to treat the Bitmap as if it were an Alien and
pass a field in the Bitmap instead of the Bitmap's base address.  Boom.

The problem is that
a) if Alien isn't installed in the image then interpreterProxy classAlien
== interpreterProxy nilObject,
and b) includesBehavior:ThatOf: always answers true if its second argument
is nilObject.

I suggest that includesBehavior:ThatOf: is incorrect:

StackInterpreter methods for plugin primitive support
includesBehavior: aClass ThatOf: aSuperclass
"Return the equivalent of
aClass includesBehavior: aSuperclass.
Note: written for efficiency and better inlining (only 1 temp)"
| theClass |
<inline: true>
(((theClass := aClass) = aSuperclass) "aClass == aSuperclass"
or:[aSuperclass = objectMemory nilObject]) "every class inherits from nil"
ifTrue:[^true].
[(theClass := self superclassOf: theClass) = aSuperclass ifTrue:[^true].
theClass ~= objectMemory nilObject] whileTrue.
^false

Classes *don't* inherit from nil.  nil is at the end of their superclass
chain.  That's different from inheriting.

At least in current images UndefinedObject includesBehavior: nil is false,
which seems right to me.  nil is not a behavior; its use as the sentinel at
the end of the superclass chain doesn't imply it is a behavior.  So I
propose that we change includesBehavior:ThatOf: to something like

StackInterpreter methods for plugin primitive support
includesBehavior: aClass ThatOf: aSuperclass
"Return the equivalent of
aClass includesBehavior: aSuperclass.
Note: written for efficiency and better inlining (only 1 temp)"
| theClass |
<inline: true>
aSuperclass = objectMemory nilObject ifTrue:
[^false].
theClass := aClass.
[theClass = aSuperclass ifTrue:
[^true].
 theClass ~= objectMemory nilObject] whileTrue:
[theClass := self superclassOf: theClass].
^false

I don't think this will affect anything other than FFI and Alien since those
are the only uses I can find, and in my reading of that code the proposed
change seems fine; safer in fact.

Agreed?

On Thu, Mar 17, 2011 at 12:30 PM, Matthew Fulmer <tapplek at gmail.com> wrote:

> On Thu, Mar 17, 2011 at 11:23:57AM -0700, Andreas Raab wrote:
> > On 3/17/2011 10:59, Lawson English wrote:
> > > The biggest holdup for getting these things into the Squeak trunk is
> > > simply getting Cobalt itself into the Squeak trunk so it can run on VM
> > > 4.2. If anyone wants to help Matt with THAT project, I'm pretty sure he
> > > will be happy for the assist.
> >
> > What do you need help with?
>
> The holdup is the VM's actually:
>
> - Recent Cog VM's crash on startup from OpenGL/FFI
> - Windows interpreter VM crashes on startup from JPEGPlugin
> - Unix interpreter VM has broken MPEGPlugin
> - Mac VM hasn't been extensively tested; John's latest may work,
>  but Esteban's is missing plugins
>
> Old Cog VM's (2316) almost work, but:
> - Windows VM is missing SoundPlugin
> - At least on windows, the router sockets timeout after they've
>  been running for 30 minutes or so
>
> There is very little that needs doing image-side; I've already
> done that part. Since we have a zero-regression policy for the
> official Cobalt releases, we can't release until we have working
> VM's. None of us are VM devs, and we have plenty to work on
> while we wait, so we're just waiting for the VM guys right now.
>
> --
> Matthew Fulmer (a.k.a. Tapple)
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20110329/94804a7a/attachment.htm