1462341 - Assertion failure: cx_->isExceptionPending(), at js/src/frontend/BytecodeCompiler.cpp:589 with ES6 modules

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 8fb36531f7d0 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off):

loadFile(`
  switch (0) {
    case (-1):
  }
`);
function loadFile(lfVarx) {
  oomTest(function() {
      let m = parseModule(lfVarx);
  });
}


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x0000000000ecb535 in AutoAssertReportedException::~AutoAssertReportedException (this=0x7fffffffaf40, __in_chrg=<optimized out>) at js/src/frontend/BytecodeCompiler.cpp:589
#0  0x0000000000ecb535 in AutoAssertReportedException::~AutoAssertReportedException (this=0x7fffffffaf40, __in_chrg=<optimized out>) at js/src/frontend/BytecodeCompiler.cpp:589
#1  0x0000000000ec88ab in js::frontend::CompileModule (cx=cx@entry=0x7ffff5f17000, optionsInput=..., srcBuf=..., alloc=..., sourceObjectOut=sourceObjectOut@entry=0x0) at js/src/frontend/BytecodeCompiler.cpp:700
#2  0x0000000000ec8a26 in js::frontend::CompileModule (cx=0x7ffff5f17000, options=..., srcBuf=...) at js/src/frontend/BytecodeCompiler.cpp:728
#3  0x00000000004622d9 in ParseModule (cx=<optimized out>, argc=<optimized out>, vp=<optimized out>) at js/src/shell/js.cpp:4261
#4  0x00000000005b4b1e in js::CallJSNative (cx=0x7ffff5f17000, native=0x461ff0 <ParseModule(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/JSContext-inl.h:280
#5  0x00000000005a9a3f in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7ffff5f17000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:467
#6  0x00000000005a9e1d in InternalCall (cx=0x7ffff5f17000, args=...) at js/src/vm/Interpreter.cpp:516
#7  0x000000000059d517 in js::CallFromStack (args=..., cx=<optimized out>) at js/src/vm/Interpreter.cpp:522
#8  Interpret (cx=0x7ffff5f17000, state=...) at js/src/vm/Interpreter.cpp:3086
#9  0x00000000005a94fd in js::RunScript (cx=0x7ffff5f17000, state=...) at js/src/vm/Interpreter.cpp:417
#10 0x00000000005a9b07 in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7ffff5f17000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:489
#11 0x00000000005a9e1d in InternalCall (cx=0x7ffff5f17000, args=...) at js/src/vm/Interpreter.cpp:516
#12 0x00000000005a9fa0 in js::Call (cx=<optimized out>, fval=..., fval@entry=..., thisv=..., thisv@entry=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:535
#13 0x0000000000a4fd21 in JS_CallFunction (cx=<optimized out>, obj=..., fun=..., fun@entry=..., args=..., rval=..., rval@entry=...) at js/src/jsapi.cpp:2948
#14 0x00000000008cd5a3 in OOMTest (cx=<optimized out>, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/TestingFunctions.cpp:1787
#15 0x00000000005b4b1e in js::CallJSNative (cx=0x7ffff5f17000, native=0x8cd1b0 <OOMTest(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/JSContext-inl.h:280
[...]
#29 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:9326
rax	0x0	0
rbx	0x7fffffffaf40	140737488334656
rcx	0x7ffff6c282ad	140737333330605
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffaf20	140737488334624
rsp	0x7fffffffaf00	140737488334592
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4780	140737354024832
r10	0x58	88
r11	0x7ffff6b9e7a0	140737332766624
r12	0x7ffff5f17000	140737319628800
r13	0x7ffff5f17000	140737319628800
r14	0x7fffffffaf50	140737488334672
r15	0x7fffffffb020	140737488334880
rip	0xecb535 <AutoAssertReportedException::~AutoAssertReportedException()+229>
=> 0xecb535 <AutoAssertReportedException::~AutoAssertReportedException()+229>:	movl   $0x0,0x0
   0xecb540 <AutoAssertReportedException::~AutoAssertReportedException()+240>:	ud2

Comment 1

[Christian Holler (:decoder)]

I'm actually seeing this in a lot of variations (same assert but various stacks), some of them not using parseModule at all. But they all have in common that they seem to use a similar switch/case statement.

Comment 2

[André Bargull [:anba]]

The test case asserts because this vector [1] uses |SystemAllocPolicy|, but doesn't report OOM when |resize| fails here [2].
There's probably a similar issue for this vector [3] when it calls |growBy| here [4].

[1] https://searchfox.org/mozilla-central/rev/d4b9e50875ad7e5d20f2fee6a53418315f6dfcc0/js/src/frontend/BytecodeEmitter.cpp#4647
[2] https://searchfox.org/mozilla-central/rev/d4b9e50875ad7e5d20f2fee6a53418315f6dfcc0/js/src/frontend/BytecodeEmitter.cpp#4694
[3] https://searchfox.org/mozilla-central/rev/d4b9e50875ad7e5d20f2fee6a53418315f6dfcc0/js/src/frontend/BytecodeEmitter.cpp#4737
[4] https://searchfox.org/mozilla-central/rev/d4b9e50875ad7e5d20f2fee6a53418315f6dfcc0/js/src/frontend/BytecodeEmitter.cpp#4824

Comment 3

[Jan de Mooij [:jandem]]

anba, thanks for investigating this.

arai, does your BytecodeEmitter switch-statement refactoring touch this code? In that case maybe we can fix this at the same time to avoid merge conflicts :)

Comment 4

[Tooru Fujisawa [:arai]]

Yes, it touches, but I think it will take a bit more to land the patch,
because the API design tightly coupled with the BinAST file format (bug 1456006 comment #1).
I can manage the conflict later, so I'll fix it separately.

Comment 5

[Jan de Mooij [:jandem]]

Thanks!

Comment 6

[Fuzzing Team]

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/627599ba1f3b
user:        Jon Coppeard
date:        Fri Apr 13 11:03:39 2018 +0100
summary:     Bug 1449220 - Use hash tables for looking up imports and exports when building module metadata r=jorendorff

This iteration took 279.399 seconds to run.

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: Properly report OOM inside BytecodeEmitter::emitSwitch.

Added ReportOutOfMemory calls to growBy failure paths.

Comment 8

[André Bargull [:anba]]

Comment on attachment 8979141 [details] [diff] [review]
Properly report OOM inside BytecodeEmitter::emitSwitch.

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

Thanks!

::: js/src/jit-test/tests/auto-regress/bug1462341.js
@@ +5,5 @@
> +    case (-1):
> +  }
> +`);
> +function loadFile(lfVarx) {
> +  oomTest(function() {

oomTest() needs a feature-test [1]:
---
if (!('oomTest' in this))
    quit();
---

[1] https://searchfox.org/mozilla-central/search?q=oomTest&case=true&regexp=false&path=js%2Fsrc%2Fjit-test%2F

Comment 9

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/69dbf5691b01d1ddd889ab1f511027ef9d1d1d65
Bug 1462341 - Properly report OOM inside BytecodeEmitter::emitSwitch. r=anba

Comment 10

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/69dbf5691b01

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider for Beta backport or can this ship with 62?

Comment 12

[Tooru Fujisawa [:arai]]

I think we can ship this with 62.