465688 - TM: "Assertion failure: (m != JSVAL_INT) || isInt32(*vp)" with (-0x80000000)

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

js> for each (let d in [-0x80000000, -0x80000000]) - -d;

Assertion failure: (m != JSVAL_INT) || isInt32(*vp), at ../jstracer.cpp:1762

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: proposed fix

We need to check that the value won't become a double in the interpreter.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Pushed fix as changeset http://hg.mozilla.org/tracemonkey/rev/41fe2911e964

Comment 3

[Brendan Eich [:brendan]]

Comment on attachment 349527 [details] [diff] [review]
proposed fix

>diff -r 6d441358bc57 js/src/jstracer.cpp
>--- a/js/src/jstracer.cpp	Fri Nov 21 15:31:59 2008 -0800
>+++ b/js/src/jstracer.cpp	Fri Nov 21 17:04:55 2008 -0800
>@@ -5648,7 +5648,8 @@
>          */
>         if (isPromoteInt(a) &&
>             (!JSVAL_IS_INT(v) || JSVAL_TO_INT(v) != 0) &&
>-            (!JSVAL_IS_DOUBLE(v) || !JSDOUBLE_IS_NEGZERO(*JSVAL_TO_DOUBLE(v))))  {
>+            (!JSVAL_IS_DOUBLE(v) || !JSDOUBLE_IS_NEGZERO(*JSVAL_TO_DOUBLE(v))) &&
>+            -asNumber(v) == (int)-asNumber(v)) {

This is wrong if you are trying to test whether v contains an integral value that fits in a jsval -- use INT_FITS_IN_JSVAL.

/be

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

I think it's okay if it doesn't fit into a JSVAL_INT, since we use full 32-bit integers on trace and unbox/box from doubles that fit into int32.

Comment 5

[Brendan Eich [:brendan]]

(In reply to comment #1)
> Created an attachment (id=349527) [details]
> proposed fix
> 
> We need to check that the value won't become a double in the interpreter.

I read this before composing comment 3.

(In reply to comment #4)
> I think it's okay if it doesn't fit into a JSVAL_INT, since we use full 32-bit
> integers on trace and unbox/box from doubles that fit into int32.

Ok, so (double checking) did comment 1 say "in the interpreter" when it should have said "on trace"?

/be

Comment 6

[Bob Clary [:bc] (inactive)]

fixed on tracemonkey, but not mozilla-central. reopening.

Comment 7

[Bob Clary [:bc] (inactive)]

Checking in js1_8/regress/regress-465688.js;
/cvsroot/mozilla/js/tests/js1_8/regress/regress-465688.js,v  <--  regress-465688.js
initial revision: 1.1
done

Comment 8

[Shawn Wilsher :sdwilsh]

http://hg.mozilla.org/mozilla-central/rev/f972e6edfa55

Comment 9

[Mike Shaver (:shaver emeritus)]

FIXED, per comment 8.

Comment 10

[Mike Shaver (:shaver emeritus)]

Scratch that.

Comment 11

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/3622d7662bae

Comment 12

[Bob Clary [:bc] (inactive)]

test included in js1_8_1/trace/trace-test.js 
http://hg.mozilla.org/mozilla-central/rev/8f967a7729e2

Comment 13

[Bob Clary [:bc] (inactive)]

v 1.9.1, 1.9.2