[Vm-dev] there might be a bug in FFIPlugin
ffiPushSignedLongLongOop: it might fail to fail
nicolas.cellier.aka.nice at gmail.com
Sat Jan 19 01:42:06 UTC 2013
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...
> 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)
>> 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:
>> ^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
>>> ifTrue: [magnitude > 16r8000000000000000] "Once, we could have
>>> written 16r8e15..."
>>> ifFalse: [magnitude >= 16r8000000000000000])
>>> ifTrue: [self primitiveFail].
>>> 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
>>> ? 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...
More information about the Vm-dev