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 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 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 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 Luke Wagner [:luke] 2010-07-26 20:55:48 PDT
Comment 4 Robert Sayre 2010-07-27 13:34:19 PDT
Branch patch?
Comment 7 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 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 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 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 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.