The following testcase asserts on mozilla-central revision 0acbd06d48a9 (no options required): function shapelessUnknownCalleeLoop(n, f, g, h, a) { for (var i = 0; i < 32000, g = new Function("ABCDEFGHIJK", "1234"); i++) g = h; } function shapelessCalleeTest() { var a = []; helper = function (i, a) a[30 + i] = i; shapelessUnknownCalleeLoop(null, helper, helper, function (i, a) a[30 + i] = -i, a); } shapelessCalleeTest()

Debug trace: Program received signal SIGSEGV, Segmentation fault. 0x085e578d in MarkValueInternal (trc=0x8cc5f1c, v=0xffff8af4) at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:470 470 JS_ASSERT(v->toGCThing()); (gdb) bt #0 0x085e578d in MarkValueInternal (trc=0x8cc5f1c, v=0xffff8af4) at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:470 #1 0x085e63a8 in js::gc::MarkValueRoot (trc=0x8cc5f1c, v=0xffff8af4, name=0x8a9e2ea "ion-argv") at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:514 #2 0x087e960d in MarkIonJSFrame (trc=0x8cc5f1c, frame=...) at /srv/repos/mozilla-central/js/src/ion/IonFrames.cpp:479 #3 0x087ea051 in MarkIonActivation (trc=0x8cc5f1c, activations=...) at /srv/repos/mozilla-central/js/src/ion/IonFrames.cpp:657 #4 0x087ea17a in js::ion::MarkIonActivations (rt=0x8cc5d38, trc=0x8cc5f1c) at /srv/repos/mozilla-central/js/src/ion/IonFrames.cpp:681 #5 0x085df3c3 in js::gc::MarkRuntime (trc=0x8cc5f1c, useSavedRoots=false) at /srv/repos/mozilla-central/js/src/gc/RootMarking.cpp:791 #6 0x081cac42 in BeginMarkPhase (rt=0x8cc5d38) at /srv/repos/mozilla-central/js/src/jsgc.cpp:2782 #7 0x081d2071 in IncrementalCollectSlice (rt=0x8cc5d38, budget=0, reason=JS::gcreason::TOO_MUCH_MALLOC, gckind=js::GC_NORMAL) at /srv/repos/mozilla-central/js/src/jsgc.cpp:4184 #8 0x081d287f in GCCycle (rt=0x8cc5d38, incremental=true, budget=0, gckind=js::GC_NORMAL, reason=JS::gcreason::TOO_MUCH_MALLOC) at /srv/repos/mozilla-central/js/src/jsgc.cpp:4369 #9 0x081d2d68 in Collect (rt=0x8cc5d38, incremental=true, budget=0, gckind=js::GC_NORMAL, reason=JS::gcreason::TOO_MUCH_MALLOC) at /srv/repos/mozilla-central/js/src/jsgc.cpp:4494 #10 0x081d3059 in js::GCSlice (rt=0x8cc5d38, gckind=js::GC_NORMAL, reason=JS::gcreason::TOO_MUCH_MALLOC, millis=0) at /srv/repos/mozilla-central/js/src/jsgc.cpp:4532 #11 0x0812ee34 in js_InvokeOperationCallback (cx=0x8cec7e0) at /srv/repos/mozilla-central/js/src/jscntxt.cpp:1135 #12 0x0812ee9d in js_HandleExecutionInterrupt (cx=0x8cec7e0) at /srv/repos/mozilla-central/js/src/jscntxt.cpp:1159 #13 0x088b5865 in js::ion::InterruptCheck (cx=0x8cec7e0) at /srv/repos/mozilla-central/js/src/ion/VMFunctions.cpp:424 #14 0xf770c920 in ?? () Cannot access memory at address 0xffffff8b (gdb) S-s due to GC-relatedness.

JSBugMon: Bisection requested, result: autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: 115546:8275b86c0b62 user: Brian Hackett date: Mon Dec 10 12:02:31 2012 -0700 summary: Remove bytecode uses analysis, keep track of SSA values that were folded away when building MIR, bug 818869. r=jandem This iteration took 94.700 seconds to run.

Needinfo from Brian based on comment 2.

Really just a continuation of bug 830042 (and should have realized this and fixed it at that point). We tolerate JM leaving object/string values with null payloads on the stack, but if these flow back to Ion code then marking will assume they are valid GC things. Just tolerating the null payloads during Ion marking is difficult as values can be split apart on the stack, but there is only a single place where values flow from the interpreter into Ion so it is easier to fix there. This can all be ripped out when JM is removed.

Comment on attachment 716086 [details] [diff] [review] patch Review of attachment 716086 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/vm/Stack.cpp @@ +252,5 @@ > +void > +StackFrame::cleanupTornValues() > +{ > + for (size_t i = 0; i < numFormalArgs(); i++) > + CleanupTornValue(this, &formals()[i]); If numActuals >= numFormals, the extra arguments don't need cleanup? And stack values >= nfixed?

Only values in slots which the method JIT computes SSA information for need to be tracked. Arguments above nformals will only be written by mjit code via the arguments object, and the only things that can be on the stack at OSR points are iterator objects, which will not be dead (other values in scripts not Ion compiled could also be there, but are also not tracked by the mjit's SSA).

Comment on attachment 716086 [details] [diff] [review] patch [Security approval request comment] How easily could an exploit be constructed based on the patch? not at all. this isn't exploitable except as a null deref Which older supported branches are affected by this flaw? only aurora/beta are affected How likely is this patch to cause regressions; how much testing does it need? tbpl is fine

Comment on attachment 716086 [details] [diff] [review] patch [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 818869 User impact if declined: potential null deref Testing completed (on m-c, etc.): on m-c Risk to taking this patch (and alternatives if risky): none

Followup fix to move the cleanupTornValues() call to the correct place. https://hg.mozilla.org/integration/mozilla-inbound/rev/f815f2ba316a

https://hg.mozilla.org/mozilla-central/rev/f815f2ba316a https://hg.mozilla.org/mozilla-central/rev/dc30947a2d26

JSBugMon: This bug has been automatically verified fixed.

Comment on attachment 716086 [details] [diff] [review] patch low risk patch for a sec-high regression,approving in aurora & beta

https://hg.mozilla.org/releases/mozilla-aurora/rev/0ca928196a14 https://hg.mozilla.org/releases/mozilla-beta/rev/d548d3afbaf5 FWIW, I qfolded the follow-up fix in with the original.