The following testcase asserts on mozilla-central revision 1301a72b1c39 (run with --ion-eager): var TIME_0000 = (function () {})(); gcparam("maxBytes", gcparam("gcBytes") + 2*1024); function f() { var inner4 = f("get"), x1,x2,x3,x4,x5,y = new summary(X),x12,x13,x14,x15,x16,x17,x18, otherGlobalSameCompartment = newGlobal("same-compartment"); function newGlobal() x17('{"Missing colon" null}', true); } assertEq("aaa".replace(/a/g, f()), "poniesponiesponies");

JSBugMon: Bisection requested, result: autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: 106052:8a2010ae3d08 user: Sean Stangl date: Tue Mar 27 12:20:22 2012 -0700 summary: Bug 735400 - Optimize JSOP_FUNCALL. r=dvander This iteration took 75.215 seconds to run.

This bug will appear every time we bailout from a deep IonFrame and run out-of-memory. The problem is that bailouts are producing a new StackFrame which is not popped in case of failure under ThunkToInterpreter (if not the entry frame). In the process of minimizing this test case, I also noticed that bug 756614 was reappearing with this other mean to cause OOM and the following test case (with --ion-eager): gcparam("maxBytes", gcparam("gcBytes") + 1024); function rec1() { var rec2 = rec1(); return function newGlobal() { rec2(); }; } rec1();

Flag this bug as s-s because it is similar to Bug 812341 and Bug 808063 might be related. The solution would be to call, under ThunkToInterpreter, into the Interpreter to do the unwinding for us as Bug 812341 do for the complementary case of this bug.

Nicolas - any plans to work on this soon?

This is the last version of the patch which is supposed to fix this problem by using the RETHROW mode of the interpreter to unwind bailed frames, such has we don't duplicate the logic of the Interpreter for throwing in case of OOM. Dvander had to slove the opposite bug (bug 812341), where EnterIon does not unwind its frame, so it has to return a Value to let the interpreter do it. This idea of this bug would be to use the same value to enter the interpreter which will use this special IonCode to go directly to the error path for unwinding the frames allocated by the bailout. Note: minimized tests cases are showing the assertion !used() of Label's destructor, which is only used to see that we are not creating unused labels. PS: I haven't verified lately if the patch was working or not, but last time I check I don't remember being able to reproduce the bug locally, which might explain why I did not submitted this patch before.

Merge with lastest m-c => remove changes made by dvander in a previous patch which are now in the tree. This patch works fine and fix the given test case.

Comment on attachment 688625 [details] [diff] [review] Use the interpreter to unwind bailed frames after OOM. Review of attachment 688625 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/Bailouts.cpp @@ +658,4 @@ > } > > + InterpretStatus status = Interpret(cx, br->entryfp(), resumeMode); > + JS_ASSERT_IF(resumeMode == JSINTERP_RETHROW, status == Interpret_Error); This assertion should hold because there can be no intervening try blocks, right?

(In reply to David Anderson [:dvander] from comment #8) > Comment on attachment 688625 [details] [diff] [review] > Use the interpreter to unwind bailed frames after OOM. > > Review of attachment 688625 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: js/src/ion/Bailouts.cpp > @@ +658,4 @@ > > } > > > > + InterpretStatus status = Interpret(cx, br->entryfp(), resumeMode); > > + JS_ASSERT_IF(resumeMode == JSINTERP_RETHROW, status == Interpret_Error); > > This assertion should hold because there can be no intervening try blocks, > right? Can we catch OOM with try blocks? I haven't seen any case like that before.

The flags were cleared by mistake..

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 1cc19f36ee66).

JSBugMon: This bug has been automatically verified fixed.

(In reply to Nicolas B. Pierron [:pierron] [:nbp] from comment #10) > https://hg.mozilla.org/integration/mozilla-inbound/rev/9d104b821f91 If this is low risk , please nominate for uplift on aurora/beta so that we can get this into our fifth beta, going to build Tuesday.

Comment on attachment 688625 [details] [diff] [review] Use the interpreter to unwind bailed frames after OOM. [Approval Request Comment] Bug caused by (feature/regressing bug #): IonMonkey User impact if declined: Potential crash (bad call return value) after OOM in ThunkToInterpreter. Testing completed (on m-c, etc.): yes Risk to taking this patch (and alternatives if risky): Low risk. It reuses the interpreter error handling in case of OOM during ThunkToInterpreter. String or UUID changes made by this patch: N/A

Comment on attachment 688625 [details] [diff] [review] Use the interpreter to unwind bailed frames after OOM. Approving on aurora/beta.Low risk patch for a sec-critical ion bug.

https://hg.mozilla.org/releases/mozilla-aurora/rev/e35d87b45325 https://hg.mozilla.org/releases/mozilla-beta/rev/3a4d799d6775 https://hg.mozilla.org/releases/mozilla-b2g18/rev/3a4d799d6775

(In reply to Ryan VanderMeulen from comment #18) > https://hg.mozilla.org/releases/mozilla-b2g18/rev/3a4d799d6775 This is not needed since B2G does not compile with IonMonkey and this error should not affect b2g18.

beta is being merged to b2g18 routinely, so anything that lands on it will end up on b2g18. Note the cset url.