Last Comment Bug 581784 - TM: Crash [@ JSObject::getClass]
: TM: Crash [@ JSObject::getClass]
[sg:critical?][ccbr] fixed-in-tracemo...
: crash, regression, testcase, verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Mac OS X
-- critical (vote)
: ---
Assigned To: Luke Wagner [:luke]
: Jason Orendorff [:jorendorff]
Depends on:
Blocks: jsfunfuzz
  Show dependency treegraph
Reported: 2010-07-25 02:48 PDT by Gary Kwong [:gkw] [:nth10sd]
Modified: 2013-02-25 16:53 PST (History)
12 users (show)
choller: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

fix (340 bytes, patch)
2010-07-25 13:52 PDT, Luke Wagner [:luke]
gal: review+
dvander: review+
Details | Diff | Splinter Review

Description User image Gary Kwong [:gkw] [:nth10sd] 2010-07-25 02:48:52 PDT
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 User image Gary Kwong [:gkw] [:nth10sd] 2010-07-25 03:25:57 PDT
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 User image Luke Wagner [:luke] 2010-07-25 13:52:13 PDT
Created attachment 460167 [details] [diff] [review]

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 User image Luke Wagner [:luke] 2010-07-26 20:55:48 PDT
Comment 4 User image Robert Sayre 2010-07-27 13:34:19 PDT
Branch patch?
Comment 7 User image Al Billings [:abillings] 2010-08-10 16:43:17 PDT
Gary, can you verify this fix on debug 1.9.1 and 1.9.2 as reporter?
Comment 8 User image Gary Kwong [:gkw] [:nth10sd] 2010-08-10 23:02:15 PDT
(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> 

$ jsfunfuzz-dbg-32-192-34511-c3119c586090/js-dbg-32-192-darwin -j
js> for (x = 0; x < 4; x++) {
    (z = Function)()
(function anonymous() {})
Comment 9 User image Al Billings [:abillings] 2010-08-10 23:25:13 PDT
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 User image Gary Kwong [:gkw] [:nth10sd] 2010-08-10 23:32:38 PDT
(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() {})
Comment 11 User image Christian Holler (:decoder) 2013-02-25 16:53:56 PST
Bug in removed tracer code, setting in-testsuite- flag.

Note You need to log in before you can comment on or make changes to this bug.