942547 - GenerationalGC: Crash [@ js::Nursery::moveToTenured]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

Attachment: debug and opt stacks

The upcoming testcase causes a MOZ_CRASH at js::Nursery::moveToTenured in js debug shell on m-c changeset ad6589ed742c with --no-baseline --ion-parallel-compile=on --no-ti --ion-inlining=off - any further reduction seems to make the crash symptoms go away.

This MOZ_CRASH and the crash in 32-bit opt shells are both intermittent.

My configure flags for the debug build are:

LD=ld CROSS_COMPILE=1 CXX="clang++ -Qunused-arguments -arch i386" RANLIB=ranlib CC="clang -Qunused-arguments -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments" HOST_CXX="clang++ -Qunused-arguments" sh ./configure --target=i386-apple-darwin9.2.0 --enable-macos-target=10.5 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --enable-exact-rooting --enable-elf-hack --enable-stdcxx-compat --enable-warnings-as-errors --enable-signmar --disable-elf-hack --enable-js-diagnostics --with-intl-api=build --enable-ctypes --disable-shared-js --enable-jemalloc --enable-gcgenerational --with-ccache --enable-threadsafe <other NSPR options>

My configure flags for the opt build are:

LD=ld CROSS_COMPILE=1 CXX="clang++ -Qunused-arguments -arch i386" RANLIB=ranlib CC="clang -Qunused-arguments -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments" HOST_CXX="clang++ -Qunused-arguments" sh ./configure --target=i386-apple-darwin9.2.0 --enable-macos-target=10.5 --enable-optimize --disable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --enable-exact-rooting --enable-elf-hack --enable-stdcxx-compat --enable-warnings-as-errors --enable-signmar --disable-elf-hack --enable-js-diagnostics --with-intl-api=build --enable-ctypes --disable-shared-js --enable-jemalloc --enable-gcgenerational --with-ccache --enable-threadsafe <other NSPR options>

I also get a lot of:

*** set a breakpoint in malloc_error_break to debug
js_ReportOutOfMemory called
js_ReportOutOfMemory called
js-dbg-32-dm-ts-er-ggc-darwin-ad6589ed742c(49949,0xaccc2a28) malloc: *** mmap(size=134221824) failed (error code=12)
*** error: can't allocate region
*** set a breakpoint in malloc_error_break to debug
/snip

messages before the symptom.

Setting needinfo from the GGC folks.

Comment 2

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

Sample debug output:

/snip
*** set a breakpoint in malloc_error_break to debug
js_ReportOutOfMemory called
js_ReportOutOfMemory called
js-dbg-32-dm-ts-er-ggc-darwin-ad6589ed742c(53746,0xaccc2a28) malloc: *** mmap(size=626688) failed (error code=12)
*** error: can't allocate region
*** set a breakpoint in malloc_error_break to debug
js-dbg-32-dm-ts-er-ggc-darwin-ad6589ed742c(53746,0xaccc2a28) malloc: *** mmap(size=626688) failed (error code=12)
*** error: can't allocate region
*** set a breakpoint in malloc_error_break to debug
js_ReportOutOfMemory called
js_ReportOutOfMemory called
-933738499

1597330582

860577046

Hit MOZ_CRASH() at /Users/fuzz4/Desktop/js-dbg-32-dm-ts-er-ggc-darwin-mozilla-central-157189-ad6589ed742c-oFX5d_.noindex/compilePath/js/src/gc/Nursery.cpp:459
Bus error: 10

Comment 3

[Jon Coppeard (:jonco)]

Crashing because it's out of memory while trying to move an object to tenured heap.  Not sure what we can do in this case.  Disable GGC if the free memory is too low?

Comment 4

[Christian Holler (:decoder)]

I'm also hitting some MOZ_CRASH signatures due to OOM. If you cannot handle the OOM, then we should make these crashes at least such that it's clear that it's an abort due to OOM (e.g. by adding an additional crash message or such). Just MOZ_CRASH isn't sufficient because this can be called for various reasons.

Comment 5

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

(In reply to Christian Holler (:decoder) from comment #4)
> I'm also hitting some MOZ_CRASH signatures due to OOM. If you cannot handle
> the OOM, then we should make these crashes at least such that it's clear
> that it's an abort due to OOM (e.g. by adding an additional crash message or
> such). Just MOZ_CRASH isn't sufficient because this can be called for
> various reasons.

Just like bug 937550, probably.

Comment 6

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

Jon / Terrence, what else needs to be done here?

Comment 7

[Terrence Cole [:terrence]]

Attachment: print_on_ggc_oom-v0.diff

Sorry it took me so long to get to this, Gary! I agree with your assessment: we should just report when we get an unhandlable oom. In JS_MORE_DETERMINISTIC builds this will print:

fprintf(stderr, "CrashAtUnhandlableOOM called: %s", reason);

You should be able to filter on CrashAtUnhandlableOOM.

Comment 8

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

In bug 937550, after the printing of the message, there is no MOZ_CRASH. In this patch, after the printing of the message, MOZ_CRASH is present. Is this intended?

Comment 9

[Jon Coppeard (:jonco)]

Comment on attachment 8349701 [details] [diff] [review]
print_on_ggc_oom-v0.diff

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

Looks good.

I do worry about just crashing on OOM but can't think of a better way of handling this right now.

Comment 10

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

> You should be able to filter on CrashAtUnhandlableOOM.

We'd filter on messages only for differential testing, but not for crashes.

Comment 11

[Christian Holler (:decoder)]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #10)
> > You should be able to filter on CrashAtUnhandlableOOM.
> 
> We'd filter on messages only for differential testing, but not for crashes.

That's not right. If we intend to crash on OOM, then we need to be able to filter this in all build types, not just deterministic builds.

I already mentioned it on IRC yesterday: If we break with the current JS engine policy (that is, we handle every OOM gracefully instead of forcing a crash), then we should emit a message indicating the OOM, and this should take place in all builds (debug, release, etc). Using the assertion message format would for example work :)

Comment 12

[Terrence Cole [:terrence]]

Attachment: print_on_ggc_oom-v1.diff

Carrying r+. Changed to unconditionally report using MOZ_ReportAssertionFailure and changed the string to match our other non-debug assertion failure messages:

[unhandlable oom] %s

Christian, Gary, does this cover everything we need?

Comment 13

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

Using the assertion message format sounds like a great idea. :)

Comment 14

[Christian Holler (:decoder)]

Looks good to me, thanks!

Comment 15

[Terrence Cole [:terrence]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1a6e1c27d9bf

Comment 16

[Guilherme Lima]

I think this patch broke Firefox-GGC on AWFY.

Comment 17

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

(In reply to Terrence Cole [:terrence] from comment #15)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/1a6e1c27d9bf

I've tested this on the same testcase (using inbound rev 1035869d1819) and now see:

Assertion failure: [unhandlable oom] Failed to allocate object while tenuring., at gc/Verifier.cpp

We're now ignoring "Assertion failure: [unhandlable oom]" messages, so I can confirm that we've all fixed this.

Thanks!

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1a6e1c27d9bf

Comment 19

[Ioana (away)]

I'm not actually seeing a testcase in this bug. Could you please add one? Or verify that this is fixed on Firefox 29?

Comment 20

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

There is no need to test this one - I've verified it in comment 17. Thanks!

Moreover GGC isn't enabled on Firefox 29, so nothing to worry there.