Last Comment Bug 740654 - "Assertion failure: enumerators == cx->enumerators,"
: "Assertion failure: enumerators == cx->enumerators,"
Status: VERIFIED FIXED
[sg:moderate] js-triage-done
: assertion, regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Linux
: -- critical (vote)
: mozilla14
Assigned To: Luke Wagner [:luke]
:
:
Mentors:
: 742094 (view as bug list)
Depends on:
Blocks: jsfunfuzz 732744
  Show dependency treegraph
 
Reported: 2012-03-29 16:33 PDT by Gary Kwong [:gkw] [:nth10sd]
Modified: 2013-02-07 05:16 PST (History)
6 users (show)
choller: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
affected
unaffected


Attachments
fix (3.11 KB, patch)
2012-03-30 13:32 PDT, Luke Wagner [:luke]
bhackett1024: review+
Details | Diff | Splinter Review

Description Gary Kwong [:gkw] [:nth10sd] 2012-03-29 16:33:38 PDT
mjitChunkLimit(31)
o = {}
o.valueOf = function() {
    for (var p in undefined) {
        a = new Function;
    }
    +o;
};
+o;

asserts 32-bit js debug shell *intermittently* on m-c changeset 92fe907ddac8 with -m and -n at Assertion failure: enumerators == cx->enumerators,

May be related to bug 736012. s-s because that bug is s-s.

(gdb) bt 10
#0  0xf776a430 in __kernel_vsyscall ()
#1  0xf773bebe in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:42
#2  0x083ade82 in MOZ_Crash () at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/mfbt/Assertions.cpp:79
#3  0x083adede in MOZ_Assert (s=0x83ed73c "enumerators == cx->enumerators", file=0x83ed5cc "/home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.cpp", ln=453) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/mfbt/Assertions.cpp:88
#4  0x08138a52 in js::CheckStackBalance::~CheckStackBalance (this=0xff864190, __in_chrg=<optimized out>) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.cpp:453
#5  0x0813896c in js::RunScript (cx=0x9008d38, script=0xf6c06128, fp=0xf6e60c38) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.cpp:455
#6  0x08138d92 in js::InvokeKernel (cx=0x9008d38, args=..., construct=js::NO_CONSTRUCT) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.cpp:528
#7  0x080930c3 in js::Invoke (cx=0x9008d38, args=..., construct=js::NO_CONSTRUCT) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.h:172
#8  0x08138f55 in js::Invoke (cx=0x9008d38, thisv=..., fval=..., argc=0, argv=0x0, rval=0xff8643c8) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.cpp:560
#9  0x08175aab in js::MaybeCallMethod (cx=0x9008d38, obj=0xf6c0c3d0, id=..., vp=0xff8643c8) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsobj.cpp:5553
(More stack frames follow...)

(2000+ stacktrace lines follow)
Comment 1 Luke Wagner [:luke] 2012-03-30 10:33:56 PDT
Ah, so the basic issue is that we rejoin the interpreter and a JSOP_ENDITER and then immediately hit the C stack recursion limit.  This jumps to error handling which assumes that, if we are parked on JSOP_ENDITER, we have already executed CloseIterator, so it doesn't unwind the iterator, leaving cx->enumerators invalid.  I'm thinking of what is the least awful fix...
Comment 2 Luke Wagner [:luke] 2012-03-30 13:32:48 PDT
Created attachment 610995 [details] [diff] [review]
fix

After talking with bhackett, we decided that the real culprit here is js::Interpret executing 'error' before jumping to *regs.pc: the assumption in interpreter and mjit stack-unwinding is that, if you throw at pc, you at least started to execute *pc.  That means it is correct for unwinding to not execute CloseIterator when *pc == JSOP_ENDITER: if we started executing JSOP_ENDITER we must have already called CloseIterator.
Comment 4 Daniel Veditz [:dveditz] 2012-04-04 10:54:00 PDT
Can content influence the mjitChunkLimit? Is that setting necessary to this bug? Trying to guess whether this is an exploitable bug we need to worry about, and whether we need to back-port to the ESR.
Comment 5 Christian Holler (:decoder) 2012-04-04 10:56:48 PDT
(In reply to Daniel Veditz [:dveditz] from comment #4)
> Can content influence the mjitChunkLimit? Is that setting necessary to this
> bug?

Content cannot influence the chunk limit, it is a fixed value there. However, I don't think it's necessary to modify the value, you just need to make the code larger to you hit the boundary in the right place with the default value.
Comment 6 Luke Wagner [:luke] 2012-04-04 10:59:44 PDT
Hitting this bug is extremely difficult.  One, you have to hit the chunk limit in just the right way, but you also have to exhaust the C stack at exactly the right place from the right op, etc.
Comment 7 Jesse Ruderman 2012-04-04 13:09:30 PDT
I wouldn't count on it being hard to exhaust the C stack at exactly the right place.
Comment 8 Luke Wagner [:luke] 2012-04-04 13:11:35 PDT
I dunno, it took the fuzzers 6 weeks to find a test-case that repros 1 in 10 only on windows...
Comment 9 Luke Wagner [:luke] 2012-04-04 13:13:01 PDT
Oh, I thought this was a question about backporting to aurora, but comment 4 is asking about ESR.  I don't think bug 732744 is on ESR.
Comment 10 Gary Kwong [:gkw] [:nth10sd] 2012-04-04 13:15:30 PDT
(In reply to Luke Wagner [:luke] from comment #8)
> I dunno, it took the fuzzers 6 weeks to find a test-case that repros 1 in 10
> only on windows...

To clarify, this repros only in 32-bit Linux and Windows. I don't think I've managed to reproduce in 64-bit Linux or Mac (both architectures).

Time taken to find a testcase is not a good gauge of how easy / hard it is to hit the bug, it's just a gauge of how easy / hard it is *for the fuzzer* to hit the bug.
Comment 11 Luke Wagner [:luke] 2012-04-04 13:19:27 PDT
(In reply to Gary Kwong [:gkw, :nth10sd] from comment #10)
> to hit the bug, it's just a gauge of how easy / hard it is *for the fuzzer*
> to hit the bug.

Sure, in general, but this is a fuzzer-only type of bug.  Given that the bug isn't on a long-term release, it seems fine to let the fix ride the train out.
Comment 12 Daniel Veditz [:dveditz] 2012-04-05 13:37:46 PDT
I think comment 9 is implying this is a regression from bug 732744 which is why we don't need it on ESR.

Taking a SWAG at sg:moderate given comment 6 explaining you'd need to reliably hit two conditions at the same time to exploit.
Comment 13 Gary Kwong [:gkw] [:nth10sd] 2012-04-05 13:38:59 PDT
This landed on mozilla-central:

https://hg.mozilla.org/mozilla-central/rev/3a185f034768
Comment 14 Christian Holler (:decoder) 2012-04-05 16:12:29 PDT
JSBugMon: This bug has been automatically verified fixed.
Comment 15 Christian Holler (:decoder) 2012-04-05 16:14:15 PDT
JSBugMon: This bug has been automatically verified fixed.
Comment 16 Gary Kwong [:gkw] [:nth10sd] 2012-04-06 16:23:47 PDT
*** Bug 742094 has been marked as a duplicate of this bug. ***
Comment 17 Christian Holler (:decoder) 2013-02-07 05:16:55 PST
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/2e891e0db397

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