502604 - TM: Assertion failure: isInt32(*p)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bob Clary [:bc] (inactive)]

http://www.innup.de/Druck/Flyer#Drucken:

Assertion failure: isInt32(*p), at /work/mozilla/builds/1.9.1-tracemonkey/mozilla/js/src/jstracer.cpp:2323

#0  JS_Assert (s=0x4317a7 "isInt32(*p)", file=0x43113c "/work/mozilla/builds/1.9.1-tracemonkey/mozilla/js/src/jstracer.cpp", ln=2323) at /work/mozilla/builds/1.9.1-tracemonkey/mozilla/js/src/jsutil.cpp:69
#1  0x003a7b89 in TraceRecorder::import (this=0x1989e990, base=0x14bb5818, offset=1988, p=0x17d3fac, t=TT_INT32, prefix=0x42df19 "global", index=4, fp=0x0) at /work/mozilla/builds/1.9.1-tracemonkey/mozilla/js/src/jstracer.cpp:2323
#2  0x003d2396 in ImportGlobalSlotVisitor::visitGlobalSlot (this=0xbfffae0c, vp=0x17d3fac, n=4, slot=239) at /work/mozilla/builds/1.9.1-tracemonkey/mozilla/js/src/jstracer.cpp:2399
#3  0x003a81d5 in VisitGlobalSlots<ImportGlobalSlotVisitor> (visitor=@0xbfffae0c, cx=0x151de00, globalObj=0x157e2c80, ngslots=7, gslots=0x840b00) at /work/mozilla/builds/1.9.1-tracemonkey/mozilla/js/src/jstracer.cpp:1305
#4  0x003b5a40 in TraceRecorder::import (this=0x1989e990, treeInfo=0x1989c920, sp=0x14bb4d64, stackSlots=9, ngslots=7, callDepth=0, typeMap=0x14bbb7f0) at /work/mozilla/builds/1.9.1-tracemonkey/mozilla/js/src/jstracer.cpp:2512
#5  0x003c7a62 in TraceRecorder::emitTreeCall (this=0x1989e990, inner=0x1989c7d0, exit=0x14bbb7b0) at /work/mozilla/builds/1.9.1-tracemonkey/mozilla/js/src/jstracer.cpp:3836
#6  0x003ce830 in js_RecordLoopEdge (cx=0x151de00, r=0x1989e990, inlineCallCount=@0xbfffb498) at /work/mozilla/builds/1.9.1-tracemonkey/mozilla/js/src/jstracer.cpp:4790
#7  0x003cef5d in js_MonitorLoopEdge (cx=0x151de00, inlineCallCount=@0xbfffb498) at /work/mozilla/builds/1.9.1-tracemonkey/mozilla/js/src/jstracer.cpp:5440
#8  0x002e9e0b in js_Interpret (cx=0x151de00) at /work/mozilla/builds/1.9.1-tracemonkey/mozilla/js/src/jsinterp.cpp:3941
#9  0x0030e98f in js_Execute (cx=0x151de00, chain=0x157e2c80, script=0x14ba6000, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1662
...

Occurs on Mac/Windows 1.9.1/1.9.2 at least
Aaron, this would be a good case for reproducing and reducing a testcase.

Comment 1

[Aaron Train [:aaronmt]]

Reproduced with 

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090704 Minefield/3.6a1pre

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5 (.NET CLR 3.5.30729)

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090704 Minefield/3.6a1pre

Comment 2

[Andreas Gal :gal]

Confirmed.

Comment 3

[Aaron Train [:aaronmt]]

So I found another assertion crash bug 502714 through drilling down a testcase (most likely related)

Comment 4

[Aaron Train [:aaronmt]]

Attachment: testcase-a

Comment 5

[Jeff Walden [:Waldo]]

Just what the doctor ordered...

Comment 6

[Andreas Gal :gal]

David is working on it. Agree with hiding the bug.

Comment 7

[Andreas Gal :gal]

Jesse, any idea why the fuzzer didn't hit this? I am worried about missing this kind of stuff. I wonder whether we can create a specialized unstable-tree-nesting fuzzer. I would feel a lot better if we can create a fuzzer that hits this bug, and then prove that it doesn't find anything else.

Comment 8

[Jesse Ruderman]

I'd have a better idea if I could see a totally-reduced testcase.

Comment 9

[Andreas Gal :gal]

david is working on a patch. we can probably reduce this some but its not trival. it involves a sequence of type transitions in a nested loop we have to mimic.

Comment 10

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

Attachment: proposed fix

There are two awesome bugs here, closely related. The excellent reduced test case found the first one, running in the browser revealed a second.

Full explanation is in the patch comments. trace-tests, the provided test cases in bug 502714 and here, and the link in comment #0, all work for me with this patch. I can run more tests later tonight, but I have to run now so I'm putting this up for review.

502714 can be considered a dup btw.

Comment 11

[Andreas Gal :gal]

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

This is some scary fix. We should fuzz this into oblivion.

Comment 12

[Jesse Ruderman]

First 20 minutes of (bug 465479) fuzzing with this patch found only bug 502777.  That bug is present even without this patch.  I'll give you another update in the morning.

Comment 13

[Brendan Eich [:brendan]]

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

Nit: major comments start like this

    /*
     * Blah blah blah...
     */

/be

Comment 14

[Jesse Ruderman]

The overnight fuzzing run didn't find any problems with this patch.

Comment 15

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

Pushed with nits from comment #13 

http://hg.mozilla.org/tracemonkey/rev/395a17e2767f

Comment 17

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #4)
> Created an attachment (id=387069) [details]
> testcase-a

Much-reduced testcase:

R = new Array('', '||||', '||||1x', '||||');
for (var i = 1;; i++) {
    for (j = 0; j < 3; j++) t = f1(R[i].split('|')[4])
}
function f1(foo) {
    YY = 0
    for (var i = 0; i < 2; i++)
    if (foo.indexOf('x') > 0) {
        YY = t
    }
}

Assertion failure: isNumber(*p) == (t == JSVAL_DOUBLE), at ../jstracer.cpp:1771 with  tm-29679-9cb0da88df44 changeset.

Comment 18

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect indicates this is probably related to the upvar2 checkin but Brendan mentions in bug 502714 comment 9 that it's probably incidental.

Comment 19

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

Yes, this bug has existed since bug 469044 landed

Comment 20

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/395a17e2767f

Comment 21

[Andreas Gal :gal]

This bug is the same kind someone exploited yesterday. We have a fix and its in m-c. Are we sure we don't want it for 3.5.1?

Comment 22

[Daniel Veditz [:dveditz]]

renoming for blocking to go with comment 21

Comment 23

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

Indeed, this could let someone inject an arbitrary value as a pointer into a whole bunch of code paths.

Comment 24

[Graydon Hoare :graydon]

Landed in 1.9.1 at gal's request. Smoketested against alexa here (plus JIT tests and such). Seemed ok.

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/315a381a0bb5

Comment 25

[Aaron Train [:aaronmt]]

(In reply to comment #24)
> Landed in 1.9.1 at gal's request. Smoketested against alexa here (plus JIT
> tests and such). Seemed ok.
> 
> http://hg.mozilla.org/releases/mozilla-1.9.1/rev/315a381a0bb5

Built 1.9.1 today and still getting the crash on the aforementioned URL with

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.1pre) Gecko/20090715 Shiretoko/3.5.1pre

Comment 26

[Aaron Train [:aaronmt]]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.1) Gecko/20090715 Firefox/3.5.1 apparently has the fix working without crash

Comment 27

[juan becerra [:juanb]]

I can corroborate what Aaron's mentioned: I crash in 3.5 but not 3.5.1(build1).

Comment 28

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

(In reply to comment #25)
> Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.1pre)
> Gecko/20090715 Shiretoko/3.5.1pre

What cset is your build from? (about:buildconfig).

Comment 29

[Aaron Train [:aaronmt]]

(In reply to comment #28)
> (In reply to comment #25)
> > Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.1pre)
> > Gecko/20090715 Shiretoko/3.5.1pre
> 
> What cset is your build from? (about:buildconfig).

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b91c9ee69e6e

Comment 30

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

Hrm, do you have a crash reporter link?

Comment 31

[Aaron Train [:aaronmt]]

(In reply to comment #30)
> Hrm, do you have a crash reporter link?

No.

Comment 32

[Bob Clary [:bc] (inactive)]

I see this assertion on mac loading http://www.innup.de/Druck/Flyer#Drucken with a build from 1.9.1 from this morning as well but not 1.9.2 or tracemonkey. I didn't do a clobber build though. Is this really fixed1.9.1.1 ?

Comment 33

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

Attachment: 1.9.1 patch on top of existing cset

I guess the patch that landed on 1.9.1 did not apply cleanly. Sorry for this mess, I should have checked beforehand and made a specific 1.9.1 patch.

Comment 34

[Andreas Gal :gal]

Great job catching this Bob.

Comment 35

[Bob Clary [:bc] (inactive)]

Aaron deserves the lion's share of credit.

Comment 36

[Andreas Gal :gal]

Great job by both of you :) I am glad the QA process worked well here.

Comment 37

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

So I'm guessing this bug is not yet verified1.9.1.1 then....

Comment 38

[Aaron Train [:aaronmt]]

Confirmed *NO* crash after update to 3.5.1 

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.1) Gecko/20090715 Firefox/3.5.1

Comment 39

[Aaron Train [:aaronmt]]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.2pre) Gecko/20090720 Shiretoko/3.5.2pre

Assertion failure: isInt32(*p), at /Users/mozilla/mozilla-1.9.1/js/src/jstracer.cpp:2098

Exception Type:  EXC_BREAKPOINT (SIGTRAP)
Exception Codes: 0x0000000000000002, 0x0000000000000000
Crashed Thread:  0

Thread 0 Crashed:
0   libmozjs.dylib                	0x002a3f72 JS_Assert + 64 (jsutil.cpp:69)
1   libmozjs.dylib                	0x002c97b9 TraceRecorder::import(nanojit::LIns*, int, long*, unsigned char, char const*, unsigned int, JSStackFrame*) + 75 (jstracer.cpp:2103)
2   libmozjs.dylib                	0x002cd476 TraceRecorder::import(TreeInfo*, nanojit::LIns*, unsigned int, unsigned int, unsigned int, unsigned char*) + 2120 (jstracer.cpp:2206)
3   libmozjs.dylib                	0x002cd939 TraceRecorder::emitTreeCall(nanojit::Fragment*, VMSideExit*) + 297 (jstracer.h:191)
4   libmozjs.dylib                	0x002cf3a9 js_RecordLoopEdge(JSContext*, TraceRecorder*, unsigned int&) + 1099
5   libmozjs.dylib                	0x002cf500 js_MonitorLoopEdge(JSContext*, unsigned int&) + 68
6   libmozjs.dylib                	0x0023fbf0 js_Interpret + 145056 (jsinterp.cpp:3875)
7   libmozjs.dylib                	0x0024656f js_Execute + 745 (jsinterp.cpp:1622)
8   libmozjs.dylib                	0x001d9fed JS_EvaluateUCScriptForPrincipals + 281 (jsapi.cpp:5145)
9   libgklayout.dylib             	0x12ebe12c nsJSContext::EvaluateString(nsAString_internal const&, void*, nsIPrincipal*, char const*, unsigned int, unsigned int, nsAString_internal*, int*) + 732 (nsJSEnvironment.cpp:1631)
10  libgklayout.dylib             	0x12c2b1f3 nsScriptLoader::EvaluateScript(nsScriptLoadRequest*, nsString const&) + 1475 (nsScriptLoader.cpp:686)

Comment 40

[Andreas Gal :gal]

aaronmt, if thats a debug build with the badly applied patch, thats expected.

Comment 41

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929