1476383 - Assertion failure: !cx->isExceptionPending(), at js/src/vm/Interpreter.cpp:447

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

The following testcase crashes on mozilla-central revision 6a320851d377 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --wasm-gc --test-wasm-await-tier2 --no-asmjs --ion-pgo=on --ion-loop-unrolling=on --ion-gvn=off --nursery-strings=off --ion-offthread-compile=off --gc-zeal=13,349 --no-threads -e maxRunTime=12000 -f):

See attachment.

Backtrace:

#0  0x000000000118f5d9 in CallJSNative (cx=0x7f6eb0817000, native=<optimized out>, args=...) at js/src/vm/Interpreter.cpp:447
#1  0x0000000001181181 in js::InternalCallOrConstruct (cx=0x7f6eb0817000, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:532
#2  0x0000000001181b4d in js::Call (cx=0x7f6eb19aa680 <_IO_2_1_stderr_>, fval=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:602
#3  0x000000000173e8c8 in JS::Call (cx=0x7f6eb0817000, thisv=..., fval=..., args=..., rval=...) at js/src/jsapi.cpp:2887
/snip

For detailed crash information, see attachment.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: Detailed Crash Information

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: Testcase

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/76de95da94dd
user:        Jon Coppeard
date:        Fri Jul 13 10:49:51 2018 +0100
summary:     Bug 1475226 - Use JSContext for malloc allocation rather than Zone where possible r=sfink

For some reason when I tried to reproduce locally I couldn't (and hence couldn't get an even smaller testcase). The logs show it might be related to bug 1475226, so setting needinfo? from Steve as a start (as Jon is out on PTO for awhile).

Feel free to move the needinfo? on where necessary.

Comment 4

[Steve Fink [:sfink] [:s:]]

Attachment: cx->pod_callocCanGC can set an exception when succeeding

In bug 1475226, jonco moved a bunch of allocation from the Zone to cx. One of these changes moved pod_callocCanGC to JSContext instead of Zone, without changing the body. It basically does

  pod_calloc()
  if that failed, call onOutOfMemory
  that does a GC and retries the allocation

The problem is the JSContext's pod_calloc sets an OOM exception on the cx on failure, and it will still be set if the fallback allocation succeeds. So this test hits an assertion that a successful operation should not set an exception. Zone's pod_calloc does *not* set an exception, so it was fine.

It seems that the fix is simple -- there's a maybe_pod_calloc seemingly for this purpose. It just tries the allocation without setting any exceptions. (It also does the malloc counter update, which we still want.)

Comment 5

[Steve Fink [:sfink] [:s:]]

Attachment: cx->pod_callocCanGC can set an exception when succeeding

Oops -- as gkw pointed out, I should include a test case here.

Comment 6

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Credit for the smaller testcase goes to :decoder and LangFuzz... :)

Comment 7

[Paul Bone [:pbone]]

Comment on attachment 8993182 [details] [diff] [review]
cx->pod_callocCanGC can set an exception when succeeding

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

LGTM

::: js/src/tests/non262/regress/regress-1476383-calloc-exc.js
@@ +6,5 @@
> +{
> +}
> +`));
> +
> +this.reportCompare && reportCompare(true, true, '');

I have no idea how to read and review this test case.

If Decoder created and and sfink thinks it's okay then it's effectively been reviewed by sfink and I'm happy.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c04c0005e485
cx->pod_callocCanGC can set an exception when succeeding, r=pbone

Comment 9

[Cristina Coroiu [:ccoroiu]]

Backed out changeset c04c0005e485 (bug 1476383) for SpiderMonkey failures at non262/regress/regress-1476383-calloc-exc.js

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/64ef8614d9a2c088b7b34077a4cadddf277c66c6

Failure push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=c04c0005e4857d5cf9bbba9be845ae175e17963b

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=188987074&repo=mozilla-inbound&lineNumber=191275

task 2018-07-19T17:48:48.282Z] TEST-PASS | non262/regress/regress-406769.js | (args: "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so") [0.0 s]
[task 2018-07-19T17:48:48.282Z] {"action": "test_start", "jitflags": [], "pid": 44477, "shell_args": "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so", "source": "jstests", "test": "non262/regress/regress-406769.js", "thread": "main", "time": 1532022528.2335649}
[task 2018-07-19T17:48:48.282Z] {"action": "test_end", "extra": {"jitflags": [], "pid": 44477, "shell_args": "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so"}, "jitflags": [], "pid": 44477, "shell_args": "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so", "source": "jstests", "status": "PASS", "test": "non262/regress/regress-406769.js", "thread": "main", "time": 1532022528.282177}
[task 2018-07-19T17:48:48.287Z] ## non262/regress/regress-1476383-calloc-exc.js: rc = 3, run time = 0.047544
[task 2018-07-19T17:48:48.287Z] /builds/worker/workspace/build/src/js/src/tests/non262/regress/regress-1476383-calloc-exc.js:3:1 ReferenceError: oomTest is not defined
[task 2018-07-19T17:48:48.287Z] Stack:
[task 2018-07-19T17:48:48.287Z]   @/builds/worker/workspace/build/src/js/src/tests/non262/regress/regress-1476383-calloc-exc.js:3:1
[task 2018-07-19T17:48:48.287Z] TEST-UNEXPECTED-FAIL | non262/regress/regress-1476383-calloc-exc.js | (args: "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so") [0.0 s]
[task 2018-07-19T17:48:48.288Z] {"action": "test_start", "jitflags": [], "pid": 44489, "shell_args": "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so", "source": "jstests", "test": "non262/regress/regress-1476383-calloc-exc.js", "thread": "main", "time": 1532022528.240539}
[task 2018-07-19T17:48:48.288Z] {"action": "test_end", "extra": {"jitflags": [], "pid": 44489, "shell_args": "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so"}, "jitflags": [], "pid": 44489, "shell_args": "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so", "source": "jstests", "status": "FAIL", "test": "non262/regress/regress-1476383-calloc-exc.js", "thread": "main", "time": 1532022528.288083}

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f4963b77d389
cx->pod_callocCanGC can set an exception when succeeding, r=pbone

Comment 11

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/f4963b77d389

Comment 12

[Jon Coppeard (:jonco)]

(In reply to Steve Fink [:sfink] [:s:] from comment #4)
Thanks for fixing this :)

Comment 13

[Steve Fink [:sfink] [:s:]]

This bug spawned the discussion with Gary in bug 1479529.