[Vm-dev] Interpreter>>signed32BitValueOf: and signed64BitValueOf: broken

[Andreas Raab]

Oh, and for the records signed64BitIntegerFor is broken too and in more 
than one way: First the short-cut into 32bit is wrong since it tests the 
magnitude and allows signed integers with 32 bits of magnitude to slip 
through (reverse problem of the one described below), and of course, it 
creates non-normalized LargeIntegers which will compare wrongly. If you 
have a file that exceeds 32 bits you can test this via:

   file size = (file size + 0)

which will evaluate to false.

Cheers,
   - Andreas

Andreas Raab wrote:
> 
> Hi -
> 
> I just noticed hat Interpreter>>signed32BitValueOf: and 
> signed64BitValueOf: are broken for edge cases. The following example 
> will illustrate the problem:
> 
> array := IntegerArray new: 1.
> array at: 1 put: 16rFFFFFFFF. "should fail but doesn't"
> array at: 1. "answers -1 incorrectly"
> 
> array := IntegerArray new: 1.
> array at: 1 put: -16rFFFFFFFF. "should fail but doesn't"
> array at: 1. "answers 1 incorrectly"
> 
> The problem is that both signed32BitValueOf: as well as 
> signed64BitValueOf: do not test whether the high bit of the magnitude is 
> set (which it mustn't to fit into a signed integer). The fix is trivial 
> in both cases - basically all that's needed at the end of both functions 
> is this:
> 
>   "Filter out values out of range for the signed interpretation such as
>   16rFFFFFFFF (positive w/ bit 32 set) and -16rFFFFFFFF (negative w/ bit
>   32 set). Since the sign is implicit in the class we require that the
>   high bit of the magnitude is not set which is a simple test here"
>   value < 0 ifTrue:[^self primitiveFail].
>   negative
>     ifTrue:[^0 - value]
>     ifFalse:[^value]
> 
> Cheers,
>   - Andreas
>