451666 - TM: Null-deref crash [@ TraceRecorder::stack] opening DOM inspector (2 cx / 1 thread&tm)

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

BUILD: Current mozilla-central with content and chrome jit enabled.

STEPS TO REPRODUCE:
1) Start Firefox (it points to the default start page)
2) Open DOM Inspector

ACTUAL RESULTS: Null-dereference crash

EXPECTED RESULTS: no crash.

ADDITIONAL DETAILS:  The issue is that we enter js_Interpret on some JSContext.  We don't have an fp on that JSContext yet.  We end up in TraceRecorder::test_property_cache, which calls js_LookupPropertyWithFlags.  This enters a resolve class hook (XBLResolve to be exact).  This resolve hook executes script on a different JSContext (I think i nthis case we have a listener from one document touching a node from another document or something along those lines).  So we reenter js_interpret.  The two jscontexts live on the same thread, so we pick up the TraceRecorder from the other JSContext and run with it.

Gdb debugging session (with stack trace trimmed to only show the relevant part):

(gdb) bt
#0  0x002e68b1 in TraceRecorder::stackval (this=0xfb582e0, n=0) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jstracer.cpp:2261
#1  0x002e6aeb in TraceRecorder::stack (this=0xfb582e0, n=0, i=0x2ec5014) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jstracer.cpp:2367
#2  0x002ef199 in TraceRecorder::record_JSOP_NULL (this=0xfb582e0) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jstracer.cpp:4451
#3  0x00251c92 in js_Interpret (cx=0xf06eea0) at jsopcode.tbl:172
#4  0x0025cdb3 in js_Execute (cx=0xf06eea0, chain=0xda87640, script=0xfb59730, down=0x0, flags=0, result=0xbfffa768) at jsinterp.cpp:1549
#5  0x001e307c in JS_EvaluateUCScriptForPrincipals (cx=0xf06eea0, obj=0xda87640, principals=0xa5b004, chars=0xf0c04e0, length=4, filename=0xf0b9fb8 "chrome://inspector/content/inspector.xml", lineno=575, rval=0xbfffa768) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jsapi.cpp:5054
#6  0x04504f58 in nsJSContext::EvaluateStringWithValue (this=0xf075350, aScript=@0xbfffa880, aScopeObject=0xda87640, aPrincipal=0x0, aURL=0xf0b9fb8 "chrome://inspector/content/inspector.xml", aLineNo=575, aVersion=180, aRetValue=0xbfffa87c, aIsUndefined=0xbfffa86c) at /Users/bzbarsky/mozilla/debug/mozilla/dom/src/base/nsJSEnvironment.cpp:1357
#7  0x044b2e1f in nsXBLProtoImplField::InstallField (this=0xf0c04a0, aContext=0xf075350, aBoundNode=0xda87640, aBindingDocURI=0xf0ba960, aDidInstall=0xbfffa8e4) at /Users/bzbarsky/mozilla/debug/mozilla/content/xbl/src/nsXBLProtoImplField.cpp:136
#8  0x0449fe92 in XBLResolve (cx=0xf0c5830, obj=0xda826c0, id=245381252, flags=5, objp=0xbfffa984) at /Users/bzbarsky/mozilla/debug/mozilla/content/xbl/src/nsXBLBinding.cpp:210
#9  0x0026f896 in js_LookupPropertyWithFlags (cx=0xf0c5830, obj=0xda826c0, id=245381252, flags=5, objp=0xbfffaaf0, propp=0xbfffaa30) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jsobj.cpp:3391
#10 0x002e85e8 in TraceRecorder::test_property_cache (this=0xfb582e0, obj=0xda87640, obj_ins=0x2ec5260, obj2=@0xbfffaaf0, pcval=@0xbfffaaec) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jstracer.cpp:2716
#11 0x002e979c in TraceRecorder::prop (this=0xfb582e0, obj=0xda87640, obj_ins=0x2ec5260, slot=@0xbfffab48, v_ins=@0xbfffab44) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jstracer.cpp:4268
#12 0x002eedd2 in TraceRecorder::getProp (this=0xfb582e0, obj=0xda87640, obj_ins=0x2ec5260) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jstracer.cpp:4383
#13 0x002f14eb in TraceRecorder::record_JSOP_GETTHISPROP (this=0xfb582e0) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jstracer.cpp:5588
#14 0x00259352 in js_Interpret (cx=0xf0c5830) at jsopcode.tbl:507
#15 0x0025e288 in js_Invoke (cx=0xf0c5830, argc=1, vp=0x1594f2c, flags=0) at jsinterp.cpp:1326
#16 0x00cd96a8 in nsXPCWrappedJSClass::CallMethod (this=0xcf34110, wrapper=0xfb2ef80, methodIndex=3, info=0x111ccd8, nativeParams=0xbfffc224) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp:1523

(gdb) frame 0
#0  0x002e68b1 in TraceRecorder::stackval (this=0xfb582e0, n=0) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jstracer.cpp:2261
2261	    jsval* sp = cx->fp->regs->sp;
(gdb) p cx
$12 = (JSContext *) 0xf0c5830
(gdb) p cx->fp
$13 = (JSStackFrame *) 0x0
(gdb) p cx->thread
$17 = (JSThread *) 0xfe6000
(gdb) frame 3
#3  0x00251c92 in js_Interpret (cx=0xf06eea0) at jsopcode.tbl:172
172	OPDEF(JSOP_NULL,      64, js_null_str,  js_null_str,  1,  0,  1, 19,  JOF_BYTE)
(gdb) p cx
$14 = (JSContext *) 0xf06eea0
(gdb) p cx->fp
$15 = (JSStackFrame *) 0xbfffa620
(gdb) p cx->thread
$18 = (JSThread *) 0xfe6000
(gdb) frame 10
#10 0x002e85e8 in TraceRecorder::test_property_cache (this=0xfb582e0, obj=0xda87640, obj_ins=0x2ec5260, obj2=@0xbfffaaf0, pcval=@0xbfffaaec) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jstracer.cpp:2716
2716	        int protoIndex = js_LookupPropertyWithFlags(cx, aobj, id, 0, &obj2, &prop);
(gdb) p cx
$16 = (JSContext *) 0xf0c5830

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Not-really-correct patch

This was my first thought, but then we destroy the recorder, so when we unwind back to the js_LookupPropertyWithFlags callsite we crash.

Can we somehow avoid using js_LookupPropertyWithFlags here, given that it can run arbitrary script?  If not, we need to handle arbitary script execution under TraceRecorder::test_property_cache.

Comment 2

[Brendan Eich [:brendan]]

Even easier: abort at top of js_Interpret if recording. We can't handle recording into a nested interpreter, and we know recording starts on a backward branch in a js_Interpret activation. So don't let a new one activate. We already have an abort at the end of the interpreter, to stop recording rather than fall off the end and unwind to an outer interpreter.

/be

Comment 3

[Boris Zbarsky [:bzbarsky]]

But the abort would destroy the tracer, no?  And then we'd unwind back into the tracer code?  Or were you thinking stop recording but leave the tracer be until we unwind?

Comment 4

[Andreas Gal :gal]

We could indicate this condition via an error flag in the recorder that will abort recording at the next possible opportunity.

Comment 5

[(not currently active) Ted Mielczarek]

bz notes that this looks suspiciously similar to the crash in bug 451904, which you can hit with only content JIT on.

Comment 7

[Brendan Eich [:brendan]]

This is really a dup of bug 451657. I'll leave the two open for a bit.

/be

Comment 9

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

Didn't read awesomebar carefully enough -- changeset 313b18ab249a's log message is not about this bug number.

Comment 10

[Brendan Eich [:brendan]]

Should be fixed now that bug 451657 is fixed. Please reopen with specifics if not.

/be

Comment 11

[Boris Zbarsky [:bzbarsky]]

It's not fixed.  Just following the steps to reproduce from comment 0 crashes exactly the same way.

One obvious reason is that the fix for bug 451657 depends on the state of the JSContext.  In this bug, as described in comment 0, we have two _different_ JSContexts, both on the same thread, so sharing a recorder.  JS executing on context #1 calls a resolve() hook, which executes some JS on context #2.  Since context #2 has executingTrace == 0, we call into the trace recorder, etc.

Comment 12

[Brendan Eich [:brendan]]

Oh, of course. Adjusting summary. I'll fix shortly.

/be

Comment 13

[Brendan Eich [:brendan]]

Attachment: fix

bz: if you have time to test this, that would be swell.

/be

Comment 14

[Brendan Eich [:brendan]]

Attachment: fix, v2

Works with and without JS_TRACER defined.

/be

Comment 15

[Brendan Eich [:brendan]]

Fixed on tracemonkey:

http://hg.mozilla.org/tracemonkey/rev/db4260e7ee13

/be

Comment 16

[Boris Zbarsky [:bzbarsky]]

Yeah, there we go.  That made things better.

Comment 17

[Brendan Eich [:brendan]]

Fixed on m-c now, but why didn't I cite the bug#? Oh well, some bugzilla elf may supply the hgweb link.

/be

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20080927020752 Minefield/3.1b1pre ID:20080927020752

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080927033433 Minefield/3.1b1pre ID:20080927033433