Closed Bug 630064 Opened 9 years ago Closed 9 years ago

MethodJIT: Assertion failure: (ptrBits & 0x7) == 0 // GC related crash

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
Tracking Status
blocking2.0 --- final+

People

(Reporter: decoder, Assigned: luke)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase, Whiteboard: [sg:critical?][hardblocker][has patch][fixed-in-tracemonkey])

Attachments

(2 files)

Attached testcase crashes on 64 bit TM tip (run with -m) and crashes when continuing.

Backtrace:

Program received signal SIGABRT, Aborted.
0x00007ffff7bd1ebb in raise () from /lib/libpthread.so.0
(gdb) bt
#0  0x00007ffff7bd1ebb in raise () from /lib/libpthread.so.0
#1  0x000000000059909c in JS_Assert (s=0x7186ea "(ptrBits & 0x7) == 0", file=0x718618 "jsval.h", ln=723) at jsutil.cpp:83
#2  0x0000000000414754 in JSVAL_TO_GCTHING_IMPL (l=...) at jsval.h:723
#3  0x000000000043261e in js::Value::toGCThing (this=0xb50eb0) at jsvalue.h:615
#4  0x00000000004d2c2d in js::gc::MarkValueRaw (trc=0x7fffffffc280, v=...) at jsgcinlines.h:599
#5  0x00000000004d2cec in js::gc::MarkValueRange (trc=0x7fffffffc280, beg=0xb50ea8, end=0xb50ed0, name=0x736b9d "generator slots") at jsgcinlines.h:616
#6  0x00000000004d5703 in generator_trace (trc=0x7fffffffc280, obj=0x7ffff6903438) at jsiter.cpp:1085
#7  0x00000000004f6a73 in js_TraceObject (trc=0x7fffffffc280, obj=0x7ffff6903438) at jsobj.cpp:6471
#8  0x00000000004d2261 in js::gc::MarkChildren (trc=0x7fffffffc280, obj=0x7ffff6903438) at jsgcinlines.h:289
#9  0x00000000004d23f7 in js::gc::TypedMarker (trc=0x7fffffffc280, thing=0x7ffff6903438) at jsgcinlines.h:347
#10 0x00000000004d68a6 in js::gc::Mark<JSObject> (trc=0x7fffffffc280, thing=0x7ffff6903438) at jsgcinlines.h:222
#11 0x00000000004d2bba in js::gc::MarkKind (trc=0x7fffffffc280, thing=0x7ffff6903438, kind=0) at jsgcinlines.h:579
#12 0x00000000004d2c78 in js::gc::MarkValueRaw (trc=0x7fffffffc280, v=...) at jsgcinlines.h:600
#13 0x00000000004d2cec in js::gc::MarkValueRange (trc=0x7fffffffc280, beg=0xb50f78, end=0xb50fa8, name=0x736b9d "generator slots") at jsgcinlines.h:616
#14 0x00000000004d5703 in generator_trace (trc=0x7fffffffc280, obj=0x7ffff6903480) at jsiter.cpp:1085
#15 0x00000000004f6a73 in js_TraceObject (trc=0x7fffffffc280, obj=0x7ffff6903480) at jsobj.cpp:6471
#16 0x00000000004e34bd in js::gc::MarkChildren (trc=0x7fffffffc280, obj=0x7ffff6903480) at jsgcinlines.h:289
#17 0x00000000004e3653 in js::gc::TypedMarker (trc=0x7fffffffc280, thing=0x7ffff6903480) at jsgcinlines.h:347
#18 0x00000000004f8de5 in js::gc::Mark<JSObject> (trc=0x7fffffffc280, thing=0x7ffff6903480) at jsgcinlines.h:222
#19 0x00000000004e3cfe in js::gc::MarkKind (trc=0x7fffffffc280, thing=0x7ffff6903480, kind=0) at jsgcinlines.h:579
#20 0x00000000004e3dbc in js::gc::MarkValueRaw (trc=0x7fffffffc280, v=...) at jsgcinlines.h:600
#21 0x00000000004f6b75 in js_TraceObject (trc=0x7fffffffc280, obj=0x7ffff690e0d0) at jsobj.cpp:6497
#22 0x00000000004ad577 in js::gc::MarkChildren (trc=0x7fffffffc280, obj=0x7ffff690e0d0) at jsgcinlines.h:289
#23 0x00000000004ad7b0 in js::gc::TypedMarker (trc=0x7fffffffc280, thing=0x7ffff690e0d0) at jsgcinlines.h:347
#24 0x00000000004b7afe in js::gc::Mark<JSObject_Slots4> (trc=0x7fffffffc280, thing=0x7ffff690e0d0) at jsgcinlines.h:222
#25 0x00000000004be04e in js::gc::Arena<JSObject_Slots4>::mark (this=0x7ffff690e000, thing=0x7ffff690e0d0, trc=0x7fffffffc280) at jsgc.cpp:226
#26 0x00000000004b2ead in js::MarkCell<JSObject_Slots4> (cell=0x7ffff690e0d0, trc=0x7fffffffc280) at jsgc.cpp:583
#27 0x00000000004b93f0 in js::MarkIfGCThingWord (trc=0x7fffffffc280, w=140737330077904, thingKind=@0x7fffffffc0d8) at jsgc.cpp:652
#28 0x00000000004aed07 in js::MarkWordConservatively (trc=0x7fffffffc280, w=140737330077904) at jsgc.cpp:712
#29 0x00000000004aee35 in js::MarkRangeConservatively (trc=0x7fffffffc280, begin=0x7fffffffc390, end=0x7ffffffff000) at jsgc.cpp:743
#30 0x00000000004aeee8 in js::MarkThreadDataConservatively (trc=0x7fffffffc280, td=0x7ffff7fc33a8) at jsgc.cpp:760
#31 0x00000000004aef8a in js::MarkConservativeStackRoots (trc=0x7fffffffc280) at jsgc.cpp:800
#32 0x00000000004b0a76 in js::MarkRuntime (trc=0x7fffffffc280) at jsgc.cpp:1646
#33 0x00000000004b1e20 in MarkAndSweep (cx=0xae90e0, gckind=GC_NORMAL) at jsgc.cpp:2403
#34 0x00000000004b223e in GCUntilDone (cx=0xae90e0, comp=0x0, gckind=GC_NORMAL) at jsgc.cpp:2728
#35 0x00000000004b2419 in js_GC (cx=0xae90e0, comp=0x0, gckind=GC_NORMAL) at jsgc.cpp:2799
#36 0x0000000000427f12 in JS_GC (cx=0xae90e0) at jsapi.cpp:2568
#37 0x0000000000407424 in GC (cx=0xae90e0, argc=0, vp=0x7ffff6abb1d0) at js.cpp:1316
#38 0x00000000004d04e6 in js::CallJSNative (cx=0xae90e0, native=0x4073df <GC(JSContext*, uintN, jsval*)>, argc=0, vp=0x7ffff6abb1d0) at jscntxtinlines.h:697
#39 0x000000000069ab55 in CallCompiler::generateNativeStub (this=0x7fffffffcdf0) at ./methodjit/MonoIC.cpp:667
#40 0x00000000006970aa in js::mjit::ic::NativeCall (f=..., ic=0xb4e8f8) at ./methodjit/MonoIC.cpp:874
#41 0x00007ffff7f9667c in ?? ()
#42 0x00007ffff7f95b90 in ?? ()
#43 0x0000000000000027 in ?? ()
#44 0x0000000000000000 in ?? ()


As the issue seems GC related and crashes, adding security lock here.
Looks bad from the stack.
blocking2.0: --- → ?
I think the bug here is that js_NewGenerator copies uninitialized stack values into gen->floatingFrame. From a brief look at the generator code, I don't think these values can be observed, and they'd be overwritten if they were to be observed.

So, two potential solutions if this is true:
 (1) mark floatingFrame conservatively
 (2) don't copy from the interpreter stack in js_NewGenerator?
blocking2.0: ? → final+
Whiteboard: softblocker
Ah, this is an argument-overflow situation.  Normally, I would have thought that the actual arguments would be synced and valid before FixupArity did any duplication that it had to do.  However, this is a call to 'new' and the invalid value is exactly the non-canonical 'this' slot so I bet this has to do with the fact that 'this' is filled in by the callee and thus is left uninitialized before FixupArity.  (1) sounds safe, then.  Its a bit of a sledgehammer though: I think this can only happen for the non-canonical 'this' slot of a call to 'new'.  I wonder if instead we should add:

  if (fp->isConstructing())
    fp->thisValue().setUndefined();

before getInlineFrameWithinLimit does the overflow-arg dupliation in FixupArity.  My reasoning is that, while the mjit can leave values in incoherent states, I thought it was in relatively restricted situations.  This seems like a different category of invalid value and perhaps it would be good not to have this corner case exist.
Assignee: general → lw
David wanted.
Assignee: lw → dvander
Bouncing back to Luke after some discussion.
Assignee: dvander → lw
Whiteboard: softblocker → [sg:critical?] softblocker
Attached patch Fix, finallySplinter Review
I added Debug_SetValueRangeToCrashOnTouch (for which I chose ObjectValue(*(JSObject *)0x42) since other variations on "set this to an invalid value" tend to pass silently through things like GC) and used this to poison the non-canonical args of generators and normal function calls.  In the process IsSaneThisObject got in the way, so I killed it since I think it adds little value for its noise (its just an assert we aren't pointing to an object class that shouldn't escape the scope chain, which is true of any value in any arg, slot, or prop).
Attachment #510428 - Flags: review?(dvander)
This is a sg:critical bug, with a patch, even. Needs to be a hardblocker.
Whiteboard: [sg:critical?] softblocker → [sg:critical?][hardblocker][has patch]
Attachment #510428 - Flags: review?(dvander) → review+
http://hg.mozilla.org/tracemonkey/rev/c2f23bc4cda7
Whiteboard: [sg:critical?][hardblocker][has patch] → [sg:critical?][hardblocker][has patch][fixed-in-tracemonkey]
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Blocks: 676763
Old bug not affecting 1.9.2 and has test with fix. Marking verified and opening.
Group: core-security
Status: RESOLVED → VERIFIED
A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/testBug630064.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.