[Vm-dev] InterpreterSimulator

[Florin Mateoc]

On 3/9/2016 3:17 PM, Florin Mateoc wrote:
> Hi again,
>
> I think I found the bug: in method InterpreterPrimitives>>signed64BitValueOf: there seems to be an assumption (even
> mentioned in the method comment) that (on 32bit machines) largeIntegers have to be either 4 or 8 bytes.
> In this case we get a 5byte largeInteger, so we get the error. What I don't understand is where does this assumption
> come from, because it does not seem limited to this method.
> Also note that on BigEndian machines the code does not act upon this assumption, so it would not fail.
>
> Actually, I suspect that the assumption comes from "generalizing" the 32-bit one, since the methods seem to be copied
> and pasted.
> For the 32bit variant, the comment stated that "The object may be either a positive SmallInteger or a four-byte
> LargeInteger". But in this case it was correct, anything less than 4 bytes would not be a LargeInteger. When moving to
> 64bit, the same does not hold true. We can have largeIntegers with 4,5,6,7 or 8 bytes fitting in 64 bits.
>
> Also, speaking of BigEndian, it seems that, in the same class, the methods #magnitude64BitValueOf: and
> #positive64BitValueOf: do not take care of the BigEndian case.
>
> Cheers,
> Florin


For what it's worth, I did try the obvious (if what I suspect is correct) fix in #signed64BitValueOf: and it seemed to work:


    ...
    self cppIf: VMBIGENDIAN
        ifTrue:
            [value := objectMemory fetchByte: sz - 1 ofObject: oop.
             sz - 2 to: 0 by: -1 do: [:i |
                value := value << 8 + (objectMemory fetchByte: i ofObject: oop)]]
        ifFalse:
            [value := (objectMemory fetchLong32: 0 ofObject: oop) asUnsignedInteger.
             4 to: sz - 1 do: [:i |
                value := value << 8 + (objectMemory fetchByte: i ofObject: oop)]].


instead of the original:


    ...
    self cppIf: VMBIGENDIAN
        ifTrue:
            [value := objectMemory fetchByte: sz - 1 ofObject: oop.
             sz - 2 to: 0 by: -1 do: [:i |
                value := value << 8 + (objectMemory fetchByte: i ofObject: oop)]]
        ifFalse: [value := sz > 4
                        ifTrue: [objectMemory fetchLong64: 0 ofObject: oop]
                        ifFalse: [(objectMemory fetchLong32: 0 ofObject: oop) asUnsignedInteger]].