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

Eliot Miranda eliot.miranda at gmail.com
Thu Jan 17 17:18:03 UTC 2013


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20130117/f172b29c/attachment.htm


More information about the Vm-dev mailing list