[Vm-dev] there might be a bug in FFIPlugin ffiPushSignedLongLongOop: it might fail to fail

[Nicolas Cellier]

I confirm that VM 4.2.5 on Mac fails to fail...

Squeak4.1 of 17 April 2010 [latest update: #9957] Squeak VM 4.2.5b1
Mac Carbon 4.2.5b1 15-Jun-10 >85D9C693-2A2A-4C33-B05C-C20B2A63B166<
VMMaker versionString 4.2.4

and this is http://bugs.squeak.org/view.php?id=7720

2013/1/19 Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>:
> Ah OK, you mean it is already corrected in ThreadedFFIPlugin :)
> I checked COG, and it is OK.
> The issue might still hold for classical Interpreter VM...
>
> Thanks
>
> 2013/1/17 Eliot Miranda <eliot.miranda at gmail.com>:
>>
>> Hi Nicolas,
>>
>>
>> On Wed, Jan 16, 2013 at 3:00 PM, Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com> wrote:
>>>
>>>
>>> I'm not really sure, but I don't see any guard against too high a
>>> magnitude to fit in a signed long long...
>>> Thus the primitive migth fail to fail when passed (1<<63), though it
>>> is most probably > LLONG_MAX.
>>>
>>> If someone can check whether this is a real bug or not by simply using
>>> FFI (I haven't anything installed handy),
>>> then we'll can think about corrections (find some ideas below)
>>>
>>> Nicolas
>>
>>
>> The issue would have to be to do with signed64BitValueOf: since that's the converter in ffiPushSignedLongLongOop:in:
>>
>> ffiPushSignedLongLongOop: oop in: calloutState
>> <var: #calloutState type: #'CalloutState *'>
>> "Push a longlong type (e.g., a 64bit integer).
>> Note: Coercions from float are *not* supported."
>> | value |
>> <var: #value type: #sqLong>
>> (oop = interpreterProxy nilObject
>> or: [oop = interpreterProxy falseObject])
>> ifTrue:[value := 0] ifFalse:
>> [oop = interpreterProxy trueObject
>> ifTrue:[value := 1] ifFalse:
>> [value := interpreterProxy signed64BitValueOf: oop.
>> interpreterProxy failed ifTrue:
>> [^FFIErrorCoercionFailed]]].
>> ^self ffiPushSignedLongLong: value in: calloutState
>>
>> So provided that signed64BitValueOf: is correct ffiPushSignedLongLongOop:in: will be correct.  That can be tested independently of the FFI no?  I think it's tested by AlienSUnit>>testLongLong.
>>
>>
>>>
>>>
>>> ----------------------------------------------------------------
>>> My favorite way to do it now that C standards have evolved enough,
>>> would be something simple like:
>>>
>>>     <var: #value type: #'signed long long'>
>>>     <var: #magnitude type: #'unsigned long long'>
>>>
>>>     magnitude := objectMemory fetch... some bytes in a loop...
>>>    (or directly fetch 1 or 2 32 bit words whenever we decide to store
>>> LargeInt with VM native endianness 32 bit digits, which I'm currently
>>> exploring).
>>>
>>>         (negative
>>>                 ifTrue: [magnitude > 16r8000000000000000] "Once, we could have
>>> written 16r8e15..."
>>>                 ifFalse: [magnitude >= 16r8000000000000000])
>>>                         ifTrue: [self primitiveFail].
>>>         negative
>>>                 ifTrue: [value := 0 - magnitude]
>>>                 ifFalse: [value := magnitude].
>>>
>>> I'm partially happy with above code...
>>> hardcoded constant, are not that nice, but:
>>> * 2-complement though not guaranteed is quite ubiquitous (and we rely
>>> on it in many places)
>>> * 64 bits length is implicitely coded thru LargeInteger byte length
>>> (8) somewhere above (not accounting for CHAR_BIT != 8)
>>>
>>> Of course, I would be even more happy if we
>>> - used an int64_t - and  rename the primitive ;) - if we really mean
>>> it to be on 64 bits,
>>> - or did some assertion about sizeof(unsigned long long)
>>>  and replaced hardcoded constants with
>>>     if((negative)
>>>         ? magnitude >  - (unsigned)(LLONG_MIN)
>>>         : magnitude > (unsigned)(LLONG_MAX))
>>> See http://stackoverflow.com/questions/12231560/correct-way-to-take-absolute-value-of-int-min
>>> for germane discussion about standardness.
>>>
>>> Alas, I don't know how to use some already existing constants from
>>> some standard header in slang....
>>> Didn't someone ever think of using a standard constant? Should I dig
>>> cCode: '....'?
>>>
>>> Otherwise, the code currently found in #signed64BitValueOf: might be correct...
>>> It's just that it's too complex for me to check if it adheres to standards...
>>
>>
>>
>>
>> --
>> best,
>> Eliot
>>