int wraparound bugs in 64bit builds

VERIFIED FIXED

Status

Tamarin
Virtual Machine
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: Edwin Smith, Unassigned)

Tracking

Details

Attachments

(1 attachment)

(Reporter)

Description

9 years ago
if you run as3/Types/Int/wraparound.abc, a number of tests will say PASSED even though they print the wrong answers.  

for example, comparing the 32bit and 64bit output (interpreter or jit, intel or ppc):

-uint.MIN_VALUE - 1 = 4294967295 = 4294967295 PASSED!
-uint.MIN_VALUE - 1 = 4294967295 = 4294967295 PASSED!
-decrement uint at uint.MIN_VALUE = 4294967295 PASSED!
-uint.MAX_VALUE * 2 = 4294967294 = 4294967294 PASSED!
+uint.MIN_VALUE - 1 = 4294967295 = -1 PASSED!
+uint.MIN_VALUE - 1 = 4294967295 = -1 PASSED!
+decrement uint at uint.MIN_VALUE = -1 PASSED!
+uint.MAX_VALUE * 2 = 4294967294 = -2 PASSED!

-uint.MAX_VALUE << 1 = 4294967294 PASSED!
+uint.MAX_VALUE << 1 = -2 PASSED!

(differential testing FTW!)

Comment 1

9 years ago
In the event that these are run in the interpreter; there is already an open bug (https://bugzilla.mozilla.org/show_bug.cgi?id=465754) on dodgy representations that may lead to strange bugs for int values in 64-bit builds.   (May or may not be related, just wanted to note it.)
(Reporter)

Comment 2

9 years ago
Looks like the problem is with string formatting; in a couple places we assume
kIntegerAtom implies int32_t, which isn't right.  Pending fix does a range
check and does unsigned formatting if the value is > 0x7fffffff.  Another fix
would be to change ConvertIntegerToStringBase10 to take intptr_t instead of
int32_t, and internally use a 22 byte buffer instead of 12 byte.  This seems
safer; votes welcome.
(Reporter)

Comment 3

9 years ago
Created attachment 355904 [details] [diff] [review]
use uintToString for kIntegerType Atoms > 0x7fffffff
Attachment #355904 - Flags: review?(stejohns)

Updated

9 years ago
Attachment #355904 - Flags: review?(stejohns) → review+
(Reporter)

Updated

9 years ago
Blocks: 468445
(Reporter)

Comment 4

9 years ago
pushed changeset:   1267:2b497ebb2153
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 5

9 years ago
verified that the as3/Types/Int/wraparound.abc produces the correct values using 32 and 64bit shells
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.