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

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Tom Ritter [:tjr]]

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.

Comment 1

[Tom Ritter [:tjr]]

Attachment: Bug 1460645 Avoid the thread_local keyword for MinGW and instead use __thread

Review commit: https://reviewboard.mozilla.org/r/243114/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/243114/

Comment 2

[Tom Ritter [:tjr]]

Comment on attachment 8974744 [details]
Bug 1460645 Avoid the thread_local keyword for MinGW and instead use __thread

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/243114/diff/1-2/

Comment 3

[(Away)]

Thanks to bug 1317176 comment 2 for the hint that __thread works ok on GCC.

Comment 4

[Tom Ritter [:tjr]]

Comment on attachment 8974744 [details]
Bug 1460645 Avoid the thread_local keyword for MinGW and instead use __thread

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/243114/diff/2-3/

Comment 5

[Nathan Froyd [:froydnj]]

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

Comment 6

[Jacek Caban]

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 7

[Tom Ritter [:tjr]]

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.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dae4adbf59b7
Avoid the thread_local keyword for MinGW and instead use __thread r=froydnj

Comment 9

[Cosmin Sabou [:CosminS]]

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

Comment 10

[Julien Cristau [:jcristau]]

Comment on attachment 8974744 [details]
Bug 1460645 Avoid the thread_local keyword for MinGW and instead use __thread

mingw fix, approved for 60.1esr

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/ccf77791ff93