ASan: initialization-order-fiasco in [@CurrentJitContext()]

RESOLVED FIXED in Firefox 46

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: tsmith, Assigned: jandem)

Tracking

Trunk
mozilla46
Points:
---

Firefox Tracking Flags

(firefox46 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
Created attachment 8707684 [details]
call_stack.txt

This just appeared on nightly as of Jan 13 2016.

A build that reproduces this issue can be found here. It happens on startup when ASAN_OPTIONS=check_initialization_order=1 is set.
Tyson just found this initialization order regression by running a recent ASan build on his local machine. Can anyone of you look into this and figure out who might be the right developer to work on this? Thanks!
Flags: needinfo?(jdemooij)
(Assignee)

Comment 2

2 years ago
This is debug-only. In IonCaches.cpp we have a class containing:

    static const ImmPtr STUB_ADDR;

The ImmPtr constructor does:

    MOZ_ASSERT(!IsCompilingAsmJS());

The "initialization order fiasco" is in CurrentJitContext, but there we will just return nullptr if the ThreadLocal is not initialized and IsCompilingAsmJS handles that case fine.

I guess the simplest fix is to use a void* instead of ImmPtr in IonCaches.cpp. And maybe CurrentJitContext could assert the ThreadLocal is initialized, to protect against this.
(Assignee)

Comment 3

2 years ago
Created attachment 8707799 [details] [diff] [review]
Patch

This patch uses a void* pointer instead of ImmPtr and changes IsCompilingAsmJS to call GetJitContext insetad of MaybeGetJitContext.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8707799 - Flags: review?(luke)

Comment 4

2 years ago
Comment on attachment 8707799 [details] [diff] [review]
Patch

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

::: js/src/jit/shared/Assembler-shared.h
@@ +138,5 @@
>  static inline bool
>  IsCompilingAsmJS()
>  {
>      // asm.js compilation pushes a JitContext with a null JSCompartment.
> +    return GetJitContext()->compartment == nullptr;

Great, this tightens the assert to catch cases where someone forgot to create a JitContext.
Attachment #8707799 - Flags: review?(luke) → review+

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c730afcadd50
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox46: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.