Closed Bug 1460645 Opened 6 years ago Closed 6 years ago

MinGW Opt Build Crashes on Startup at 0xffffffff`9a9f0000

Categories

(Firefox Build System :: General, enhancement)

Product:
Firefox Build System
Component:
General
Version:
3 Branch
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox-esr60 fixed, firefox62 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla62
Tracking Flags:
Tracking Status
firefox-esr60 --- fixed
firefox62 --- fixed

People

(Reporter: tjr, Assigned: tjr)

References

Details

Attachments

(1 file)

Bug 1460645 Avoid the thread_local keyword for MinGW and instead use __thread
59 bytes, text/x-review-board-request
froydnj
: review+
jcristau
: approval-mozilla-esr60+
Details 
For an unknown reason the Opt build crashes at AUTO_PROFILER_LABEL in https://hg.mozilla.org/mozilla-central/file/3089af2474ef/toolkit/xre/nsAppRunner.cpp#l4695

From the top of the function, the assembly it emits is

> 0:000> uf xul!XRE_GetBootstrap+0x22404be
> xul!XRE_GetBootstrap+0x2240450:
> 00000000`06dff630 55              push    rbp
> 00000000`06dff631 4156            push    r14
> 00000000`06dff633 4155            push    r13
> 00000000`06dff635 4154            push    r12
> 00000000`06dff637 57              push    rdi
> 00000000`06dff638 56              push    rsi
> 00000000`06dff639 53              push    rbx
> 00000000`06dff63a 4881ec00010000  sub     rsp,100h
> 00000000`06dff641 488dac2480000000 lea     rbp,[rsp+80h]
> 00000000`06dff649 4889ce          mov     rsi,rcx
> 00000000`06dff64c 4189d4          mov     r12d,edx
> 00000000`06dff64f 4c89c3          mov     rbx,r8
> 00000000`06dff652 4c89cf          mov     rdi,r9
> 00000000`06dff655 e89670d9fd      call    xul!DumpJSStack+0x69b00 (00000000`04b966f0)
> 00000000`06dff65a e831f36402      call    xul!XRE_GetBootstrap+0x488f7b0 (00000000`0944e990)
> 00000000`06dff65f c6450f00        mov     byte ptr [rbp+0Fh],0
> 00000000`06dff663 488d4d0f        lea     rcx,[rbp+0Fh]
> 00000000`06dff667 48894d10        mov     qword ptr [rbp+10h],rcx
> 00000000`06dff66b e89034e1fd      call    xul!XRE_GetBootstrap+0x53920 (00000000`04c12b00)
> 00000000`06dff670 488b4510        mov     rax,qword ptr [rbp+10h]
> 00000000`06dff674 4883f8ff        cmp     rax,0FFFFFFFFFFFFFFFFh
> 00000000`06dff678 0f84b2050000    je      xul!XRE_GetBootstrap+0x2240a50 (00000000`06dffc30)  Branch
>  
> xul!XRE_GetBootstrap+0x224049e:
> 00000000`06dff67e c60001          mov     byte ptr [rax],1
> 00000000`06dff681 c645f000        mov     byte ptr [rbp-10h],0
> 00000000`06dff685 488d45f0        lea     rax,[rbp-10h]
> 00000000`06dff689 48894518        mov     qword ptr [rbp+18h],rax
> 00000000`06dff68d 488d056c09bf93  lea     rax,[ffffffff`9a9f0000]
> 00000000`06dff694 4885c0          test    rax,rax
> 00000000`06dff697 7405            je      xul!XRE_GetBootstrap+0x22404be (00000000`06dff69e)  Branch
>  
> xul!XRE_GetBootstrap+0x22404b9:
> 00000000`06dff699 e86209bf93      call    ffffffff`9a9f0000

call    ffffffff`9a9f0000 is actually hardcoded in the assembly, it's not a relocation.  On a bunch, dmajor suggested I try changing the Thread Local switch.

https://searchfox.org/mozilla-central/rev/5ff2d7683078c96e4b11b8a13674daded935aa44/mfbt/ThreadLocal.h#240

We were using the topmost case. The second and third cases both work and do not cause this crash.  The second case looks more desirable.
Thanks to bug 1317176 comment 2 for the hint that __thread works ok on GCC.
See Also: → 1317176
Comment on attachment 8974744 [details]
Bug 1460645 Avoid the thread_local keyword for MinGW and instead use __thread

https://reviewboard.mozilla.org/r/243114/#review249236
Attachment #8974744 - Flags: review?(nfroyd) → review+
Comment on attachment 8974744 [details]
Bug 1460645 Avoid the thread_local keyword for MinGW and instead use __thread

https://reviewboard.mozilla.org/r/243114/#review249602

::: mfbt/ThreadLocal.h:241
(Diff revision 3)
>      MOZ_CRASH();
>    }
>  }
>  
> -#if defined(XP_WIN) || defined(MACOSX_HAS_THREAD_LOCAL)
> +#if (defined(XP_WIN) || defined(MACOSX_HAS_THREAD_LOCAL)) && !defined(__MINGW32__)
>  #define MOZ_THREAD_LOCAL(TYPE) thread_local mozilla::detail::ThreadLocal<TYPE, mozilla::detail::ThreadLocalNativeStorage>

I'd suggest to change defined(XP_WIN) to defined(_MSC_VER) instead.
Comment on attachment 8974744 [details]
Bug 1460645 Avoid the thread_local keyword for MinGW and instead use __thread

[Approval Request Comment]

This is one of several MinGW Build patches I'd like to land in esr60 for Tor. It will prevent them from carrying their own patches for the lifetime of esr60 and will enable us to keep the MinGW build functioning and know if/when/how it was broken by new commits into esr60.

This commit only affects the MinGW build configuration, so it is low-risk.
Attachment #8974744 - Flags: approval-mozilla-esr60?
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/dae4adbf59b7
Avoid the thread_local keyword for MinGW and instead use __thread r=froydnj
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/dae4adbf59b7
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox62: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Comment on attachment 8974744 [details]
Bug 1460645 Avoid the thread_local keyword for MinGW and instead use __thread

mingw fix, approved for 60.1esr
Attachment #8974744 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: