914174 - ASan: JS Standalone builds don't have allow_user_segv_handler set

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision c7cc85e13f7a (run with --fuzzing-safe --ion-eager):


for (i=0; i<24; i++)
var fu=new Function();
timeout(1);
evaluate("for (var $2 = y = this ; i < 1000; i-- ) {}");

Comment 1

[Christian Holler (:decoder)]

I assume this test doesn't reproduce on any other build than my optimized 64-bit ASan build because it depends on interrupting script execution at the right spot (using timeout). I was able to get this trace:


==29165==ERROR: AddressSanitizer: SEGV on unknown address 0x7fb0764d2b13 (pc 0x000000beec7b sp 0x7fff13c18b30 bp 0x61900000b810 T0)
AddressSanitizer can not provide additional info.
    #0 0xbeec7a in JSC::X86Assembler::setInt32(void*, int) assembler/assembler/X86Assembler.h:3028
    #1 0xe095e2 in js::jit::InterruptCheck(JSContext*) jit/VMFunctions.cpp:436
    #2 0x7ff9a358fbb0 (+0xdbb0)


Jandem suggests this might be related to a change bhackett did recently to the ion interrupt mechanism. Marking this s-s because this crashes while trying to access an invalid address.

In general it would be really nice if there would be better functions to reliably reproduce/find these issues, rather than timeout.

Comment 2

[Christian Holler (:decoder)]

Solved this one... the problem is not in the JS engine, but rather this was regressed by me in bug 898230. It seems that for standalone js shell builds, mozglue is not involved and therefore, the required ASan option isn't set.

Comment 3

[Christian Holler (:decoder)]

I talked to glandium and although mozglue not being used for standalone JS builds is a bug, he agrees that it is easier now to fix it by adding code guarded by JS_STANDALONE back into the js engine.

Patch for that will be up soon.

Comment 4

[Christian Holler (:decoder)]

Attachment: asan-js-standalone.patch

Tested with a JS standalone build, as well as a browser build.

Comment 5

[Luke Wagner [:luke]]

Comment on attachment 802371 [details] [diff] [review]
asan-js-standalone.patch

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

::: js/src/jit/AsmJSSignalHandlers.cpp
@@ +1034,5 @@
>  #endif
>  }
> +
> +#ifdef MOZ_ASAN
> +#ifdef JS_STANDALONE

#if defined(MOZ_ASAN) && defined(JS_STANDALONE)

Comment 6

[Christian Holler (:decoder)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0f7c370491bf

Comment 7

[Christian Holler (:decoder)]

Accidentially pushed the patch without the review change, here's the follow up push with nit fixed:

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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Backed out for breaking Linux debug mochitest-bc.
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e8db3422a37

https://tbpl.mozilla.org/php/getParsedLog.php?id=27649124&tree=Mozilla-Inbound

Comment 9

[Ryan VanderMeulen [:RyanVM]]

This wasn't at fault. Re-landed.
https://hg.mozilla.org/integration/mozilla-inbound/rev/4f3fe05d6dc8

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Forgot to re-land the follow-up too.
https://hg.mozilla.org/integration/mozilla-inbound/rev/3116abfc2f95

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4f3fe05d6dc8
https://hg.mozilla.org/mozilla-central/rev/3116abfc2f95