[PATCH] D78438: [ELF] Delete "TLS attribute mismatch" diagnostics

[Peter Smith via Phabricator via llvm-commits]

psmith added a comment.

In D78438#1992307 <https://reviews.llvm.org/D78438#1992307>, @MaskRay wrote:

> > For example if we added the check in scanRelocs the symbol combination would have completed so if there was a non-TLS symbol reference (via bitcode or assembly) that was replaced by a TLS definition we would accept that. However if the definition were non-TLS we would.
>
> (1)The example I intend to add to `lld/test/ELF/tls-mismatch.s` (below) and (2) https://bugs.llvm.org/show_bug.cgi?id=45600 suggests that testing STT_TLS is not suitable.
>
>   # RUN: echo '.globl tls1' | llvm-mc -filetype=obj -triple=x86_64 - -o %t2.o
>   # RUN: ld.lld %t2.o %t.o -o /dev/null
>
>
> We can add the diagnostic back when (2) is implemented (likely via a new RelExpr member) (My experience is that this diagnostic has not detected anything.)


There is at least one that I know of that was caught by a link time error in LLVM D43860 <https://reviews.llvm.org/D43860> the linker was BFD

  /usr/bin/aarch64-linux-gnu-ld: /tmp/test2-67df6a.o(.debug_info+0x37): R_AARCH64_ABS64 used with TLS symbol x

At a glance it looks like that transformation of using an STT_SECTION symbol is not legal in ELF. I can see that an STT_SECTION symbol for a section with SHF_TLS could be considered equivalent to a local STT_TLS symbol, and LLD could be modified to handle it that way; but that isn't what the spec says and other linkers could have special case handling of TLS symbols for STT_TLS symbols and even if they don't error they may not produce correct output. Apologies if I've missed something in my haste.

  STT_TLS
  The symbol specifies a Thread-Local Storage entity. When defined, it gives the assigned offset for the symbol, not the actual address. Symbols of type STT_TLS can be referenced by only special thread-local storage relocations and thread-local storage relocations can only reference symbols with type STT_TLS. Implementation need not support thread-local storage.

What I was thinking of was something when processing relocations
if (reloc is TLS)

  check symbol is TLS and error/warn if not

else

  check symbol is not TLS and error/warn if not

By that point we need to know if the symbol is TLS (reference from bitcode should be replaced by TLS definition). That should support the use case in PR45598. Looking at PR36049 I don't think that assigning a TLS variable to an absolute value in a script or defsym makes sense and I don't think BFD or gold are particularly helpful in providing guidance. Assigning a TLS variable to another tls2=tls1 can make sense and Gold and BFD do the right thing and make tls2 STT_TLS. I think we could fix that.

> 
> 
>> Btw, do you know what is the behavior of modern bfd/gold and/or their plans?
> 
> GNU ld does not check TLS mismatching. gold has a `symbol '%s' used as both __thread and non-__thread` error but its use case is a bit different (despite the similarity of code).

https://bugs.llvm.org/show_bug.cgi?id=21077 suggests that BFD at least used to for AArch64

  /usr/bin/aarch64-linux-gnu-ld: /tmp/test2-67df6a.o(.debug_info+0x37): R_AARCH64_ABS64 used with TLS symbol x

A grep for "used with TLS" brings up aarch64, arm, m68k, xtensa, and PPC backends with this error.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78438/new/

https://reviews.llvm.org/D78438