There is no bound check when marshalling FFI atomic integer arguments

[nicolas-cellier-aka-nice]

When marshalling integer values, the value is first extracted from the Smalltalk oop via the method ffiIntegerValueOf: .

• SmallInteger -> SmallInteger value (signed value on 31 or 61 bits on 32bits and 64bits VM)
• false/true -> 0/1
• nil -> 0
• Character -> Character value (may be more than 8 bits long!)
• LargePositiveInteger -> 32bits positive integer value (or fail) (or 64 bits if ThreadedFFIPlugin)
• LargeNegativeInteger -> 32bits signed integer value (or fail) (or 64bits if ThreadedFFIPlugin)

Then the value is cast to the expected value in ffiArgByValue: which will have the effect of silently masking potential errors... (1<<31 could be passed to a signed int and re-interpreted as -(1<<31) for example).

IMO we could enforce bound checks and raise an Error... We should at least do this in the debug version if ever there is a concern about speed (but I don't think so).

[nicolas-cellier-aka-nice]

Note: we also have a tolerance for passing a pointer on integer types of equal size but wrong signedness... See code like (atomicType >> 1) = (specType >> 1) in

• ffiValidateExternalData:AtomicType: for passing agrument to an aliased type by reference
• ffiAtomicArgByReference:Class: which has the tolerance for signed/unsigned char/byte only

If we are enforcing bound checks, we might have unconsistent behavior And we might have code that depend on this liberality due to platforms where char are signed...

[OpenSmalltalk-Bot]

On Wed, 18 Apr. 2018, 15:56 Nicolas Cellier, [email protected] wrote:

Note: we also have a tolerance for passing a pointer on integer types of equal size but wrong signedness... See code like(atomicType >> 1) = (specType >> 1) in

• ffiValidateExternalData:AtomicType: for passing agrument to an aliased type by reference
• ffiAtomicArgByReference:Class: which has the tolerance for signed/unsigned char/byte only

If we are enforcing bound checks, we might have unconsistent behavior And we might have code that depend on this liberality due to platforms where char are signed...

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/251#issuecomment-382298579, or mute the https://github.com/notifications/unsubscribe-auth/AhLyW5Dajowoj2MTkljp2ttQEvwmeBH4ks5tpvGOgaJpZM4TZkRD

Naive question... Nowadays is uint8_t a better choice than char?

Cheers -ben

[nicolas-cellier-aka-nice]

We are talking about Foreign Function, so we have to bear other's choice. But we also can take full control at the Interface. For this, we support 4 different FFI types:

• sbyte and byte if the intention is to handle 8 bit integer (signed and unsigned respectively)
• schar and char if the intention is to handle 8 bit character code (signed and unsigned respectively)

With those types, we can re-interpret precisely the intentions of external library thru our <cdecl: returnType "function_prototype"(argType)> declarations.

The question raised is more about changing the contract and making a Smalltalk module fail when it previously worked: such precise control was not always necessary in the past.

The main usage is (const char*), where the foreign expectation is receiving a NULL terminated string. Since char is interpreted as unsigned by FFI - whatever the underlying machine/compiler dependent interpretation, and our character code are unsigned, the main usage should not be a problem.

Note that encoding has to be handled at an upper level, it's not explicitely described in FFI prototype, FFI is encoding-agnostic.

Note that we can pass a ByteString that will automatically be copied so as to have a NULL terminator. Most ByteString are already Null terminated in Spur, unless their size is a multiple of 8, so we could optimize further and avoid a copy, but that's another subject.