The interpreter engine for the core JavaScript language, independent of the browser's object model. File ONLY core JavaScript language bugs in this category. For bugs involving browser objects such as "window" and "document", use the "DOM" component. For bugs involving calls between JavaScript and C++, use the "XPConnect" component.

Here's what's happening:
- ion::Compile calls ion::IonCompile, which fails, returning AbortReason_Alloc.
- ion::Compile does not clear the exception. It returns Method_Skipped
which is not treated as an error by its callers.
Until bug 865960, OOM did not set an exception, so this wasn't a problem. I'm a little worried there could be a lot of extremely minor bugs like this for the fuzzer to find. :-\

(In reply to Jason Orendorff [:jorendorff] from comment #2)
> I'm a little worried there could be a lot of extremely minor bugs like this
> for the fuzzer to find. :-\
The fuzzer is already finding this assertion with multiple stacks, without oomAfterAllocations. This is and has always been a problem, because the tests are usually hard to reduce and they hide real problems, making the assertion basically worthless. I hope that with short testcases, these bugs get fixed because that's the only way forward :) Until that, the fuzzer will ignore everything matching the attached signature.

While the second signature here might be a different bug, it doesn't seem to be possible to reduce a testcase for it, as I end up with the first stack. So we'll just treat these as one until the first is fixed.

(In reply to Jason Orendorff [:jorendorff] from comment #9)
>
> By the time we get here, js_ReportOutOfMemory has already been called (at
> least with the present test case).
Where? It certainly was not for me; I traced the failure path twice trying to find out where js_ReportOutOfMemory was supposed to be called.
> I think this should just return Method_Error.
That version of the patch resulted in us exiting with code 3 and printing nothing.

It appears we are indeed hitting a different allocation failure. You are failing in JSContext::malloc which does JS_OOM_POSSIBLY_FAIL_REPORT, but I'm failing in LifoAlloc::alloc, which does JS_OOM_POSSIBLY_FAIL. In IonCompile this is cx->new_ vs alloc->new_. I think this is fine: LifoAlloc is a generic allocator and is not meant to report its own OOM failures. Of course, my hacky solution will end up calling js_ReportOutOfMemory twice if we fail to alloc in cx->new_. I think maybe something more sophisticated is needed.
(gdb) bt
#0 js::LifoAlloc::alloc (this=0x1a31ba0, n=0x38) at ../../ds/LifoAlloc.h:263
#1 0x00000000009314cc in js::LifoAlloc::new_<js::jit::CompileInfo, JSScript*, JSFunction*, unsigned char*, bool, js::jit::ExecutionMode> (this=0x1a31ba0, p1=0x7ffff59551c0, p2=(JSFunction *) 0x7ffff596b240 [object Function <unnamed>], p3=0x0, p4=0x0, p5=js::jit::SequentialExecution) at ../ds/LifoAlloc.h:403
#2 0x0000000000924308 in js::jit::IonCompile (cx=0x1a11bb0, script=0x7ffff59551c0, baselineFrame=0x0, osrPc=0x0, constructing=0x0, executionMode=js::jit::SequentialExecution) at /home/terrence/moz/branch/fuzz_exact_rooting/mozilla/js/src/jit/Ion.cpp:1630
#3 0x0000000000924e6d in js::jit::Compile (cx=0x1a11bb0, script=0x7ffff59551c0, osrFrame=0x0, osrPc=0x0, constructing=0x0, executionMode=js::jit::SequentialExecution) at /home/terrence/moz/branch/fuzz_exact_rooting/mozilla/js/src/jit/Ion.cpp:1834
#4 0x000000000092567d in js::jit::CanEnter (cx=0x1a11bb0, state=...) at /home/terrence/moz/branch/fuzz_exact_rooting/mozilla/js/src/jit/Ion.cpp:1963
#5 0x00000000004a6bad in js::RunScript (cx=0x1a11bb0, state=...) at /home/terrence/moz/branch/fuzz_exact_rooting/mozilla/js/src/vm/Interpreter.cpp:421
#6 0x00000000004a7138 in js::Invoke (cx=0x1a11bb0, args=..., construct=js::NO_CONSTRUCT) at /home/terrence/moz/branch/fuzz_exact_rooting/mozilla/js/src/vm/Interpreter.cpp:508
#7 0x00000000006ba145 in js::FastInvokeGuard::invoke (this=0x7fffffffbed0, cx=0x1a11bb0) at /home/terrence/moz/branch/fuzz_exact_rooting/mozilla/js/src/vm/Interpreter-inl.h:737
#8 0x00000000006a9f78 in (anonymous namespace)::SortComparatorFunction::operator() (this=0x7fffffffbe80, a=..., b=..., lessOrEqualp=0x7fffffffbe0f) at /home/terrence/moz/branch/fuzz_exact_rooting/mozilla/js/src/jsarray.cpp:1465
#9 0x00000000006b34de in js::MergeSort<JS::Value, {anonymous}::SortComparatorFunction>(JS::Value *, size_t, JS::Value *, (anonymous namespace)::SortComparatorFunction) (array=0x1a33f40, nelems=0x3, scratch=0x1a33f58, c=...) at /home/terrence/moz/branch/fuzz_exact_rooting/mozilla/js/src/ds/Sort.h:100
#10 0x00000000006ab15b in js::array_sort (cx=0x1a11bb0, argc=0x1, vp=0x7fffffffc858) at /home/terrence/moz/branch/fuzz_exact_rooting/mozilla/js/src/jsarray.cpp:1862
#11 0x00000000004bc1f4 in js::CallJSNative (cx=0x1a11bb0, native=0x6aa802 <js::array_sort(JSContext*, unsigned int, JS::Value*)>, args=...) at ../jscntxtinlines.h:219
#12 0x00000000004a7009 in js::Invoke (cx=0x1a11bb0, args=..., construct=js::NO_CONSTRUCT) at /home/terrence/moz/branch/fuzz_exact_rooting/mozilla/js/src/vm/Interpreter.cpp:489
#13 0x00000000004a73dd in js::Invoke (cx=0x1a11bb0, thisv=..., fval=..., argc=0x1, argv=0x7fffffffcbe8, rval=JSVAL_VOID) at /home/terrence/moz/branch/fuzz_exact_rooting/mozilla/js/src/vm/Interpreter.cpp:539
#14 0x00000000008a1f49 in js::jit::DoCallFallback (cx=0x1a11bb0, frame=0x7fffffffcc28, stub=0x1a34118, argc=0x1, vp=0x7fffffffcbd8, res=JSVAL_VOID) at /home/terrence/moz/branch/fuzz_exact_rooting/mozilla/js/src/jit/BaselineIC.cpp:7554
#15 0x00007ffff7e35b8e in ?? ()
#16 0x00007fffffffcad0 in ?? ()
(In reply to Jason Orendorff [:jorendorff] from comment #12)
> (In reply to Terrence Cole [:terrence] from comment #10)
> > That version of the patch resulted in us exiting with code 3 and printing
> > nothing.
>
> On my machine, hacking the return value from js::jit::Compile() to be
> Method_Error causes "out of memory" to be printed. Twice! It's a little
> weird. js_ReportOutOfMemory gets called two more times (from exception
> reporting machinery). Maybe on your platform some exception-handling
> machinery failed in such a way that the exception was lost. Transcript below.
>
> Background: We recently changed OOM to be treated as a catchable exception.
> I agree with the spirit of the change, making OOM a condition that a caller
> somewhere has to explicitly clear, but the specific approach was a mistake.
> Letting scripts catch OOM is not helpful. The exception handling machinery
> keeps trying to do stuff and failing with another OOM, which makes the
> observable behavior unpredictable and dependent on implementation details.
> Uncatchable errors (what we did with OOM before) resulted in less stuff
> happening as the stack unwound.
I also get "out of memory" twice because of the double-oom in the exception machinery. I guess this is a different problem. You seem to understand what we should be doing here better, so maybe you should file?

Looking at the failure though, it seems to me that the new bustage is a pre-existing different OOM bug. This happens quite often with OOM tests, that if you fix the actual bug, then a new bug pops up right after it. If that's the case, then I suggest filing that new bug (with the same test) and commit this bug without a test. The test will then be added with the newly filed bug.

Created attachment 805831[details][diff][review]bug877437.patch
Stealing this, so terrence doesn't have to bother with stuff that I can do myself from here :) I've rebased the patch over current trunk and removed the test as discussed. Carrying over r+ from last review.