Assertion failure: INT_FITS_IN_JSVAL(i), at js/src/jsinterp.cpp:3877 http://hg.mozilla.org/mozilla-central/index.cgi/file/ab42787eac97/js/src/jsinterp.cpp#l3877 STEPS TO REPRODUCE 1. run mochitest suite on Firefox, it crashes at: http://localhost:8888/tests/MochiKit_Unit_Tests/test_MochiKit-DateTime.html PLATFORMS AND BUILDS TESTED Crash occurs for Firefox debug build on Linux x86_64 (mozilla-central tip)

On 64-bit Linux: @sm-valgrind01~/m/20-ff/js/src> ./Linux_All_DBG.OBJ/js js> var i =28800000; var i =28800000; js> -i -i Assertion failure: INT_FITS_IN_JSVAL(i), at jsinterp.cpp:3877 Aborted ~

Changeset 7194bbc1c007 changed this: -#define INT_FITS_IN_JSVAL(i) ((jsuint)((i)+JSVAL_INT_MAX) <= 2*JSVAL_INT_MAX ) +#define INT_FITS_IN_JSVAL(i) ((jsuint)(i) + JSVAL_INT_MAX + 1 <= \ + 2 * JSVAL_INT_MAX + 1) The old definition uses a brendan trick where you check that an integer is in a certain range [a...b] by adding -a to it, converting to uint, and checking that the result is <= (b-a). The new definition gets the trick slightly wrong and thus doesn't work on 64-bit platforms. (JSVAL_INT_MAX is 64-bit.)

The patch uses explicit (unsigned type)(x - a) <= (unsigned type)(b - a) to fix my overzealous parentheses removal.

(In reply to comment #3) > Created an attachment (id=327273) [details] > v1 The patch is wrong. The problem is that according to C (or C++) standard the int overflow is undefined. The latest GCC takes advantage of this to "optimize" ((unsigned)((i) - (int)JSVAL_INT_MIN) <= \ (unsigned) (JSVAL_INT_MAX - JSVAL_INT_MIN)) as (unsigned)(i) <= 0xC0000000 To witness consider the following session: ~/s $ cat x.c #define JSVAL_INT_MIN (-(long)0x40000000) #define JSVAL_INT_MAX ((long)0x3FFFFFFF) #define INT_FITS_IN_JSVAL(i) \ ((unsigned)((i) - (int)JSVAL_INT_MIN) <= \ (unsigned) (JSVAL_INT_MAX - JSVAL_INT_MIN)) int f(int i) { return INT_FITS_IN_JSVAL(i); } ~/s $ gcc -c -Os x.c ; objdump -d x.o x.o: file format elf32-i386 Disassembly of section .text: 00000000 <f>: 0: 55 push %ebp 1: 31 c0 xor %eax,%eax 3: 89 e5 mov %esp,%ebp 5: 81 7d 08 00 00 00 c0 cmpl $0xc0000000,0x8(%ebp) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ c: 5d pop %ebp d: 0f 9d c0 setge %al 10: c3 ret It is interesting to note that the original code before the bug 432881 was landed could also suffer from such treatment while the code currently in mozilla-central on 32 bit platform is free from this treatment. Proper implementation of INT_FITS_IN_JSVAL should use unsigned arithmetic as shown by the following program: ~/s $ cat x.c #define JSVAL_INT_MIN (-(long)0x40000000) #define JSVAL_INT_MAX ((long)0x3FFFFFFF) #define INT_FITS_IN_JSVAL(i) \ ((unsigned)(i) - (unsigned)JSVAL_INT_MIN <= \ (unsigned)(JSVAL_INT_MAX - JSVAL_INT_MIN)) int f(int i) { return INT_FITS_IN_JSVAL(i); } ~/s $ gcc -c -Os x.c ; objdump -d x.o x.o: file format elf32-i386 Disassembly of section .text: 00000000 <f>: 0: 55 push %ebp 1: 89 e5 mov %esp,%ebp 3: 8b 45 08 mov 0x8(%ebp),%eax 6: 5d pop %ebp 7: 05 00 00 00 40 add $0x40000000,%eax c: f7 d0 not %eax e: c1 e8 1f shr $0x1f,%eax 11: c3 ret

The new version uses unsigned arithmetic to avoid triggering integer overflow with its undefined behavior.

i hit this on trunk from mozilla-central in the wild. visit: http://aramex.com/intro.aspx

Comment on attachment 327322 [details] [diff] [review] v2 Sorry, I should have caught that.

I pushed the patch from the comment 5 to mozilla-central: http://hg.mozilla.org/mozilla-central/index.cgi/rev/e8195d1f3e73

/cvsroot/mozilla/js/tests/ecma_3/Number/regress-442242-01.js,v <-- regress-442242-01.js initial revision: 1.1 mozilla-central: changeset: 16479:56312cb1b9fd