The following testcase asserts on mozilla-central revision 5f626f86b374 (run with --ion-eager): function jit(on) function GetContext() {} test(); function test() { function i () { } jit(true); for (var j = 0; j < 3; i) { 1 & Date; } }

This looks like a NULL deref in opt builds, however, I'm marking s-s for two reasons. One is that the crash signature differs between Valgrind and gdb: ==13191== Invalid read of size 4 ==13191== at 0x8285C1D: js::gc::MarkKind(JSTracer*, void**, JSGCTraceKind) (Heap.h:989) ==13191== by 0x8286388: js::gc::MarkValueRootRange(JSTracer*, unsigned int, JS::Value*, char const*) (Marking.cpp:461) ==13191== by 0x820D0E1: js::StackSpace::mark(JSTracer*) (Marking.h:174) ==13191== by 0x827B19B: js::gc::MarkRuntime(JSTracer*, bool) (RootMarking.cpp:764) ==13191== by 0x80CD182: IncrementalCollectSlice(JSRuntime*, long long, js::gcreason::Reason, js::JSGCInvocationKind) (jsgc.cpp:2650) ==13191== by 0x80CE02C: GCCycle(JSRuntime*, bool, long long, js::JSGCInvocationKind, js::gcreason::Reason) (jsgc.cpp:4007) ==13191== by 0x80CE38A: _ZL7CollectP9JSRuntimebxN2js18JSGCInvocationKindENS1_8gcreason6ReasonE.part.278 (jsgc.cpp:4125) ==13191== by 0x8091A61: js_HandleExecutionInterrupt(JSContext*) (jscntxt.cpp:1287) ==13191== by 0x83E57FF: js::ion::InterruptCheck(JSContext*) (VMFunctions.cpp:424) ==13191== by 0x5543CCB: ??? ==13191== Address 0x0 is not stack'd, malloc'd or (recently) free'd vs. Program received signal SIGSEGV, Segmentation fault. compartment (this=0x0) at ../gc/Heap.h:989 989 return arenaHeader()->compartment; (gdb) bt #0 compartment (this=0x0) at ../gc/Heap.h:989 #1 MarkInternal<JSObject> (thingp=0xffffc28c, trc=0x857dddc) at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:152 #2 js::gc::MarkKind (trc=0x857dddc, thingp=0xffffc28c, kind=JSTRACE_OBJECT) at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:339 #3 0x08286389 in MarkValueInternal (v=0xf76970b0, trc=0x857dddc) at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:461 #4 MarkValueInternal (v=0xf76970b0, trc=0x857dddc) at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:513 #5 js::gc::MarkValueRootRange (trc=0x857dddc, len=2, vec=0xf76970b0, name=0x849f27b "vm_stack") at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:518 #6 0x0820d0e2 in MarkValueRootRange (name=0x849f27b "vm_stack", end=<optimized out>, begin=<optimized out>, trc=0x857dddc) at ../gc/Marking.h:174 #7 markFrame (slotsEnd=<optimized out>, fp=0xf7697070, trc=0x857dddc, this=<optimized out>) at /srv/repos/mozilla-central/js/src/vm/Stack.cpp:643 #8 js::StackSpace::mark (this=0x857dd00, trc=0x857dddc) at /srv/repos/mozilla-central/js/src/vm/Stack.cpp:665 #9 0x0827b19c in js::gc::MarkRuntime (trc=0x857dddc, useSavedRoots=false) at /srv/repos/mozilla-central/js/src/gc/RootMarking.cpp:764 #10 0x080cd183 in BeginMarkPhase (rt=0x857dcd8) at /srv/repos/mozilla-central/js/src/jsgc.cpp:2650 #11 IncrementalCollectSlice (rt=0x857dcd8, budget=0, reason=js::gcreason::TOO_MUCH_MALLOC, gckind=js::GC_NORMAL) at /srv/repos/mozilla-central/js/src/jsgc.cpp:3820 [...] and the second reason is that there is a lot of "Use of uninitialized value" observable in Valgrind: ==13191== Use of uninitialised value of size 4 ==13191== at 0x8282FEB: PushMarkStack(js::GCMarker*, js::types::TypeObject*) (Heap.h:672) ==13191== by 0x8285BFC: js::gc::MarkKind(JSTracer*, void**, JSGCTraceKind) (Marking.cpp:153) ==13191== by 0x827A1BC: MarkWordConservatively(JSTracer*, unsigned int) (RootMarking.cpp:237) ==13191== by 0x827A373: _ZL26MarkConservativeStackRootsP8JSTracerb.isra.70 (RootMarking.cpp:271) ==13191== by 0x827ADA0: js::gc::MarkRuntime(JSTracer*, bool) (RootMarking.cpp:683) ==13191== by 0x80CD182: IncrementalCollectSlice(JSRuntime*, long long, js::gcreason::Reason, js::JSGCInvocationKind) (jsgc.cpp:2650) ==13191== by 0x80CE02C: GCCycle(JSRuntime*, bool, long long, js::JSGCInvocationKind, js::gcreason::Reason) (jsgc.cpp:4007) ==13191== by 0x80CE38A: _ZL7CollectP9JSRuntimebxN2js18JSGCInvocationKindENS1_8gcreason6ReasonE.part.278 (jsgc.cpp:4125) ==13191== by 0x8091A61: js_HandleExecutionInterrupt(JSContext*) (jscntxt.cpp:1287) ==13191== by 0x83E57FF: js::ion::InterruptCheck(JSContext*) (VMFunctions.cpp:424) ==13191== by 0x5543CCB: ???

JSBugMon: Bisection requested, result: Due to skipped revisions, the first bad revision could be any of: changeset: 114661:0952f7c80055 user: Brian Hackett date: Fri Nov 30 15:59:52 2012 -0700 summary: Add analysis to eliminate dead resume point operands, bug 814997. r=dvander changeset: 114662:cd51ab1d33f5 user: Vincent Chang date: Fri Nov 30 19:37:34 2012 +0800 summary: Bug 815461 - wifi hotspot is not found after reboot. r=gal This iteration took 40.611 seconds to run.

Adding needinfo based on Comment 2.

Here's a stack trace from a debug build. Christian, it helps with triage if you include these in the report. It's often much more useful than opt-mode stacks. #0 0x0804b64a in OBJECT_TO_JSVAL_IMPL (obj=0x0) at ../../jsval.h:469 #1 0x0804b9df in JS::Value::setObject (this=0xffffc3d0, obj=...) at ../../jsapi.h:287 #2 0x080abd03 in js::ObjectValue (obj=...) at ../vm/ObjectImpl.h:1339 #3 0x08513a9b in js::ion::SnapshotIterator::FromTypedPayload (type= JSVAL_TYPE_OBJECT, payload=0) at /home/billm/mozilla/in1/js/src/ion/IonFrames.cpp:775 #4 0x08513cae in js::ion::SnapshotIterator::slotValue (this=0xffffc460, slot= ...) at /home/billm/mozilla/in1/js/src/ion/IonFrames.cpp:812 #5 0x08115e58 in js::ion::SnapshotIterator::read (this=0xffffc460) at ../ion/IonFrameIterator.h:229 #6 0x084ae9fb in js::StackFrame::initFromBailout (this=0xf76a8070, cx= 0x891edc0, iter=...) at /home/billm/mozilla/in1/js/src/ion/Bailouts.cpp:155 #7 0x084af325 in ConvertFrames (cx=0x891edc0, activation=0xffffc73c, it=...) at /home/billm/mozilla/in1/js/src/ion/Bailouts.cpp:291 #8 0x084af76d in js::ion::InvalidationBailout (sp=0xffffc5f4, frameSizeOut= 0xffffc5f0) at /home/billm/mozilla/in1/js/src/ion/Bailouts.cpp:399 #9 0xf769902a in ?? ()

(In reply to Bill McCloskey (:billm) from comment #4) > Here's a stack trace from a debug build. Christian, it helps with triage if > you include these in the report. It's often much more useful than opt-mode > stacks. The opt-mode stacks are usually required for security triage. I'll try to include both of them in future bugs then :)

The basic problem here is that the function is compiled multiple times, with later compilations extending the lifetime of the 'i' variable. In the first compilation uses of 'i' after its lifetime were removed from resume points, then we recompiled and read the (dead) undefined value written to 'i' back as an object. The lifetime changed because in the first compilation the phi for i at the loop header was redundant and eliminated (there being no OSR value included), and since that phi is not used anywhere the variable appeared totally dead when eliminating resume point operands after the initial lambda. In the second compilation, the phi still does not have any uses, but it is not redundant due to OSR and considered observable due to its being popped by a useless variable access in the loop. For eliminating dead resume point operands to work (possibly for eliminating unobservable phis as well, not sure), recompilation should not extend the lifetimes of SSA values. This can be done with the bytecodeUses mechanism used for phis, but that is too imprecise for eliminating resume point operands. The attached patch goes with what is basically the strategy rejected in bug 754713 comment 1, though to me it does not seem too onerous. Eliminating phis and resume point operands is one of the first passes done to the SSA, and the only concern is how IonBuilder and TypePolicy stuff can alter lifetimes. The attached patch watches for points in these passes which constant fold operations, and sets a bit to indicate that the ignored input cannot be eliminated later on. For phis, this replaces the bytecodeUses mechanism.

NULL deref, not s-s (variations of the underlying problem will still be NULL derefs). re: comment 1, those valgrind/gdb stacks are the same, and those uninitialized value warnings will show up any time the conservative stack scanner is in play.

A couple jit-tests were failing with the earlier patch.

(In reply to Brian Hackett (:bhackett) from comment #7) > re: comment 1, those valgrind/gdb stacks are the same, and those > uninitialized value warnings will show up any time the conservative stack > scanner is in play. Is that also the case with --enable-valgrind? All builds I test with valgrind are built with that flag and hence should not show these.

One thing to be aware of is that --enable-valgrind recently got broken see https://bugzilla.mozilla.org/show_bug.cgi?id=815931#c15 and I don't know whether the fix has landed yet.

Comment on attachment 690143 [details] [diff] [review] fix a couple bugs Review of attachment 690143 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/IonBuilder.cpp @@ +6217,5 @@ > // Property access is a known constant -- safe to emit. > JS_ASSERT(!testString || !testObject); > if (testObject) > current->add(MGuardObject::New(obj)); > else if (testString) Pre-existing nit, but indentation looks wrong.

(In reply to Jan de Mooij [:jandem] from comment #11) > Pre-existing nit, but indentation looks wrong. Tabs.... https://hg.mozilla.org/integration/mozilla-inbound/rev/8275b86c0b62