TM: Assertion failure: isInt32(*p)

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
9 years ago
5 years ago

People

(Reporter: bc, Assigned: dvander)

Tracking

(4 keywords)

Trunk
assertion, regression, testcase, verified1.9.1.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1.1 +
wanted1.9.0.x -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?], URL)

Attachments

(3 attachments)

(Reporter)

Description

9 years ago
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.
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

9 years ago
Confirmed.
So I found another assertion crash bug 502714 through drilling down a testcase (most likely related)
Created attachment 387069 [details]
testcase-a
Just what the doctor ordered...
Group: core-security

Comment 6

9 years ago
David is working on it. Agree with hiding the bug.
Assignee: general → dvander

Comment 7

9 years ago
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

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

Comment 9

9 years ago
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.
(Reporter)

Updated

9 years ago
Attachment #387069 - Attachment mime type: application/x-javascript → text/plain
(Assignee)

Comment 10

9 years ago
Created attachment 387122 [details] [diff] [review]
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.
Attachment #387122 - Flags: review?(gal)

Comment 11

9 years ago
Comment on attachment 387122 [details] [diff] [review]
proposed fix

This is some scary fix. We should fuzz this into oblivion.
Attachment #387122 - Flags: review?(gal) → review+

Comment 12

9 years ago
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 on attachment 387122 [details] [diff] [review]
proposed fix

Nit: major comments start like this

    /*
     * Blah blah blah...
     */

/be

Comment 14

9 years ago
The overnight fuzzing run didn't find any problems with this patch.
(Assignee)

Updated

9 years ago
Blocks: 502768
(Assignee)

Comment 15

9 years ago
Pushed with nits from comment #13 

http://hg.mozilla.org/tracemonkey/rev/395a17e2767f
(Assignee)

Updated

9 years ago
Duplicate of this bug: 502714
(Assignee)

Updated

9 years ago
Flags: blocking1.9.1.1?
(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.
Severity: normal → critical
Flags: in-testsuite?
Keywords: regression, testcase
Hardware: x86 → All
autoBisect indicates this is probably related to the upvar2 checkin but Brendan mentions in bug 502714 comment 9 that it's probably incidental.
Blocks: 452498
(Assignee)

Comment 19

9 years ago
Yes, this bug has existed since bug 469044 landed
Blocks: 469044

Comment 20

9 years ago
http://hg.mozilla.org/mozilla-central/rev/395a17e2767f
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
blocking1.9.1: --- → .2+
Flags: blocking1.9.1.1? → blocking1.9.1.1-
Whiteboard: [sg:investigate]

Comment 21

9 years ago
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?

Updated

9 years ago
Attachment #387122 - Flags: approval1.9.1.1?
renoming for blocking to go with comment 21
Flags: blocking1.9.1.1- → blocking1.9.1.1?
Whiteboard: [sg:investigate] → [sg:critical?]
(Assignee)

Comment 23

9 years ago
Indeed, this could let someone inject an arbitrary value as a pointer into a whole bunch of code paths.
blocking1.9.1: .2+ → needed
Flags: blocking1.9.1.1? → blocking1.9.1.1+
Attachment #387122 - Flags: approval1.9.1.1? → approval1.9.1.1+
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
Keywords: fixed1.9.1.1
(Assignee)

Updated

9 years ago
Keywords: fixed1.9.1.1 → verified1.9.1.1
(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
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
I can corroborate what Aaron's mentioned: I crash in 3.5 but not 3.5.1(build1).
(Assignee)

Comment 28

9 years ago
(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).
(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
(Assignee)

Comment 30

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

No.
(Reporter)

Comment 32

9 years ago
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 ?
(Assignee)

Comment 33

9 years ago
Created attachment 389052 [details] [diff] [review]
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

9 years ago
Great job catching this Bob.
(Reporter)

Comment 35

9 years ago
Aaron deserves the lion's share of credit.

Comment 36

9 years ago
Great job by both of you :) I am glad the QA process worked well here.
So I'm guessing this bug is not yet verified1.9.1.1 then....
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
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

9 years ago
aaronmt, if thats a debug build with the badly applied patch, thats expected.
blocking1.9.1: needed → ---
Flags: wanted1.9.0.x-
Blocks: 504706
Group: core-security
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.