[OpenSmalltalk/opensmalltalk-vm] ThreadedFFIPlugin throws FFIErrorCoercionFailed when passing 64-bit indexable objects to 64-bit pointer arguments (#569)
Given the external function defined by the following Squeak method: ```smalltalk myFunc: aFloat64Array
<apicall: void 'myFunc' (double*)> ^ self externalCallFailed ```
When passing an instance of `Float64Array` to this method, the FFI plugin throws error code 6 (`Could not coerce arguments`). The same can be reproduced in similar scenarios for other 64-bit indexable objects like `DoubleWordArray` and `SignedDoubleWordArray` (which ultimately share the same class format anyway). There were no issues when doing the same using 32-bit indexable objects.
---
As far as I can tell the issue seems to stem from `ThreadedFFIPlugin >> ffiAtomicArgByReference:Class:in:` which ends with the following code: ```smalltalk (atomicType <= FFITypeDoubleFloat) ifTrue: [((interpreterProxy isWords: oop) or: [oopClass = interpreterProxy classByteArray]) ifTrue: [^self ffiPushPointer: (interpreterProxy firstIndexableField: oop) in: calloutState]. (oopClass = interpreterProxy classExternalAddress) ifTrue: [^self ffiPushPointer: (self longAt: oop + interpreterProxy baseHeaderSize) in: calloutState]. isAlien ifTrue: [^self ffiPushPointer: (self pointerForOop: (self startOfData: oop)) in: calloutState]].
^FFIErrorCoercionFailed ``` While the second line checks 32-bit indexable via `isWords:` and byte arrays, it seems to be missing a test for `isLong64s:`.
---
Just in case it's relevant for some reason: ``` Squeak latest update: #20467 Image format 68021 (64 bit) Preferred bytecode set: SistaV1 ``` ``` Open Smalltalk Cog[Spur] VM [CoInterpreterPrimitives VMMaker.oscog-eem.2940] Unix built on Jun 9 2021 14:12:17 Compiler: Clang 10.0.0 platform sources revision VM: 202102090434 Date: Mon Feb 8 20:34:38 2021 CommitHash: 9f1b4644e Plugins: 202102090434 CoInterpreter VMMaker.oscog-eem.2940 uuid: 29171f50-756f-4560-8488-ced65f2d4841 Jun 9 2021 StackToRegisterMappingCogit VMMaker.oscog-eem.2940 uuid: 29171f50-756f-4560-8488-ced65f2d4841 Jun 9 2021 ```
Hi Stephan, (Hi Nicolas),
I’ll try and fix this soon. If you can, look at ThreadedFFIPlugin, and we would need to add slots in the specialObjectsArray for Float32Array and Float64Array so that the marshaling code can identify instances of these classes quickly. Better still, these classes could be given known class indices and then the marshaling code can identify instances of these classes even quicker :-)
_,,,^..^,,,_ (phone)
On Jun 10, 2021, at 12:36 AM, Stephan Lutz ***@***.***> wrote:
Given the external function defined by the following Squeak method:
myFunc: aFloat64Array
<apicall: void 'myFunc' (double*)> ^ self externalCallFailed When passing an instance of Float64Array to this method, the FFI plugin throws error code 6 (Could not coerce arguments). The same can be reproduced in similar scenarios for other 64-bit indexable objects like DoubleWordArray and SignedDoubleWordArray (which ultimately share the same class format anyway). There were no issues when doing the same using 32-bit indexable objects.
As far as I can tell the issue seems to stem from ThreadedFFIPlugin >> ffiAtomicArgByReference:Class:in: which ends with the following code:
(atomicType <= FFITypeDoubleFloat) ifTrue: [((interpreterProxy isWords: oop) or: [oopClass = interpreterProxy classByteArray]) ifTrue: [^self ffiPushPointer: (interpreterProxy firstIndexableField: oop) in: calloutState]. (oopClass = interpreterProxy classExternalAddress) ifTrue: [^self ffiPushPointer: (self longAt: oop + interpreterProxy baseHeaderSize) in: calloutState]. isAlien ifTrue: [^self ffiPushPointer: (self pointerForOop: (self startOfData: oop)) in: calloutState]].
^FFIErrorCoercionFailed While the second line checks 32-bit indexable via isWords: and byte arrays, it seems to be missing a test for isLong64s:.
Just in case it's relevant for some reason:
Squeak latest update: #20467 Image format 68021 (64 bit) Preferred bytecode set: SistaV1 Open Smalltalk Cog[Spur] VM [CoInterpreterPrimitives VMMaker.oscog-eem.2940] Unix built on Jun 9 2021 14:12:17 Compiler: Clang 10.0.0 platform sources revision VM: 202102090434 Date: Mon Feb 8 20:34:38 2021 CommitHash: 9f1b4644e Plugins: 202102090434 CoInterpreter VMMaker.oscog-eem.2940 uuid: 29171f50-756f-4560-8488-ced65f2d4841 Jun 9 2021 StackToRegisterMappingCogit VMMaker.oscog-eem.2940 uuid: 29171f50-756f-4560-8488-ced65f2d4841 Jun 9 2021 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.
Hi Stephan,
abd excuse me. You have indeed identified exactly the location of the issue. Replying on my phone I hadn’t scrolled down far enough. So the solution involves extending InterpreterProxy with the relevant support functions and/or determining the class indices for the float arrays.
Since being able to use the pure puts arrays is so useful, eg passing in a byte array for a double * parameter, not just a 64-bit array, we really need InterpreterProxy to support isPureBits:. Arguably exposing format so marshalling and plug-in code can identify the indexable size of a pure bits array is more useful than locking in the plugin to spur.
_,,,^..^,,,_ (phone)
On Jul 2, 2021, at 5:25 AM, Eliot Miranda ***@***.***> wrote:
Hi Stephan, (Hi Nicolas),
I’ll try and fix this soon. If you can, look at ThreadedFFIPlugin, and we would need to add slots in the specialObjectsArray for Float32Array and Float64Array so that the marshaling code can identify instances of these classes quickly. Better still, these classes could be given known class indices and then the marshaling code can identify instances of these classes even quicker :-)_,,,^..^,,,_ (phone)
On Jun 10, 2021, at 12:36 AM, Stephan Lutz ***@***.***> wrote:
Given the external function defined by the following Squeak method:
myFunc: aFloat64Array
<apicall: void 'myFunc' (double*)> ^ self externalCallFailed When passing an instance of Float64Array to this method, the FFI plugin throws error code 6 (Could not coerce arguments). The same can be reproduced in similar scenarios for other 64-bit indexable objects like DoubleWordArray and SignedDoubleWordArray (which ultimately share the same class format anyway). There were no issues when doing the same using 32-bit indexable objects.
As far as I can tell the issue seems to stem from ThreadedFFIPlugin >> ffiAtomicArgByReference:Class:in: which ends with the following code:
(atomicType <= FFITypeDoubleFloat) ifTrue: [((interpreterProxy isWords: oop) or: [oopClass = interpreterProxy classByteArray]) ifTrue: [^self ffiPushPointer: (interpreterProxy firstIndexableField: oop) in: calloutState]. (oopClass = interpreterProxy classExternalAddress) ifTrue: [^self ffiPushPointer: (self longAt: oop + interpreterProxy baseHeaderSize) in: calloutState]. isAlien ifTrue: [^self ffiPushPointer: (self pointerForOop: (self startOfData: oop)) in: calloutState]].
^FFIErrorCoercionFailed While the second line checks 32-bit indexable via isWords: and byte arrays, it seems to be missing a test for isLong64s:.
Just in case it's relevant for some reason:
Squeak latest update: #20467 Image format 68021 (64 bit) Preferred bytecode set: SistaV1 Open Smalltalk Cog[Spur] VM [CoInterpreterPrimitives VMMaker.oscog-eem.2940] Unix built on Jun 9 2021 14:12:17 Compiler: Clang 10.0.0 platform sources revision VM: 202102090434 Date: Mon Feb 8 20:34:38 2021 CommitHash: 9f1b4644e Plugins: 202102090434 CoInterpreter VMMaker.oscog-eem.2940 uuid: 29171f50-756f-4560-8488-ced65f2d4841 Jun 9 2021 StackToRegisterMappingCogit VMMaker.oscog-eem.2940 uuid: 29171f50-756f-4560-8488-ced65f2d4841 Jun 9 2021 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.
Closed #569.
See commit ae0c1a6b473aa5ed05433d91a3257804aa1ddd90 Author: Eliot Miranda eliot.miranda@gmail.com Date: Sat Sep 11 16:19:04 2021 -0700
CogVM source as per VMMaker.oscog-eem.3068
ThreadedFFIPlugin: pass any kind of non-pointer object to non-byte */char * args. We could insist on void *, but let's see how we get on with this.
vm-dev@lists.squeakfoundation.org
-
Eliot Miranda -
Stephan Lutz