1020517 - Assertion failure: (munmap(code, totalBytes) == 0), at jit/AsmJSModule.cpp

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

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

Attachment: stack

+++ This bug was initially created as a clone of Bug #1008613 +++

The upcoming testcase asserts js debug shell on m-c changeset c7fdd7e755cd with --no-baseline --ion-parallel-compile=off --ion-eager at Assertion failure: (munmap(code, totalBytes) == 0), at jit/AsmJSModule.cpp

Sometimes this needs ASLR disabled, sometimes this doesn't, to make this more reliable. See http://stackoverflow.com/a/5194709 for how to do so.

Setting s-s and sec-critical because this has been a pain to reduce, is intermittent, and is a gc-related assertion. (Also cc'ing our gc gurus)

My configure flags are:

AR=ar sh /home/fuzz2lin/trees/mozilla-central/js/src/configure --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>

Might be related to bug 1008613 but the stack looks a little different. s-s until this is confirmed to not be dangerous.

Comment 2

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

Setting needinfo? from Luke and/or Terrence, not sure if this is asm.js or gc.

I'll try to get a bisect, but not sure if it will be reliable.

Comment 3

[Luke Wagner [:luke]]

IIUC, this is just munmap failing.  Given that munmap is a fallible API, the assertion is technically invalid and could be removed.  We'd leak the memory, sure, but there's not much you can do when "free" fails.  I like the assertion, though, because it asserts that the API is being called correctly (in the 99.999% case where it doesn't fail because of some OOM situation).

Comment 4

[Dan Gohman [:sunfish]]

When munmap fails with EINVAL, it's usually due to a bug, so an assertion for that is valid. According to bug 1008613, munmap can also return ENOMEM in some cases, possibly including the failure in this bug. If so, perhaps the assertion could be loosened to this:

    JS_ALWAYS_TRUE(munmap(code, totalBytes) == 0 || errno == ENOMEM);

Comment 5

[Luke Wagner [:luke]]

Right I'm assuming it's ENOMEM failure here, but you're right we can loosen, but still keep, the assert.

Comment 6

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

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/fe2e4e1be13c
user:        Sean Stangl
date:        Thu Feb 20 14:43:03 2014 -0800
summary:     Bug 933001 - Part 1/5 - Define SharedArrayBufferObject. r=sfink

Again, this may not necessarily be the cause. If this bug is just an OOM and not s-s, please feel free to open it up.

Comment 7

[Luke Wagner [:luke]]

Gary: does replacing the failing line in AsmJSModule.cpp with what Dan suggested in comment 4 fix the assert?  (If not, could you printf the errno?)

Comment 8

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

Getting this on my plate.

Comment 9

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

Compile failure:

In file included from /home/fuzz2lin/Desktop/shell-cache/js-dbg-64-dm-ts-linux-4fc2737aac94/objdir-js-dbg-4fc2737aac94-Um5iX8/js/src/Unified_cpp_js_src2.cpp:93:0:
/home/fuzz2lin/trees/mozilla-central/js/src/jit/AsmJSModule.cpp: In function ‘void DeallocateExecutableMemory(uint8_t*, size_t)’:
/home/fuzz2lin/trees/mozilla-central/js/src/jit/AsmJSModule.cpp:100:98: error: ‘errno’ was not declared in this scope
     JS_ALWAYS_TRUE(munmap(code, totalBytes) == 0 || errno == ENOMEM);
                                                                                                  ^
/home/fuzz2lin/trees/mozilla-central/js/src/jit/AsmJSModule.cpp:100:107: error: ‘ENOMEM’ was not declared in this scope
     JS_ALWAYS_TRUE(munmap(code, totalBytes) == 0 || errno == ENOMEM);
                                                                                                           ^
/home/fuzz2lin/trees/mozilla-central/js/src/jit/AsmJSModule.cpp:100:117: error: no matching function for call to ‘ValidateAssertConditionType()’
     JS_ALWAYS_TRUE(munmap(code, totalBytes) == 0 || errno == ENOMEM);
                                                                                                                     ^
/home/fuzz2lin/trees/mozilla-central/js/src/jit/AsmJSModule.cpp:100:117: note: candidate is:
In file included from ../../dist/include/mozilla/ArrayUtils.h:14:0,
                 from ../../dist/include/mozilla/Vector.h:14,
                 from ../../dist/include/js/Vector.h:10,
                 from /home/fuzz2lin/trees/mozilla-central/js/src/jscntxt.h:14,
                 from /home/fuzz2lin/trees/mozilla-central/js/src/irregexp/RegExpEngine.h:34,
                 from /home/fuzz2lin/trees/mozilla-central/js/src/irregexp/RegExpAST.h:34,
                 from /home/fuzz2lin/trees/mozilla-central/js/src/irregexp/RegExpMacroAssembler.h:34,
                 from /home/fuzz2lin/trees/mozilla-central/js/src/irregexp/RegExpInterpreter.cpp:34,
                 from /home/fuzz2lin/Desktop/shell-cache/js-dbg-64-dm-ts-linux-4fc2737aac94/objdir-js-dbg-4fc2737aac94-Um5iX8/js/src/Unified_cpp_js_src2.cpp:2:
../../dist/include/mozilla/Assertions.h:334:6: note: template<class T> void mozilla::detail::ValidateAssertConditionType()
 void ValidateAssertConditionType()
      ^
../../dist/include/mozilla/Assertions.h:334:6: note:   template argument deduction/substitution failed:
In file included from /home/fuzz2lin/Desktop/shell-cache/js-dbg-64-dm-ts-linux-4fc2737aac94/objdir-js-dbg-4fc2737aac94-Um5iX8/js/src/Unified_cpp_js_src2.cpp:93:0:
/home/fuzz2lin/trees/mozilla-central/js/src/jit/AsmJSModule.cpp:100:117: error: template argument 1 is invalid
     JS_ALWAYS_TRUE(munmap(code, totalBytes) == 0 || errno == ENOMEM);
                                                                                                                     ^

Comment 10

[Luke Wagner [:luke]]

Oh, you need to #include "errno.h".

Comment 11

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

Attachment: patch

Since Dan suggested this patch, I've put in his name.

Comment 12

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

I tested this patch with the following results:

Without the patch, the testcase asserts when run once every 5-10 times.
With the patch, the testcase didn't assert when I ran it 90 times.

Comment 13

[Christian Holler (:decoder)]

This means that munmap is failing due to OOM, removing sec flags.

Comment 14

[Luke Wagner [:luke]]

Comment on attachment 8435871 [details] [diff] [review]
patch

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

Great, thanks!

::: js/src/jit/AsmJSModule.cpp
@@ +27,5 @@
>  
>  #include "jsobjinlines.h"
>  
>  #include "frontend/ParseNode-inl.h"
> +#include "errno.h"

I told you wrong, it should be #include <errno.h>.  SM include style
  https://wiki.mozilla.org/JavaScript:SpiderMonkey:C_Coding_Style#.23include_ordering
will want you to put this #include after the mozilla/ #includes and before the js*.h.  I'd run 'make check-style' to be sure.

Comment 15

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

I spoke to Dan about this over IRC and he agreed to take this to completion.

Comment 16

[Dan Gohman [:sunfish]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b0561d529f95

Comment 17

[Ryan VanderMeulen [:RyanVM]]

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