581784 - TM: Crash [@ JSObject::getClass]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

for (x = 0; x < 4; x++) {
    (z = Function)()
}

crashes js debug shell with -j on TM tip at JSObject::getClass

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000004
0x0000f211 in JSObject::getClass (this=0x0) at jsobj.h:306
306             return clasp;
(gdb) bt
#0  0x0000f211 in JSObject::getClass (this=0x0) at jsobj.h:306
#1  0x0000f2ea in JSObject::isFunction (this=0x0) at jsfun.h:253
#2  0x0018027c in IsFunctionObject (v=@0x10000f8) at jsfun.h:277
#3  0x001849e5 in js::NativeToValue (cx=0x809400, v=@0x10000f8, type=JSVAL_TYPE_NONFUNOBJ, slot=0x84ec08) at ../jstracer.cpp:2825
#4  0x001c14ad in js::FlushNativeStackFrameVisitor::visitStackSlots (this=0xbfffdfa8, vp=0x10000f8, count=2, fp=0x10000a0) at ../jstracer.cpp:3003
#5  0x00185196 in js::VisitFrameSlots<js::FlushNativeStackFrameVisitor> (visitor=@0xbfffdfa8, cx=0x809400, depth=0, i=@0xbfffdf50, up=0x0) at ../jstracer.cpp:1787
#6  0x0018604e in js::VisitStackSlots<js::FlushNativeStackFrameVisitor> (visitor=@0xbfffdfa8, cx=0x809400, callDepth=0) at ../jstracer.cpp:1809
#7  0x001860bd in js::FlushNativeStackFrame (cx=0x809400, callDepth=0, mp=0x86cfc8, np=0x84ec00, stopFrame=0x0, ignoreSlots=0) at ../jstracer.cpp:3372
#8  0x0018d273 in js::LeaveTree (tm=0x8375d4, state=@0xbfffe458, lr=0x86cf74) at ../jstracer.cpp:7025
#9  0x0018d4fb in js::DeepBail (cx=0x809400) at ../jstracer.cpp:7952
#10 0x0007101c in js::LeaveTrace (cx=0x809400) at jscntxt.h:2959
#11 0x00071250 in js_GetTopStackFrame (cx=0x809400) at jscntxt.h:2995
#12 0x0007294e in Function (cx=0x809400, obj=0x606910, argc=0, argv=0xbfffe3b8, rval=0xbfffe3c0) at ../jsfun.cpp:2128
#13 0x003eff45 in ?? ()
#14 0x001816f0 in js::ExecuteTrace (cx=0x809400, f=0x864244, state=@0xbfffe458) at ../jstracer.cpp:6598
#15 0x0018e0db in js::ExecuteTree (cx=0x809400, f=0x864244, inlineCallCount=@0xbfffecf0, innermostNestedGuardp=0xbfffe534, lrp=0xbfffe538) at ../jstracer.cpp:6699
#16 0x0019fd22 in js::MonitorLoopEdge (cx=0x809400, inlineCallCount=@0xbfffecf0, reason=js::Record_Branch) at ../jstracer.cpp:7186
#17 0x00095bb9 in js::Interpret (cx=0x809400) at ../jsinterp.cpp:3386
#18 0x000b3588 in js::Execute (cx=0x809400, chain=0x602000, script=0x40c140, down=0x0, flags=0, result=0xbffff680) at jsinterp.cpp:904
#19 0x00017cae in JS_ExecuteScript (cx=0x809400, obj=0x602000, script=0x40c140, rval=0xbffff680) at ../jsapi.cpp:4662
#20 0x0000cc36 in Process (cx=0x809400, obj=0x602000, filename=0x0, forceTTY=0) at ../../shell/js.cpp:532
#21 0x0000d60d in ProcessArgs (cx=0x809400, obj=0x602000, argv=0xbffff84c, argc=1) at ../../shell/js.cpp:853
#22 0x0000d726 in shell (cx=0x809400, argc=1, argv=0xbffff84c, envp=0xbffff854) at ../../shell/js.cpp:4949
#23 0x0000d84a in main (argc=1, argv=0xbffff84c, envp=0xbffff854) at ../../shell/js.cpp:5036
(gdb) x/i $eip
0xf211 <_ZNK8JSObject8getClassEv+9>:    mov    0x4(%eax),%eax
(gdb) x/b $eax 
0x0:    Cannot access memory at address 0x0

Comment 1

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

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   47991:faf55b60e857
user:        Luke Wagner
date:        Tue Jul 20 19:32:04 2010 -0700
summary:     Bug 580187 - Pass vp to ComputeThisFromVp (r=dvander)

Comment 2

[Luke Wagner [:luke]]

Attachment: fix

It looks like this is a pre-existing bug caught be fatval's added assertions.  Bug 580187 just fixed a fatval bug that let fatval assert properly.

The problem is that TraceRecorder::callNative builds the correct this_ins value to pass to the slow native, but does not set this_ins in the tracker for vp[1].  Thus, on deep bail, the value of vp[1] is what it was before computing this, which seems to be NULL, hence the assertion when the snapshot says its supposed to be JSVAL_TYPE_OBJECT.  Modifying a pre-fatval revision to JS_ASSERT(!JSVAL_IS_NULL(v)) in NativeToValue for TT_OBJECT also fails on the test in comment 0.

The fact that this only asserts on OS X means that the value of vp[1] that is produced by LeaveTree isn't deterministic, which makes sense since its just the last LIns* for vp[1] hanging in the tracker.  So, I think that makes this a security bug.

The patch is just the obvious set(&vp[1], this_ins).  It seems like the right place to put it, but I'm not quite so familiar with the subtleties of deep-bailing, so please check me on its placement.

Comment 3

[Luke Wagner [:luke]]

http://hg.mozilla.org/tracemonkey/rev/01fb7a9e1ae4

Comment 4

[Robert Sayre]

Branch patch?

Comment 5

[Luke Wagner [:luke]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/53d164e4a461
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f3e222eb33ba

Comment 6

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/01fb7a9e1ae4

Comment 7

[Al Billings [:abillings - ex-MoCo]]

Gary, can you verify this fix on debug 1.9.1 and 1.9.2 as reporter?

Comment 8

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

(In reply to comment #7)
> Gary, can you verify this fix on debug 1.9.1 and 1.9.2 as reporter?

Sure, verified. :)

$ jsfunfuzz-dbg-32-191-27038-e356f6a54a72/js-dbg-32-191-darwin -j
js> for (x = 0; x < 4; x++) {
    (z = Function)()
}

function anonymous() {
}
js> js> 
js> 
js> 

$ jsfunfuzz-dbg-32-192-34511-c3119c586090/js-dbg-32-192-darwin -j
js> for (x = 0; x < 4; x++) {
    (z = Function)()
}
(function anonymous() {})
js> 
js> 
js>

Comment 9

[Al Billings [:abillings - ex-MoCo]]

Well, unless you verified it on trunk as well, we should leave it resolved. I'll add the verified1.9.1 and verified1.9.2 keywords though.

Thank you.

Comment 10

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

(In reply to comment #9)
> Well, unless you verified it on trunk as well, we should leave it resolved.
> I'll add the verified1.9.1 and verified1.9.2 keywords though.

D'oh, sorry about that. Verified on trunk too. :)

$ jsfunfuzz-dbg-32-mc-49374-cbf493ff91ae/js-dbg-32-mc-darwin -j 
js> for (x = 0; x < 4; x++) {
    (z = Function)()
}
(function anonymous() {})
js> 
js>

Comment 11

[Christian Holler (:decoder)]

Bug in removed tracer code, setting in-testsuite- flag.