Page MenuHomePhabricator

[ELF] Reject local-exec TLS relocations for -shared
ClosedPublic

Authored by MaskRay on Dec 15 2020, 12:20 PM.

Details

Summary

For x86-64, D33100 added a diagnostic for local-exec TLS relocations referencing a preemptible symbol.

This patch generalizes it to non-preemptible symbols (see -Bsymbolic in tls.s)
on all targets.

Local-exec TLS relocations resolve to offsets relative to a fixed point within
the static TLS block, which are only meaningful for the executable.

With this change, clang -fpic -shared -fuse-ld=bfd a.c on the following example will be flagged for AArch64/ARM/i386/x86-64/PowerPC/RISC-V

static __attribute__((tls_model("local-exec"))) __thread long TlsVar = 42;
long bump() { return ++TlsVar; }

Note, in GNU ld, at least arm, riscv and x86's ports have the similar
diagnostics, but aarch64 and ppc64 do not error.

Diff Detail

Unit TestsFailed

TimeTest
17,450 msx64 debian > LLVM.Bindings/Go::go.test
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/bin/llvm-go go=/usr/bin/go test llvm.org/llvm/bindings/go/llvm
100 msx64 windows > LLVM.CodeGen/XCore::threads.ll
Script: -- : 'RUN: at line 1'; c:\ws\w16c2-1\llvm-project\premerge-checks\build\bin\llc.exe -march=xcore < C:\ws\w16c2-1\llvm-project\premerge-checks\llvm\test\CodeGen\XCore\threads.ll | c:\ws\w16c2-1\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w16c2-1\llvm-project\premerge-checks\llvm\test\CodeGen\XCore\threads.ll

Event Timeline

MaskRay created this revision.Dec 15 2020, 12:20 PM
MaskRay requested review of this revision.Dec 15 2020, 12:20 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 15 2020, 12:20 PM
MaskRay updated this revision to Diff 311994.Dec 15 2020, 12:23 PM

Improve i386-tls-le.s

lld/test/ELF/ppc64-local-exec-tls.s
17

Note this test is not well organized. All the relocation types have been checked so I'll not add --implicit-check-not=error:

It looks OK to me.

lld/ELF/Relocations.cpp
1401

I think this should also check for R_NEG_TLS. AFAIK R_NEG_TLS/R_386_TLS_LE_32 is only used on 32-bit x86, and compilers usually prefer R_TLS/R_386_TLS_LE, but it's possible to bypass the new error with assembly:

movl %gs:TlsVar@NTPOFF, %eax ==> R_TLS
movl %gs:TlsVar@TPOFF, %eax ==> R_NEG_TLS

MaskRay updated this revision to Diff 312244.Dec 16 2020, 9:38 AM
MaskRay edited the summary of this revision. (Show Details)

Reject R_NEG_TLS

MaskRay marked an inline comment as done.Dec 16 2020, 9:39 AM
MaskRay added inline comments.
lld/ELF/Relocations.cpp
1401

Thanks. R_386_TLS_LE_32 is an obsoleted Solaris TLS relocation type.

jhenderson added inline comments.Dec 17 2020, 3:49 AM
lld/ELF/Relocations.cpp
1408

handleTlsRelocation has a number of config->shared checks. Can these be dropped given this new diagnostic?

MaskRay marked 2 inline comments as done.Dec 17 2020, 9:21 AM
MaskRay added inline comments.
lld/ELF/Relocations.cpp
1408

No. The config->shared checks in handleTlsRelocation are about whether GD/LD/IE can be relaxed to IE/LE.

This check is about LE in -shared.

Actually, R_TLS should really be renamed to R_TPOFFSET to be less confusing and I am going to send such a patch.

MaskRay marked an inline comment as done.Dec 17 2020, 9:21 AM
MaskRay updated this revision to Diff 312812.Dec 18 2020, 8:29 AM

-> R_TPREL/R_TPREL_NEG

MaskRay edited the summary of this revision. (Show Details)Dec 18 2020, 8:29 AM

X86 part looks good to me.

This revision was not accepted when it landed; it landed in state Needs Review.Dec 21 2020, 8:47 AM
This revision was automatically updated to reflect the committed changes.