1100493 - Assertion failure: maybecx->runtime()->hadOutOfMemory, at jscntxt.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

The upcoming testcase asserts js debug shell on m-c changeset 7f0d92595432 with --fuzzing-safe --no-baseline --ion-eager --no-threads at Assertion failure: maybecx->runtime()->hadOutOfMemory, at jscntxt.cpp

Debug configure options:

LD=ld CROSS_COMPILE=1 CC="clang -Qunused-arguments -msse2 -mfpmath=sse -arch i386" RANLIB=ranlib CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments -msse2 -mfpmath=sse" AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 HOST_CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse" sh /Users/fuzz3/trees/mozilla-central/js/src/configure --target=i386-apple-darwin9.2.0 --enable-macos-target=10.5 --enable-debug --enable-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

This was found by combining random jit-tests together with jsfunfuzz, the specific files are:

http://hg.mozilla.org/mozilla-central/file/7f0d92595432/js/src/jit-test/tests/closures/incr-exit.js
http://hg.mozilla.org/mozilla-central/file/7f0d92595432/js/src/jit-test/tests/parser/bug-888002-1.js
http://hg.mozilla.org/mozilla-central/file/7f0d92595432/js/src/jit-test/tests/basic/bug858097.js

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/ba4beabcb40b
user:        Nicholas Nethercote
date:        Mon Oct 27 15:37:48 2014 -0700
summary:     Bug 588522 - Remove JSOP_ENDINIT. r=jorendorff.

:njn, is bug 588522 a likely regressor?

Comment 1

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

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x8088456, 0x004f7cdf js-dbg-opt-32-dm-nsprBuild-darwin-7f0d92595432`js::ThreadSafeContext::recoverFromOutOfMemory() [inlined] STRING_TO_JSVAL_IMPL(str=<unavailable>) + 15 at Value.h:470, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=2, address=0x0)
  * frame #0: 0x004f7cdf js-dbg-opt-32-dm-nsprBuild-darwin-7f0d92595432`js::ThreadSafeContext::recoverFromOutOfMemory() [inlined] STRING_TO_JSVAL_IMPL(str=<unavailable>) + 15 at Value.h:470
    frame #1: 0x004f7cd0 js-dbg-opt-32-dm-nsprBuild-darwin-7f0d92595432`js::ThreadSafeContext::recoverFromOutOfMemory() [inlined] JS::Value::setString(str=<unavailable>) at Value.h:1036
    frame #2: 0x004f7cd0 js-dbg-opt-32-dm-nsprBuild-darwin-7f0d92595432`js::ThreadSafeContext::recoverFromOutOfMemory() [inlined] JS::StringValue(str=<unavailable>) at Value.h:1419
    frame #3: 0x004f7cd0 js-dbg-opt-32-dm-nsprBuild-darwin-7f0d92595432`js::ThreadSafeContext::recoverFromOutOfMemory() [inlined] JSContext::isThrowingOutOfMemory(this=<unavailable>) at jscntxt.cpp:1055
    frame #4: 0x004f7cd0 js-dbg-opt-32-dm-nsprBuild-darwin-7f0d92595432`js::ThreadSafeContext::recoverFromOutOfMemory(this=<unavailable>) + 256 at jscntxt.cpp:1000
(lldb)

Comment 3

[Nicholas Nethercote [inactive]]

> :njn, is bug 588522 a likely regressor?

Not at first glance, no. I'll take a closer look.

Comment 4

[Nicholas Nethercote [inactive]]

I can reproduce, and I also see that my patch is the apparent culprit, alas.

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: Add a JSOP_NOP to avoid an assertion

I generated this patch like a program would, rather than like a human would --
I just undid bits of bug 588522's patch until I got the minimal change that
avoided the assertion. Let me know if this is too gross for words.

Comment 6

[Christian Holler (:decoder)]

Fwiw, here's a shorter test that I found in FuzzManager (m-c 134d1cfc5c9c, x86-64 debug build, options --fuzzing-safe):


gczeal(4);
gcparam("maxBytes", gcparam("gcBytes") + 4*1024);
arr = [(1.1), 5e1, 9e19, 0.1e20, 1.3e20, 1e20, 9e20, 9.99e20, 
    0.1e21, 1e21, 1e21+65537, 1e21+65536, 1e21-65536, 1]; 
for (var i = 0; i < 4000; i-- ) {
    arr.push(1e19 + i*1e19);
}

Comment 7

[Nicholas Nethercote [inactive]]

> Fwiw, here's a shorter test that I found in FuzzManager

I can't reproduce with that one. Can you try the attached patch and see if it helps? Thank you.

Comment 8

[Jason Orendorff [:jorendorff]]

Comment on attachment 8524917 [details] [diff] [review]
Add a JSOP_NOP to avoid an assertion

(In reply to Nicholas Nethercote [:njn] from comment #5)
> Let me know if this is too gross for words.

:) Yeah, medium-strength r- on this one.

Does it happen with --no-ion? If so, please dig a bit more (or I can take it if you're too busy); if not, please ni?jandem and ask him to investigate.

Comment 9

[Nicholas Nethercote [inactive]]

> Does it happen with --no-ion? If so, please dig a bit more (or I can take it
> if you're too busy)

It does happen with |--no-ion --no-baseline|, so it's not related to the JITs.

I think this is actually related to GC. The first line of Gary's test case (which he emailed to me) looks like this:

> libdir = "/Users/fuzz3/trees/mozilla-central/js/src/jit-test/lib/"

If I remove that line the assertion goes away. The assertion also goes away when I change the string to some other values, but it's unpredictable which values have that effect. I have no idea how that could interact with the JSOP_ENDINIT removal. So maybe the NOP thing is a red herring and there's something more subtle happening. It's conceivable that my JSOP_ENDINIT removal just got unlucky and exposed some latent defect here.

Digging some more... the OOM occurs because a call to hashify() fails, because ensureOwnBaseShape() fails, because makeOwnBaseShape() fails, because refillFreeListFromAnyThread() fails, because refillFreeListFromMainThread() fails, because allocateFromArena() fails, because allocateArena() fails.

This really feels like there's a GC problem, and my JSOP_ENDINIT removal was just unlucky in that it uncovered a latent defect.

Comment 10

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

Setting needinfo? from GC folks as per comment 9.

Comment 11

[Terrence Cole [:terrence]]

So poking about a bit it looks like js_ReportOutOfMemory only generally gets called for malloc failures, not for GC alloc failures, at least in the places I looked. So I guess we're accidentally asserting that the OOM here was a malloc OOM and not a GC OOM. As to what this code /should/ do under GC OOM, however, is basically anyone's guess. I'd think it should probably call js_ReportOutOfMemory, but who knows? I've certainly no idea. I'd be happy to fix it if we can decide what the right behavior should be; I expect there's a ton of stuff broken in exactly this manner.

Comment 12

[Jason Orendorff [:jorendorff]]

(In reply to Terrence Cole [:terrence] from comment #11)
> So poking about a bit it looks like js_ReportOutOfMemory only generally gets
> called for malloc failures, not for GC alloc failures, at least in the
> places I looked.

Where did you look? GC alloc failures should call js_ReportOutOfMemory. What else could possibly be correct?

Comment 13

[Terrence Cole [:terrence]]

Attachment: bug-1100493-v0.diff

(In reply to Jason Orendorff [:jorendorff] from comment #12)
> (In reply to Terrence Cole [:terrence] from comment #11)
> > So poking about a bit it looks like js_ReportOutOfMemory only generally gets
> > called for malloc failures, not for GC alloc failures, at least in the
> > places I looked.
> 
> Where did you look? GC alloc failures should call js_ReportOutOfMemory. What
> else could possibly be correct?

Speckled chickens?

Looking higher up the allocation stack, refillFreeListFromMainThread does indeed call js_ReportOutOfMemory, but only in one of the two failure paths. Adding the second report seems to solve the crash. I also added one to refillFreeListOffMainThread. And I made the /*NOT REACHED*/ comment at the end of Shape::search into a MOZ_CRASH since I was there.

Comment 14

[Nicholas Nethercote [inactive]]

\o/  Thank you, terrence.

Comment 15

[Terrence Cole [:terrence]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/eae28492fdc6

Comment 16

[Ryan VanderMeulen [:RyanVM]]

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