Hi All,<div><br></div><div> 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:</div>
<div><br></div><div>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:</div>
<div><br></div><div><div><span class="Apple-tab-span" style="white-space:pre"> </span>isAlien := (isString := interpreterProxy </div><div><span class="Apple-tab-span" style="white-space:pre"> </span>includesBehavior: oopClass </div>
<div><span class="Apple-tab-span" style="white-space:pre"> </span>ThatOf: interpreterProxy classString)</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>ifTrue: [false]</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>ifFalse:</div>
<div><span class="Apple-tab-span" style="white-space:pre"> </span>[interpreterProxy </div><div><span class="Apple-tab-span" style="white-space:pre"> </span>includesBehavior: oopClass </div><div><span class="Apple-tab-span" style="white-space:pre"> </span>ThatOf: interpreterProxy classAlien].</div>
</div><div><br></div><div>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.</div>
<div><br></div><div>The problem is that</div><div><span class="Apple-tab-span" style="white-space: pre; "> </span>a) if Alien isn't installed in the image then interpreterProxy classAlien == interpreterProxy nilObject,</div>
<div>and<span class="Apple-tab-span" style="white-space: pre; "> </span>b) includesBehavior:ThatOf: always answers true if its second argument is nilObject.</div><div><br></div><div>I suggest that includesBehavior:ThatOf: is incorrect:</div>
<div><br></div><div>StackInterpreter methods for plugin primitive support</div><div><div>includesBehavior: aClass ThatOf: aSuperclass</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>"Return the equivalent of </div>
<div><span class="Apple-tab-span" style="white-space:pre"> </span>aClass includesBehavior: aSuperclass.</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>Note: written for efficiency and better inlining (only 1 temp)"</div>
<div><span class="Apple-tab-span" style="white-space:pre"> </span>| theClass |</div><div><span class="Apple-tab-span" style="white-space:pre"> </span><inline: true></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>(((theClass := aClass) = aSuperclass) "aClass == aSuperclass"</div>
<div><span class="Apple-tab-span" style="white-space:pre"> </span>or:[aSuperclass = objectMemory nilObject]) "every class inherits from nil"</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>ifTrue:[^true].</div>
<div><span class="Apple-tab-span" style="white-space:pre"> </span>[(theClass := self superclassOf: theClass) = aSuperclass ifTrue:[^true].</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>theClass ~= objectMemory nilObject] whileTrue.</div>
<div><span class="Apple-tab-span" style="white-space:pre"> </span>^false</div></div><div><br></div><div>Classes *don't* inherit from nil. nil is at the end of their superclass chain. That's different from inheriting.</div>
<div><br></div><div>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</div>
<div><br></div><div>StackInterpreter methods for plugin primitive support</div><div><div>includesBehavior: aClass ThatOf: aSuperclass</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>"Return the equivalent of </div>
<div><span class="Apple-tab-span" style="white-space:pre"> </span>aClass includesBehavior: aSuperclass.</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>Note: written for efficiency and better inlining (only 1 temp)"</div>
<div><span class="Apple-tab-span" style="white-space:pre"> </span>| theClass |</div><div><span class="Apple-tab-span" style="white-space:pre"> </span><inline: true></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>aSuperclass = objectMemory nilObject ifTrue:</div>
<div><span class="Apple-tab-span" style="white-space:pre"> </span>[^false].</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>theClass := aClass.</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>[theClass = aSuperclass ifTrue:</div>
<div><span class="Apple-tab-span" style="white-space:pre"> </span>[^true].</div><div><span class="Apple-tab-span" style="white-space:pre"> </span> theClass ~= objectMemory nilObject] whileTrue:</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>[theClass := self superclassOf: theClass].</div>
<div><span class="Apple-tab-span" style="white-space:pre"> </span>^false</div></div><div><br></div><div>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.</div>
<div><br></div><div>Agreed?<br><br><div class="gmail_quote">On Thu, Mar 17, 2011 at 12:30 PM, Matthew Fulmer <span dir="ltr"><<a href="mailto:tapplek@gmail.com">tapplek@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">On Thu, Mar 17, 2011 at 11:23:57AM -0700, Andreas Raab wrote:<br>
> On 3/17/2011 10:59, Lawson English wrote:<br>
> > The biggest holdup for getting these things into the Squeak trunk is<br>
> > simply getting Cobalt itself into the Squeak trunk so it can run on VM<br>
> > 4.2. If anyone wants to help Matt with THAT project, I'm pretty sure he<br>
> > will be happy for the assist.<br>
><br>
> What do you need help with?<br>
<br>
</div>The holdup is the VM's actually:<br>
<br>
- Recent Cog VM's crash on startup from OpenGL/FFI<br>
- Windows interpreter VM crashes on startup from JPEGPlugin<br>
- Unix interpreter VM has broken MPEGPlugin<br>
- Mac VM hasn't been extensively tested; John's latest may work,<br>
but Esteban's is missing plugins<br>
<br>
Old Cog VM's (2316) almost work, but:<br>
- Windows VM is missing SoundPlugin<br>
- At least on windows, the router sockets timeout after they've<br>
been running for 30 minutes or so<br>
<br>
There is very little that needs doing image-side; I've already<br>
done that part. Since we have a zero-regression policy for the<br>
official Cobalt releases, we can't release until we have working<br>
VM's. None of us are VM devs, and we have plenty to work on<br>
while we wait, so we're just waiting for the VM guys right now.<br>
<font color="#888888"><br>
--<br>
Matthew Fulmer (a.k.a. Tapple)<br>
<br>
</font></blockquote></div><br></div>