840280 - OdinMonkey: signal handler support

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

This bug is for installing SIGSEGV handlers to catch out-of-bounds memory access from Odin-compiled asm.  The important thing is to avoid masking general seg faults (e.g., from breakpad) and generally play well with the rest of Mozilla, which also installs signal handlers.  Fortunately, OdinMonkey puts all the code from a single asm.js module into a single linear segment and maintains a dynamic activation stack of modules so a signal handler can easily check if the current pc is in the innermost module activation's range.  On out-of-bounds, the signal handler can patch in the correct value specified by JS (undefined|0 -> 0, +undefined -> NaN).

Once we have this basic support, there are a bunch of things we can optimize:
 - remove the operation callback check
 - remove the stack overflow check
 - remove the alignment mask for typed array loads by unmasking alignment faults
 - remove the idiv branches for X/0 and INT32_MIN/-1 by handling these signals
 - remove branching in ToInt by unmasking overflow exceptions in cvttsd2si

Comment 1

[Luke Wagner [:luke]]

Attachment: WIP 1

This WIP patch handles SIGSEGV on linxu64.  Still more work to do to move the ArrayBuffer's memory into a new 4GB reserved space.

Mike, do you suppose you could give a quick look at AsmJSSignalHandlers.cpp and tell me if my usage of sigaction() looks reasonable?  I don't think I need a separate stack since my stack use is so minimal.

Comment 2

[Luke Wagner [:luke]]

Attachment: WIP 2

This patch works on Linux64 and contains/passes a bunch of tests.  (This is only for out-of-bounds; all the other things (slow script, stack overflow, alignment) will come after the initial landing.)

Comment 3

[Luke Wagner [:luke]]

Comment on attachment 714189 [details] [diff] [review]
WIP 2

Oops, I meant f?.

Comment 4

[Luke Wagner [:luke]]

Attachment: WIP 3

This version should work on linux64 and osx.  The only difference from Linux is the naming of fields of the mcontext_t struct and the fact that access violations are SIGBUS (instead of SIGSEGV).

I'll close out this bug; the other platforms will come with their respective platform bugs.  I've also postponed all the exciting items in the list at the end of comment 0 until after initial landing.

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 714874 [details] [diff] [review]
WIP 3

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

Seems reasonable to me.

::: js/src/ion/AsmJSSignalHandlers.cpp
@@ +255,5 @@
> +    AsmJSActivation *activation = threadData->asmJSActivation;
> +    if (!activation)
> +        return false;
> +
> +#ifdef JS_CPU_X64

#ifdef HAVE_64BIT_OS ?

@@ +290,5 @@
> +
> +    // This signal is not for any asm.js code we expect, so we need to forward
> +    // the signal to the next handler. If there is no next handler (SIG_IGN or
> +    // SIG_DFL), then it's time to crash. To do this, we set the signal back to
> +    // it's previous disposition and return. This will cause the faulting op to

s/it's/its/

Comment 6

[Luke Wagner [:luke]]

Thanks Mike!  IonMonkey tends to use defined(JS_CPU_X64) throughout as the canonical 64-bit-ness test, so I think I should stick with that.

Comment 7

[Luke Wagner [:luke]]

Landed on the project repo:
http://hg.mozilla.org/users/lwagner_mozilla.com/odinmonkey/rev/a55682467286

Review will happen in bug 840282.