Page MenuHomeFreeBSD

Hack around incorrect TLS defaults for mips64
ClosedPublic

Authored by kevans on Sep 18 2019, 2:26 AM.

Details

Summary

This was shamelessly stolen from @jhb's MIPS/LLVM branch after discovering that LLVM still does this wrong. Specify initial-exec for MIPS64, as the default is currently (incorrectly) dynamic TLS without -fPIC.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kevans created this revision.Sep 18 2019, 2:26 AM
kevans added inline comments.Sep 18 2019, 2:28 AM
share/mk/bsd.cpu.mk
327 ↗(On Diff #62249)

Whoops... this is clearly unrelated, but still stolen from jhb's mips_clang branch.

arichardson accepted this revision.Sep 18 2019, 7:59 AM

Looks good to me but I'd wait for @jhb to review it too.

share/mk/bsd.cpu.mk
327 ↗(On Diff #62249)

I can remember if integrated as is now enabled by default for mips64. If not we might still need this change.

This revision is now accepted and ready to land.Sep 18 2019, 7:59 AM
kevans added inline comments.Sep 18 2019, 12:52 PM
share/mk/bsd.cpu.mk
327 ↗(On Diff #62249)

The original commit was unclear- this was addressing some build breakage, yeah? mips64 built fine without it, but it inadvertently rode in with the TLS fixes that I merged in locally after discovering that the mips64 world was completely hosed.

I can't remember why we need this change. There were some cases where we really need inital-exec for executables rather than global-dynamic.
I tend to forget everything about TLS once I no longer need to know it but maybe @jrtc27_jrtc27.com, @brooks or @jhb know.

I can't remember why we need this change. There were some cases where we really need inital-exec for executables rather than global-dynamic.
I tend to forget everything about TLS once I no longer need to know it but maybe @jrtc27_jrtc27.com, @brooks or @jhb know.

Actually, it's possible that lld wasn't (or isn't) able to optimize these relocations for static linking, which might cause statically linked binaries to fail.

I can't remember why we need this change. There were some cases where we really need inital-exec for executables rather than global-dynamic.
I tend to forget everything about TLS once I no longer need to know it but maybe @jrtc27_jrtc27.com, @brooks or @jhb know.

Actually, it's possible that lld wasn't (or isn't) able to optimize these relocations for static linking, which might cause statically linked binaries to fail.

Yes, for some reason Clang and LLVM get a bit confused about PIC; -fno-PIC gives:

clang-9: warning: ignoring '-fno-PIC' option as it cannot be used with implicit usage of -mabicalls and the N64 ABI [-Woption-ignored]

and thus the compiler thinks it's doing full-blown PIC so has to assume global dynamic TLS (or local dynamic for non-preemptible symbols). This works on glibc-based systems because their static libc has a simple __tls_get_addr implementation that just does what the initial exec implementation would have done had it been inlined (but with the additional pointless module index in the GOT that will always be 1), but our __tls_get_addr is instead a stub that returns 0. So it's a combination of a Clang/LLVM stupidity and an unhelpful libc on our part.

(and yes, MIPS doesn't do TLS linker relaxation with any linker, otherwise this wouldn't be noticeable)

share/mk/bsd.cpu.mk
327 ↗(On Diff #62249)

Yes, the integrated assembler has been the default everywhere on MIPS since LLVM r348934/r348935 (part of 8.0.0), but was also enabled by default on FreeBSD since LLVM r336004 (part of 7.0.0), so this is no longer needed unless you want to support older compilers.

(FWIW, to complete the picture, -fPIE *is* accepted since it's still a position-independent code model and thus doesn't upset -mabicalls (though really PIC and ABI calls shouldn't be conflated), and thus overrides the implicit default -fPIC, giving initial exec as the default TLS model, so that might be a nicer way to go...)

jhb added a comment.Sep 27 2019, 5:48 PM

The original log message at https://github.com/freebsd/freebsd/commit/0e0da5913af0ce4250d4484b28ac2450760c6e68 might be better as it has more detail. You would drop the last line about fixing it upstream, but the rest of the commit log explains the reasoning for this change.