[Vm-dev] there might be a bug in FFIPlugin
ffiPushSignedLongLongOop: it might fail to fail
nicolas.cellier.aka.nice at gmail.com
Fri Jan 18 23:22:23 UTC 2013
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