Hi Eliot, list I found a bug :) With latest VM (32 bits, using PharoVM flavour), I crash the VM when executing this: LGitError giterr_last. LGitError class>>giterr_last ^ self call: #(void *giterr_last()) options: #( ) this is a very basic FFI call, and should answer an ExternalData instance (pointing to NULL, in this case). Digging into it, I found at a point (when instantiating the pointer) it answers nil instead an instantiated ExternalData, and this is because in Spur32BitMemoryManager>>instantiateClass:indexableSize: otherwise clause is: otherwise: "non-indexable" [self cppIf: (PharoVM or: [true]) "Leave the old code but ignore it completely unless someone complains." ifTrue: [^nil] ifFalse: ["some Squeak images include funky fixed subclasses of abstract variable superclasses. e.g. DirectoryEntry as a subclass of ArrayedCollection. Allow fixed classes to be instantiated here iff nElements = 0." (nElements ~= 0 or: [instSpec > self lastPointerFormat]) ifTrue: [^nil]. numSlots := self fixedFieldsOfClassFormat: classFormat. fillValue := nilObj]]. So it will always answer nil/NULL. Now, I don’t know why it was commented out, but since there it says "Leave the old code but ignore it completely unless someone complains.”, I complain :D Thing is… if I restore old code it works fine. Question is: should we restore that code? or this shows a deeper problem? cheers, Esteban |
Hi Esteban,
On Tue, Aug 22, 2017 at 3:47 AM, Esteban Lorenzano <[hidden email]> wrote:
We can replace it with the ifFalse: branch. I will do so. I made this change because Nicolai Hess wrote to the Pharo mailing list. I knew that Squeak relied on the behavior but not Pharo. So cheers, _,,,^..^,,,_ best, Eliot |
In reply to this post by EstebanLM
On Tue, Aug 22, 2017 at 3:47 AM, Esteban Lorenzano <[hidden email]> wrote:
OK, I didn't mean to hit send. Ignore the previous posting. I made this change because Nicolai Hess wrote to the Pharo mailing list. I knew that Squeak relied on the behavior but not Pharo. So I decided to keep the behavior for Squeak and make it illegal for Pharo. Now we know that we know that the FFI depends on it. Looking at the code the use is here: ThreadedFFIPlugin>>ffiReturnPointer:ofType:in: ... self remapOop: oop in: [retOop := interpreterProxy instantiateClass: interpreterProxy classExternalData indexableSize: 0]. ... Alas InterpreterProxy doesn't provide instantiateClass:, only instantiateClass:indexableSize:. So the quick fix is to keep the old behavior (that new: 0 works for non-indexable classes) but the right fix is to change the ThreadedFFIPlugin and InterpreterProxy so we can write ... self remapOop: oop in: [retOop := interpreterProxy instantiateClass: interpreterProxy classExternalData]. ... What do people think? _,,,^..^,,,_ best, Eliot |
is ok for me :) Esteban
|
Free forum by Nabble | Edit this page |