Assertion failure: !cx->isExceptionPending(), at ../jscntxtinlines.h:349 with OOM

RESOLVED FIXED in mozilla27

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: decoder, Assigned: decoder)

Tracking

(Blocks: 2 bugs, {assertion, testcase})

Trunk
mozilla27
x86_64
Linux
assertion, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

4 years ago
The following testcase asserts on mozilla-central revision 8d85de779506 (run with --ion-eager):


x = [];
x[12] = 1;
x.unshift(0);
x.unshift(0);
x.sort(function() {
  oomAfterAllocations(5);
})
(Assignee)

Comment 1

4 years ago
Created attachment 755654 [details]
[crash-signature] Machine-readable crash signature
(Assignee)

Updated

4 years ago
Whiteboard: [jsbugmon:update]
Assignee: general → jorendorff
Blocks: 865960
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. :-\
(Assignee)

Comment 3

4 years ago
(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.
(Assignee)

Comment 4

4 years ago
Created attachment 756005 [details]
[crash-signature] Machine-readable crash signature
Attachment #755654 - Attachment is obsolete: true
(Assignee)

Comment 5

4 years ago
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.
(Assignee)

Comment 6

4 years ago
Created attachment 758758 [details]
[crash-signature] Machine-readable crash signature
Attachment #756005 - Attachment is obsolete: true
Assignee: jorendorff → terrence
Blocks: 912928
Created attachment 800423 [details] [diff] [review]
oom_877437-v0.diff

https://tbpl.mozilla.org/?tree=Try&rev=9eea8eda9ad2

It looks like we are simply not reporting OOM from Ion compilation. Whoops.
Attachment #800423 - Flags: review?(jdemooij)
Comment on attachment 800423 [details] [diff] [review]
oom_877437-v0.diff

Review of attachment 800423 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch..
Attachment #800423 - Flags: review?(jdemooij) → review+
Comment on attachment 800423 [details] [diff] [review]
oom_877437-v0.diff

Review of attachment 800423 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit/Ion.cpp
@@ +1835,5 @@
>      if (reason == AbortReason_Disable)
>          return Method_CantCompile;
> +    if (reason == AbortReason_Alloc) {
> +        js_ReportOutOfMemory(cx);
> +        return Method_Error;

By the time we get here, js_ReportOutOfMemory has already been called (at least with the present test case).

I think this should just return Method_Error.
(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.
The stack is shown below. Could this discrepancy be due to different platforms doing different numbers of allocations, so that the failure happens in a different place? (If so, that would mean that some OOM paths report OOM and some do not, which would be bad.)

My steps to reproduce:

  1. Update to rev 6f837a184a90 and build --disable-optimize --enable-debug=-g3

  2. Run the testcase from comment 0 with --ion-eager, in gdb,
     with a breakpoint on js_ReportOutOfMemory:

         gdb --args ./d-objdir/js --ion-eager crasher.js

(gdb) break js_ReportOutOfMemory(js::ThreadSafeContext*) 
Breakpoint 1 at 0x1003770bf: file jscntxt.cpp, line 383.
(gdb) r
Starting program: /Users/jorendorff/dev/mi/js/src/d-objdir/js --ion-eager crasher.js
Reading symbols for shared libraries ++++++............................................................... done
Breakpoint 1, js_ReportOutOfMemory (cxArg=0x10240bc40) at jscntxt.cpp:383
383	    if (!cxArg->isJSContext())
(gdb) bt
#0  js_ReportOutOfMemory (cxArg=0x10240bc40) at jscntxt.cpp:383
#1  0x00000001001d419c in JSRuntime::onOutOfMemory (this=0x102827200, p=0x0, nbytes=56, cx=0x10240bc40) at /Users/jorendorff/dev/mi/js/src/vm/Runtime.cpp:712
#2  0x000000010001ad2b in js::ThreadSafeContext::onOutOfMemory (this=0x10240bc40, p=0x0, nbytes=56) at jscntxt.h:252
#3  0x000000010001aef9 in js::MallocProvider<js::ThreadSafeContext>::malloc_ (this=0x10240bc40, bytes=56) at Runtime.h:591
#4  0x00000001006a2436 in js::MallocProvider<js::ThreadSafeContext>::new_<js::LifoAlloc, unsigned long> (this=0x10240bc40, p1=4096) at Runtime.h:655
#5  0x000000010069798b in js::jit::IonCompile (cx=0x10240bc40, script=0x1031551c0, baselineFrame=0x7fff5fbfd028, osrPc=0x0, constructing=false, executionMode=js::jit::SequentialExecution) at /Users/jorendorff/dev/mi/js/src/jit/Ion.cpp:1608
#6  0x0000000100693683 in js::jit::Compile (cx=0x10240bc40, script={<js::HandleBase<JSScript *>> = {<No data fields>}, ptr = 0x7fff5fbfcf68}, osrFrame=0x7fff5fbfd028, osrPc=0x0, constructing=false, executionMode=js::jit::SequentialExecution) at /Users/jorendorff/dev/mi/js/src/jit/Ion.cpp:1839
#7  0x000000010069422c in js::jit::CompileFunctionForBaseline (cx=0x10240bc40, script={<js::HandleBase<JSScript *>> = {<No data fields>}, ptr = 0x7fff5fbfcf68}, frame=0x7fff5fbfd028, isConstructing=false) at /Users/jorendorff/dev/mi/js/src/jit/Ion.cpp:1994
#8  0x00000001005effec in js::jit::EnsureCanEnterIon (cx=0x10240bc40, stub=0x103300160, frame=0x7fff5fbfd028, script={<js::HandleBase<JSScript *>> = {<No data fields>}, ptr = 0x7fff5fbfcf68}, pc=0x103300019 "?", jitcodePtr=0x7fff5fbfcf38) at /Users/jorendorff/dev/mi/js/src/jit/BaselineIC.cpp:734
#9  0x00000001005c1fac in js::jit::DoUseCountFallback (cx=0x10240bc40, stub=0x103300160, frame=0x7fff5fbfd028, infoPtr=0x7fff5fbfd000) at /Users/jorendorff/dev/mi/js/src/jit/BaselineIC.cpp:917
#10 0x000000010252ea0e in ?? ()
#11 0x0000000103300160 in ?? ()
#12 0x00000001025256cf in ?? ()
#13 0x0000000100619fdc in EnterBaseline (cx=0x10240bc40, data=@0x7fff5fbfd430) at /Users/jorendorff/dev/mi/js/src/jit/BaselineJIT.cpp:112
#14 0x0000000100619ae4 in js::jit::EnterBaselineMethod (cx=0x10240bc40, state=@0x7fff5fbfd968) at /Users/jorendorff/dev/mi/js/src/jit/BaselineJIT.cpp:143
#15 0x00000001000e4f1b in js::RunScript (cx=0x10240bc40, state=@0x7fff5fbfd968) at /Users/jorendorff/dev/mi/js/src/vm/Interpreter.cpp:435
#16 0x00000001000f1025 in js::Invoke (cx=0x10240bc40, args={<JS::detail::CallArgsBase<0>> = {<JS::CallReceiver> = {<JS::detail::CallReceiverBase<0>> = {<JS::detail::UsedRvalBase<IncludeUsedRval>> = {usedRval_ = false}, argv_ = 0x7fff5fbfde70}, <No data fields>}, argc_ = 2}, <No data fields>}, construct=js::NO_CONSTRUCT) at /Users/jorendorff/dev/mi/js/src/vm/Interpreter.cpp:508
#17 0x0000000100363ab2 in js::FastInvokeGuard::invoke (this=0x7fff5fbfde08, cx=0x10240bc40) at Interpreter-inl.h:736
#18 0x0000000100351696 in (anonymous namespace)::SortComparatorFunction::operator() (this=0x7fff5fbfdc60, a=@0x103300048, b=@0x103300050, lessOrEqualp=0x7fff5fbfdc07) at jsarray.cpp:1465
#19 0x000000010034b434 in js::MergeSort<JS::Value, (anonymous namespace)::SortComparatorFunction> (array=0x103300040, nelems=3, scratch=0x103300058, c={cx = 0x10240bc40, fval = @0x7fff5fbfe0c0, fig = @0x7fff5fbfde08}) at Sort.h:100
#20 0x0000000100349f38 in js::array_sort (cx=0x10240bc40, argc=1, vp=0x7fff5fbfe750) at jsarray.cpp:1861
#21 0x00000001000fd6d5 in js::CallJSNative (cx=0x10240bc40, native=0x100349440 <js::array_sort(JSContext*, unsigned int, JS::Value*)>, args=@0x7fff5fbfe620) at jscntxtinlines.h:219
#22 0x00000001000f0f16 in js::Invoke (cx=0x10240bc40, args={<JS::detail::CallArgsBase<0>> = {<JS::CallReceiver> = {<JS::detail::CallReceiverBase<0>> = {<JS::detail::UsedRvalBase<IncludeUsedRval>> = {usedRval_ = false}, argv_ = 0x7fff5fbfe760}, <No data fields>}, argc_ = 1}, <No data fields>}, construct=js::NO_CONSTRUCT) at /Users/jorendorff/dev/mi/js/src/vm/Interpreter.cpp:489
#23 0x00000001000f1760 in js::Invoke (cx=0x10240bc40, thisv=@0x7fff5fbfe9b8, fval=@0x7fff5fbfe9e8, argc=1, argv=0x7fff5fbfeb18, rval={<js::MutableHandleBase<JS::Value>> = {<js::MutableValueOperations<JS::MutableHandle<JS::Value> >> = {<js::UnbarrieredMutableValueOperations<JS::MutableHandle<JS::Value> >> = {<js::ValueOperations<JS::MutableHandle<JS::Value> >> = {<No data fields>}, <No data fields>}, <No data fields>}, <No data fields>}, ptr = 0x7fff5fbfeac0}) at /Users/jorendorff/dev/mi/js/src/vm/Interpreter.cpp:539
#24 0x00000001005db492 in js::jit::DoCallFallback (cx=0x10240bc40, frame=0x7fff5fbfeb58, stub=0x103200b78, argc=1, vp=0x7fff5fbfeb08, res={<js::MutableHandleBase<JS::Value>> = {<js::MutableValueOperations<JS::MutableHandle<JS::Value> >> = {<js::UnbarrieredMutableValueOperations<JS::MutableHandle<JS::Value> >> = {<js::ValueOperations<JS::MutableHandle<JS::Value> >> = {<No data fields>}, <No data fields>}, <No data fields>}, <No data fields>}, ptr = 0x7fff5fbfeac0}) at /Users/jorendorff/dev/mi/js/src/jit/BaselineIC.cpp:7554
#25 0x000000010252d124 in ?? ()
#26 0x0000000103200b78 in ?? ()
(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.


(gdb) break js_ReportOutOfMemory
Breakpoint 1 at 0x1003770bf: file jscntxt.cpp, line 383.
(gdb) r
Starting program: /Users/jorendorff/dev/mi/js/src/d-objdir/js --ion-eager crasher.js
Reading symbols for shared libraries ++++++............................................................... done

Breakpoint 1, js_ReportOutOfMemory (cxArg=0x102500000) at jscntxt.cpp:383
383	    if (!cxArg->isJSContext())
(gdb) fin
Run till exit from #0  js_ReportOutOfMemory (cxArg=0x102500000) at jscntxt.cpp:383
JSRuntime::onOutOfMemory (this=0x102827200, p=0x0, nbytes=56, cx=0x102500000) at /Users/jorendorff/dev/mi/js/src/vm/Runtime.cpp:713
713	    return NULL;
(gdb) 
Run till exit from #0  JSRuntime::onOutOfMemory (this=0x102827200, p=0x0, nbytes=56, cx=0x102500000) at /Users/jorendorff/dev/mi/js/src/vm/Runtime.cpp:713
0x000000010001ad2b in js::ThreadSafeContext::onOutOfMemory (this=0x102500000, p=0x0, nbytes=56) at jscntxt.h:252
252	        return runtime_->onOutOfMemory(p, nbytes, maybeJSContext());
Value returned is $1 = (void *) 0x0
(gdb) 
Run till exit from #0  0x000000010001ad2b in js::ThreadSafeContext::onOutOfMemory (this=0x102500000, p=0x0, nbytes=56) at jscntxt.h:252
0x000000010001aef9 in js::MallocProvider<js::ThreadSafeContext>::malloc_ (this=0x102500000, bytes=56) at Runtime.h:591
591	        return JS_LIKELY(!!p) ? p : client->onOutOfMemory(NULL, bytes);
Value returned is $2 = (void *) 0x0
(gdb) 
Run till exit from #0  0x000000010001aef9 in js::MallocProvider<js::ThreadSafeContext>::malloc_ (this=0x102500000, bytes=56) at Runtime.h:591
0x00000001006a2436 in js::MallocProvider<js::ThreadSafeContext>::new_<js::LifoAlloc, unsigned long> (this=0x102500000, p1=4096) at Runtime.h:655
655	    JS_DECLARE_NEW_METHODS(new_, malloc_, JS_ALWAYS_INLINE)
Value returned is $3 = (void *) 0x0
(gdb) 
Run till exit from #0  0x00000001006a2436 in js::MallocProvider<js::ThreadSafeContext>::new_<js::LifoAlloc, unsigned long> (this=0x102500000, p1=4096) at Runtime.h:655
0x000000010069798b in js::jit::IonCompile (cx=0x102500000, script=0x1039551c0, baselineFrame=0x7fff5fbfd028, osrPc=0x0, constructing=false, executionMode=js::jit::SequentialExecution) at /Users/jorendorff/dev/mi/js/src/jit/Ion.cpp:1608
1608	    LifoAlloc *alloc = cx->new_<LifoAlloc>(BUILDER_LIFO_ALLOC_PRIMARY_CHUNK_SIZE);
Value returned is $4 = ('js::LifoAlloc' *) 0x0
(gdb) 
Run till exit from #0  0x000000010069798b in js::jit::IonCompile (cx=0x102500000, script=0x1039551c0, baselineFrame=0x7fff5fbfd028, osrPc=0x0, constructing=false, executionMode=js::jit::SequentialExecution) at /Users/jorendorff/dev/mi/js/src/jit/Ion.cpp:1608
0x0000000100693683 in js::jit::Compile (cx=0x102500000, script={<js::HandleBase<JSScript *>> = {<No data fields>}, ptr = 0x7fff5fbfcf68}, osrFrame=0x7fff5fbfd028, osrPc=0x0, constructing=false, executionMode=js::jit::SequentialExecution) at /Users/jorendorff/dev/mi/js/src/jit/Ion.cpp:1839
1839	    AbortReason reason = IonCompile(cx, script, osrFrame, osrPc, constructing, executionMode);
Value returned is $5 = js::jit::AbortReason_Alloc
(gdb) 
Run till exit from #0  0x0000000100693683 in js::jit::Compile (cx=0x102500000, script={<js::HandleBase<JSScript *>> = {<No data fields>}, ptr = 0x7fff5fbfcf68}, osrFrame=0x7fff5fbfd028, osrPc=0x0, constructing=false, executionMode=js::jit::SequentialExecution) at /Users/jorendorff/dev/mi/js/src/jit/Ion.cpp:1839
0x000000010069422c in js::jit::CompileFunctionForBaseline (cx=0x102500000, script={<js::HandleBase<JSScript *>> = {<No data fields>}, ptr = 0x7fff5fbfcf68}, frame=0x7fff5fbfd028, isConstructing=false) at /Users/jorendorff/dev/mi/js/src/jit/Ion.cpp:1994
1994	    MethodStatus status = Compile(cx, script, frame, NULL, isConstructing, SequentialExecution);
Value returned is $6 = js::jit::Method_Skipped
(gdb) n
1995	    if (status != Method_Compiled) {
(gdb) set status = Method_Error
(gdb) c
Continuing.

Breakpoint 1, js_ReportOutOfMemory (cxArg=0x102500000) at jscntxt.cpp:383
383	    if (!cxArg->isJSContext())
(gdb) bt
#0  js_ReportOutOfMemory (cxArg=0x102500000) at jscntxt.cpp:383
#1  0x00000001001d419c in JSRuntime::onOutOfMemory (this=0x102827200, p=0x0, nbytes=14, cx=0x102500000) at /Users/jorendorff/dev/mi/js/src/vm/Runtime.cpp:712
#2  0x000000010001ad2b in js::ThreadSafeContext::onOutOfMemory (this=0x102500000, p=0x0, nbytes=14) at jscntxt.h:252
#3  0x000000010001aef9 in js::MallocProvider<js::ThreadSafeContext>::malloc_ (this=0x102500000, bytes=14) at Runtime.h:591
#4  0x000000010008df52 in js::MallocProvider<js::ThreadSafeContext>::pod_malloc<unsigned char> (this=0x102500000, numElems=14) at Runtime.h:642
#5  0x000000010008c433 in JS::LossyTwoByteCharsToNewLatin1CharsZ (cx=0x102500000, tbchars={<mozilla::Range<unsigned short>> = {mStart = {ptr = 0x102724ed0, rangeStart = 0x102724ed0, rangeEnd = 0x102724eea}, mEnd = {ptr = 0x102724eea, rangeStart = 0x102724ed0, rangeEnd = 0x102724eea}}, <No data fields>}) at /Users/jorendorff/dev/mi/js/src/vm/CharacterEncoding.cpp:19
#6  0x000000010032dae4 in JS_EncodeString (cx=0x102500000, str=0x102724ec0) at jsapi.cpp:5739
#7  0x000000010001bab2 in JSAutoByteString::encodeLatin1 (this=0x7fff5fbfef00, cx=0x102500000, str=0x102724ec0) at jsapi.h:4017
#8  0x00000001003a7657 in js_ReportUncaughtException (cx=0x102500000) at jsexn.cpp:1116
#9  0x00000001003403b5 in AutoLastFrameCheck::~AutoLastFrameCheck (this=0x7fff5fbff348) at jsapi.cpp:4571
#10 0x00000001003375f5 in AutoLastFrameCheck::~AutoLastFrameCheck (this=0x7fff5fbff348) at jsapi.cpp:4567
#11 0x000000010032a37f in JS_ExecuteScript (cx=0x102500000, objArg=0x103951060, scriptArg=0x103955100, rval=0x0) at jsapi.cpp:5085
#12 0x00000001000044f2 in RunFile (cx=0x102500000, obj={<js::HandleBase<JSObject *>> = {<No data fields>}, ptr = 0x7fff5fbff600}, filename=0x7fff5fbffbd8 "crasher.js", file=0x7fff799e00a0, compileOnly=false) at js.cpp:438
#13 0x00000001000040b1 in Process (cx=0x102500000, obj_=0x103951060, filename=0x7fff5fbffbd8 "crasher.js", forceTTY=false) at js.cpp:572
#14 0x00000001000024c6 in ProcessArgs (cx=0x102500000, obj_=0x103951060, op=0x7fff5fbff940) at js.cpp:5261
#15 0x0000000100000ff2 in Shell (cx=0x102500000, op=0x7fff5fbff940, envp=0x7fff5fbffa88) at js.cpp:5304
#16 0x00000001000030de in main (argc=3, argv=0x7fff5fbffa68, envp=0x7fff5fbffa88) at js.cpp:5548
(gdb) c
Continuing.
out of memory

Breakpoint 1, js_ReportOutOfMemory (cxArg=0x102500000) at jscntxt.cpp:383
383	    if (!cxArg->isJSContext())
(gdb) bt
#0  js_ReportOutOfMemory (cxArg=0x102500000) at jscntxt.cpp:383
#1  0x00000001001d419c in JSRuntime::onOutOfMemory (this=0x102827200, p=0x0, nbytes=16, cx=0x102500000) at /Users/jorendorff/dev/mi/js/src/vm/Runtime.cpp:712
#2  0x000000010001ad2b in js::ThreadSafeContext::onOutOfMemory (this=0x102500000, p=0x0, nbytes=16) at jscntxt.h:252
#3  0x000000010001aef9 in js::MallocProvider<js::ThreadSafeContext>::malloc_ (this=0x102500000, bytes=16) at Runtime.h:591
#4  0x000000010037c388 in js::MallocProvider<js::ThreadSafeContext>::pod_malloc<unsigned short const*> (this=0x102500000, numElems=2) at Runtime.h:642
#5  0x0000000100378779 in js_ExpandErrorArguments (cx=0x102500000, callback=0x100377480 <js_GetErrorMessage(void*, char const*, unsigned int)>, userRef=0x0, errorNumber=147, messagep=0x7fff5fbfeac8, reportp=0x7fff5fbfead0, argumentsType=js::ArgumentsAreASCII, ap=0x7fff5fbfed30) at jscntxt.cpp:682
#6  0x0000000100379173 in js_ReportErrorNumberVA (cx=0x102500000, flags=0, callback=0x100377480 <js_GetErrorMessage(void*, char const*, unsigned int)>, userRef=0x0, errorNumber=147, argumentsType=js::ArgumentsAreASCII, ap=0x7fff5fbfed30) at jscntxt.cpp:827
#7  0x000000010032e5bc in JS_ReportErrorNumberVA (cx=0x102500000, errorCallback=0x100377480 <js_GetErrorMessage(void*, char const*, unsigned int)>, userRef=0x0, errorNumber=147, ap=0x7fff5fbfed30) at jsapi.cpp:5868
#8  0x00000001002f371a in JS_ReportErrorNumber (cx=0x102500000, errorCallback=0x100377480 <js_GetErrorMessage(void*, char const*, unsigned int)>, userRef=0x0, errorNumber=147) at jsapi.cpp:5858
#9  0x00000001003a76b3 in js_ReportUncaughtException (cx=0x102500000) at jsexn.cpp:1121
#10 0x00000001003403b5 in AutoLastFrameCheck::~AutoLastFrameCheck (this=0x7fff5fbff348) at jsapi.cpp:4571
#11 0x00000001003375f5 in AutoLastFrameCheck::~AutoLastFrameCheck (this=0x7fff5fbff348) at jsapi.cpp:4567
#12 0x000000010032a37f in JS_ExecuteScript (cx=0x102500000, objArg=0x103951060, scriptArg=0x103955100, rval=0x0) at jsapi.cpp:5085
#13 0x00000001000044f2 in RunFile (cx=0x102500000, obj={<js::HandleBase<JSObject *>> = {<No data fields>}, ptr = 0x7fff5fbff600}, filename=0x7fff5fbffbd8 "crasher.js", file=0x7fff799e00a0, compileOnly=false) at js.cpp:438
#14 0x00000001000040b1 in Process (cx=0x102500000, obj_=0x103951060, filename=0x7fff5fbffbd8 "crasher.js", forceTTY=false) at js.cpp:572
#15 0x00000001000024c6 in ProcessArgs (cx=0x102500000, obj_=0x103951060, op=0x7fff5fbff940) at js.cpp:5261
#16 0x0000000100000ff2 in Shell (cx=0x102500000, op=0x7fff5fbff940, envp=0x7fff5fbffa88) at js.cpp:5304
#17 0x00000001000030de in main (argc=3, argv=0x7fff5fbffa68, envp=0x7fff5fbffa88) at js.cpp:5548
(gdb) c
Continuing.
out of memory

Program exited with code 03.
(gdb)
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?
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f67033f26f0

After discussing with Jason on IRC, I'm pushing this now: it's the intended mechanism for Ion, regardless of other concerns. At worst we'll get "out of memory" reported twice with this, which is equivalent to current behavior anyway.
And a follow-up because I forgot that the try run was orange: oomAfterAllocations is not available in opt builds so test for it explicitly.

https://hg.mozilla.org/integration/mozilla-inbound/rev/5ec98e8b1be9
Still busted. Backed out.
https://hg.mozilla.org/integration/mozilla-inbound/rev/3aa2a28398e2

https://tbpl.mozilla.org/php/getParsedLog.php?id=27741185&tree=Mozilla-Inbound
(Assignee)

Comment 17

4 years ago
Needinfo from terrence for bustage in comment 16. Thanks!
Flags: needinfo?(terrence)
(Assignee)

Comment 18

4 years ago
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.
Excellent idea!

I filed the other OOM as bug 915497; that is going to be a bit harder to solve.
Flags: needinfo?(terrence)
(Assignee)

Comment 20

4 years ago
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.
Assignee: terrence → choller
Attachment #800423 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #805831 - Flags: review+
(Assignee)

Comment 21

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ed1e8f4226b
https://hg.mozilla.org/mozilla-central/rev/4ed1e8f4226b
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.