This is an archive of the discontinued LLVM Phabricator instance.

[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

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
1405

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
1405

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
1412

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
1412

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.
lld/test/ELF/tls.s