Assertion failure: !cx->isExceptionPending(), at js/src/vm/Interpreter.cpp:3542 with OOM

RESOLVED FIXED in Firefox 44

Status

()

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

People

(Reporter: decoder, Assigned: lth)

Tracking

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

Trunk
mozilla44
x86
Linux
assertion, regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38 affected, firefox44 fixed)

Details

(Whiteboard: [fuzzblocker] [jsbugmon:])

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
The following testcase crashes on mozilla-central revision e0cb32a0b1aa (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --target=i686-pc-linux-gnu --enable-debug, run with --fuzzing-safe --thread-count=2 --ion-offthread-compile=off --ion-eager):

var gTestcases = new Array();
var gTc = 0;
function TestCase()
  gTestcases[gTc++] = this;
function reportCompare() {
  new TestCase();
}
gcparam("maxBytes", gcparam("gcBytes"))
ToObject_1();
function ToObject_1() {
  reportCompare();
  try {
    foo["bar"];
  } catch ( e ) {
    ToObject_1();
  } finally {
    new TestCase();
  }
}



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x08208773 in js::Throw (cx=0x9676a40, v=...) at js/src/vm/Interpreter.cpp:3542
3542	    MOZ_ASSERT(!cx->isExceptionPending());
#0  0x08208773 in js::Throw (cx=0x9676a40, v=...) at js/src/vm/Interpreter.cpp:3542
#1  0xf5ef454b in ?? ()
#2  0xf766211c in ?? ()
[...]
#114 0xf5ef0c25 in ?? ()
#115 0x083e7a2a in EnterBaseline (cx=0xf5efb77f, cx@entry=0x9676a40, data=...) at js/src/jit/BaselineJIT.cpp:123
#116 0x083e8175 in js::jit::EnterBaselineMethod (cx=cx@entry=0x9676a40, state=...) at js/src/jit/BaselineJIT.cpp:154
#117 0x08246f43 in RunScript (state=..., cx=0x9676a40) at js/src/vm/Interpreter.cpp:434
#118 js::RunScript (cx=0x9676a40, state=...) at js/src/vm/Interpreter.cpp:411
#119 0x082476d5 in js::Invoke (cx=cx@entry=0x9676a40, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:513
#120 0x0824860e in js::Invoke (cx=cx@entry=0x9676a40, thisv=..., fval=..., argc=argc@entry=0, argv=argv@entry=0xffffbac8, rval=...) at js/src/vm/Interpreter.cpp:550
#121 0x0845201c in js::jit::DoCallFallback (cx=0x9676a40, frame=0xffffbaf0, stub_=0x97256e0, argc=0, vp=0xffffbab8, res=...) at js/src/jit/BaselineIC.cpp:9563
#122 0xf5ef4a5c in ?? ()
#123 0x097256e0 in ?? ()
#124 0xf5ef0c25 in ?? ()
#125 0x083e7a2a in EnterBaseline (cx=0xf5efb5f5, cx@entry=0x9676a40, data=...) at js/src/jit/BaselineJIT.cpp:123
#126 0x083e8175 in js::jit::EnterBaselineMethod (cx=cx@entry=0x9676a40, state=...) at js/src/jit/BaselineJIT.cpp:154
#127 0x08246f43 in RunScript (state=..., cx=0x9676a40) at js/src/vm/Interpreter.cpp:434
eax	0x0	0
ebx	0x964aff4	157593588
ecx	0xf7e558ac	-135964500
edx	0x0	0
esi	0xf5efb77f	-168839297
edi	0x9725fd0	158490576
ebp	0xffff9908	4294940936
esp	0xffff98f0	4294940912
eip	0x8208773 <js::Throw(JSContext*, JS::Handle<JS::Value>)+83>
=> 0x8208773 <js::Throw(JSContext*, JS::Handle<JS::Value>)+83>:	movl   $0xdd6,0x0
   0x820877d <js::Throw(JSContext*, JS::Handle<JS::Value>)+93>:	call   0x804aa90 <abort@plt>


This should be looked at while it still reproduces (OOM issue) and asserts like this happen quite frequently.
(Reporter)

Updated

2 years ago
Whiteboard: [jsbugmon:update,bisect][fuzzblocker] → [fuzzblocker] [jsbugmon:update]
(Reporter)

Comment 1

2 years ago
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/bb18a5bf3a1c
user:        Nick Fitzgerald
date:        Tue Feb 10 12:06:00 2015 -0500
summary:     Bug 1131326 - Part 0: Implement the Debugger.Memory.prototype.allocationsLogOverflowed getter. r=shu

This iteration took 261.021 seconds to run.
(Reporter)

Updated

2 years ago
Whiteboard: [fuzzblocker] [jsbugmon:update] → [fuzzblocker] [jsbugmon:update,ignore]
(Reporter)

Comment 2

2 years ago
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 0189941a3fd5).
(Reporter)

Comment 3

2 years ago
Probably not fixed, but let's give it a try.
Whiteboard: [fuzzblocker] [jsbugmon:update,ignore] → [fuzzblocker] [jsbugmon:bisectfix]
(Reporter)

Updated

2 years ago
Whiteboard: [fuzzblocker] [jsbugmon:bisectfix] → [fuzzblocker] [jsbugmon:]
(Reporter)

Comment 4

2 years ago
JSBugMon: Fix Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/a0844d972d08
user:        Terrence Cole
date:        Thu Mar 05 08:57:32 2015 -0800
summary:     Bug 1133140 - Move runtime heap size limit checks up to GCIfNeeded; r=sfink

This iteration took 251.987 seconds to run.
(Reporter)

Comment 5

2 years ago
Terrence, I assume comment 4 didn't fix the actual bug here, did it?
Flags: needinfo?(terrence)
No, but it might have perturbed our OOM behavior enough to make the test pass. :-(
Flags: needinfo?(terrence)
(Reporter)

Updated

2 years ago
Blocks: 912928
Assignee: nobody → evilpies
Assignee: evilpies → nobody
(Assignee)

Comment 7

2 years ago
Seems likely that this is caused by somebody ignoring a 'false' result from a function that had an OOM condition.  (Pick your favorite function.)
(Reporter)

Comment 8

2 years ago
This is an automated crash issue comment:

Summary: Assertion failure: !cx->isExceptionPending(), at js/src/vm/Interpreter.cpp:4160
Build version: mozilla-central-oom revision c119c16978b4f08f5e0c1269b52b9fdd9085be5f
Build flags: --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug
Runtime options: --fuzzing-safe

Testcase:

function TestCase(n, d, e, a) {
  this.name = n;
  this.passed = getTestCaseResult(e, a);
  this.type = (typeof window == 'undefined' ? 'shell' : 'browser');
}
function getTestCaseResult(expected, actual) {
  if (actual != expected)
    return Math.abs(actual - expected) <= 1E-10;
}
ToObject_1();
function ToObject_1() {
  result = "failed: no exception thrown";
  try {
    result = foo["bar"];
  } catch ( e ) {
    ToObject_1()          ,
      exception = e.toString();
  } finally {
    new TestCase(
      result );
  }
}


Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x00000000006d20df in js::Throw (cx=0x7ffff6907400, v=...) at js/src/vm/Interpreter.cpp:4160
#0  0x00000000006d20df in js::Throw (cx=0x7ffff6907400, v=...) at js/src/vm/Interpreter.cpp:4160
#1  0x00007ffff7fee861 in ?? ()
[...]
#4  0xfffc7ffff7e77ac0 in ?? ()
#5  0x0000000001b658e0 in js::jit::DoRestFallbackInfo ()
#6  0x00007ffff7e569a0 in ?? ()
[...]
#23 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7ffff6907400	140737330050048
rcx	0x7ffff6ca588d	140737333844109
rdx	0x0	0
rsi	0x7ffff6f7a9d0	140737336814032
rdi	0x7ffff6f791c0	140737336807872
rbp	0x7fffffe01200	140737486262784
rsp	0x7fffffe011d0	140737486262736
r8	0x7ffff7fcc780	140737353926528
r9	0x6e492f6d762f6372	7946935164652708722
r10	0x7fffffe00f90	140737486262160
r11	0x7ffff6c27ee0	140737333329632
r12	0x8	8
r13	0x7fffffffc410	140737488339984
r14	0x7ffff7e77ac0	140737352530624
r15	0x7fffffffbcd0	140737488338128
rip	0x6d20df <js::Throw(JSContext*, JS::Handle<JS::Value>)+303>
=> 0x6d20df <js::Throw(JSContext*, JS::Handle<JS::Value>)+303>:	movl   $0x1040,0x0
   0x6d20ea <js::Throw(JSContext*, JS::Handle<JS::Value>)+314>:	callq  0x4983a0 <abort()>
(Assignee)

Comment 9

2 years ago
Stack overflow on Mac OS X, will need to defer this until I am on my Linux system.
(Assignee)

Comment 10

2 years ago
Reproduces on Linux - not yet under GDB, but a core dump shows the same back trace as in the report.
(Assignee)

Comment 11

2 years ago
Setting gMaxStackSize to something larger in the shell (i increased it by 8x) makes it easier to provoke this error in the debugger.
(Assignee)

Comment 12

2 years ago
1. If setting gMaxStackSize to 4x [sic] what it normally is, remember to do ulimit -s 16384 at the
   shell, or there will be weird crashes elsewhere
2. Crashes also with --no-threads
3. After the first ReportOverRecursed we bail from a baseline frame, but the test does not crash
   if run with --no-ion (or indeed with --no-baseline or with both) -- looks like a combination
   of ion and baseline is needed
4. Does not (yet) crash in no-optimize builds
(Assignee)

Comment 13

2 years ago
This is the offending stack:

#0  js::ReportOverRecursed () at jscntxt.cpp:358
#1  js::ReportOverRecursed () at jscntxt.cpp:367
#2  js::frontend::Parser<js::frontend::FullParseHandler>::assignExpr () at frontend/Parser.cpp:7153
#3  js::frontend::Parser<js::frontend::FullParseHandler>::expr () at frontend/Parser.cpp:6904
#4  js::frontend::Parser<js::frontend::FullParseHandler>::exprInParens () at frontend/Parser.cpp:9539
#5  js::frontend::Parser<js::frontend::FullParseHandler>::condition () at frontend/Parser.cpp:3238
#6  js::frontend::Parser<js::frontend::FullParseHandler>::ifStatement () at frontend/Parser.cpp:5099
#7  js::frontend::Parser<js::frontend::FullParseHandler>::statement () at frontend/Parser.cpp:6800
#8  js::frontend::Parser<js::frontend::FullParseHandler>::statements () at frontend/Parser.cpp:3193
#9  js::frontend::Parser<js::frontend::FullParseHandler>::functionBody () at frontend/Parser.cpp:1204
#10 js::frontend::Parser<js::frontend::FullParseHandler>::functionArgsAndBodyGeneric () at frontend/Parser.cpp:2885
#11 js::frontend::Parser<js::frontend::FullParseHandler>::standaloneLazyFunction () at frontend/Parser.cpp:2802
#12 js::frontend::CompileLazyFunction () at frontend/BytecodeCompiler.cpp:909
#13 JSFunction::createScriptForLazilyInterpretedFunction () at jsfun.cpp:1418
#14 JSFunction::getOrCreateScript () at shell/../jsfun.h:384
#15 js::jit::IonBuilder::canInlineTarget () at jit/IonBuilder.cpp:466
#16 js::jit::IonBuilder::makeInliningDecision () at jit/IonBuilder.cpp:5206
#17 js::jit::IonBuilder::inlineCallsite () at jit/IonBuilder.cpp:5554
#18 js::jit::IonBuilder::jsop_call () at jit/IonBuilder.cpp:6468
#19 js::jit::IonBuilder::inspectOpcode () at jit/IonBuilder.cpp:1871
#20 js::jit::IonBuilder::traverseBytecode () at jit/IonBuilder.cpp:1517
#21 js::jit::IonBuilder::build () at jit/IonBuilder.cpp:913
#22 js::jit::AnalyzeNewScriptDefiniteProperties () at jit/IonAnalysis.cpp:3314
#23 js::TypeNewScript::maybeAnalyze () at vm/TypeInference.cpp:3714
#24 js::CreateThisForFunctionWithProto () at jsobj.cpp:983
#25 js::CreateThisForFunction () at jsobj.cpp:1019
#26 js::RunState::maybeCreateThisForConstructor () at vm/Interpreter.cpp:367
#27 js::jit::CanEnter () at jit/Ion.cpp:2549
#28 js::RunScript () at vm/Interpreter.cpp:701
#29 js::Invoke () at vm/Interpreter.cpp:802
#30 InternalConstruct () at vm/Interpreter.cpp:866
#31 js::Construct () at vm/Interpreter.cpp:913
#32 js::jit::DoCallFallback () at jit/BaselineIC.cpp:9019

This fragment in AnalyzeNewScriptDefiniteProperties drops the OOM on the floor:

IonAnalysis.cpp:3314

    if (!builder.build()) {
        if (builder.abortReason() == AbortReason_Alloc)
            return false;
        return true;
    }

The reason is that for a stack overflow the abort reason is just Disable (the default value).

It's not obvious exactly where, earlier, the problem could be caught - this is the first place we have a context to inspect.  There are just three calls to build(), so they could be patched up individually to handle this.
(Assignee)

Comment 14

2 years ago
Created attachment 8673019 [details] [diff] [review]
bug1133630-stack.patch

This patch would probably be a net win if it were to land as-is: the two cases in IonAnalysis.cpp are justified by maybeAnalyze() always checking its return value propertly (so this error will be propagated properly), and the case in Ion.cpp is justified by there being no reasonable handling of stack overflow above that point in the call chain.

However, we could also add another reason code to track stack overflows.  I suspect this is overkill, but opinions are hereby solicited.
Attachment #8673019 - Flags: feedback?(jdemooij)
(Assignee)

Updated

2 years ago
Assignee: nobody → lhansen
Comment on attachment 8673019 [details] [diff] [review]
bug1133630-stack.patch

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

::: js/src/jit/Ion.cpp
@@ +2206,5 @@
> +            // Not clear how to handle this without introducing a new
> +            // reason code, a stack overflow is not OOM -- it can't be
> +            // handled with a GC, for example -- and should not be
> +            // handled as such.  Also see code in IonAnalysis.cpp.
> +            MOZ_CRASH("Stack overflow during compilation");

I think normal (non-analysis) IonBuilder shouldn't be able to throw overrecursion errors, because it doesn't have a non-null JSContext. So crashing makes sense; if we do ever hit this we can see where we're throwing this exception.
Attachment #8673019 - Flags: feedback?(jdemooij) → feedback+
(Assignee)

Comment 16

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a8c9ce02db91
(Assignee)

Comment 17

2 years ago
A bunch of failures in that run but I claim none of them.  I think this can land.
(Assignee)

Comment 18

2 years ago
Um, I think this can go for final review, is what I meant.
(Assignee)

Comment 19

2 years ago
Created attachment 8674091 [details] [diff] [review]
bug1133630-stack.patch, v2

This is the same patch except that the comment in Ion.cpp has been cleaned up, I have added a comment in IonBuilder.h, and I have checked that the one caller of buildInline() will handle stack overflow properly (no code was needed, there's a general check for exceptions).
Attachment #8674091 - Flags: review?(jdemooij)

Updated

2 years ago
Attachment #8674091 - Flags: review?(jdemooij) → review+

Comment 20

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7ec2e6fccc4
https://hg.mozilla.org/mozilla-central/rev/e7ec2e6fccc4
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.