[lld] ba06171 - [ELF] Move TLS mismatch error from Symbol::replace to postParse

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2022-02-23T20:34:48-08:00
New Revision: ba061713d3103c1f84312901d7ad29f39a700616

URL: https://github.com/llvm/llvm-project/commit/ba061713d3103c1f84312901d7ad29f39a700616
DIFF: https://github.com/llvm/llvm-project/commit/ba061713d3103c1f84312901d7ad29f39a700616.diff

LOG: [ELF] Move TLS mismatch error from Symbol::replace to postParse

* detect `def_tls.o undef_nontls.o` violation
* place error checking code (checking duplicate symbol) together
* allow `--defsym tls1=tls2 def_tls.o`

As a degraded error checking, `--defsym tls1=42` violation will not be detected.

Added: 
    

Modified: 
    lld/ELF/InputFiles.cpp
    lld/ELF/Symbols.h
    lld/test/ELF/tls-mismatch.s

Removed: 
    


################################################################################
diff  --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index e4de1a3462823..ba1d7944c58e6 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -1156,6 +1156,16 @@ template <class ELFT> void ObjFile<ELFT>::postParse() {
   for (size_t i = firstGlobal, end = eSyms.size(); i != end; ++i) {
     const Elf_Sym &eSym = eSyms[i];
     const Symbol &sym = *symbols[i];
+
+    // st_value of STT_TLS represents the assigned offset, not the actual
+    // address which is used by STT_FUNC and STT_OBJECT. STT_TLS symbols can
+    // only be referenced by special TLS relocations. It is usually an error if
+    // a STT_TLS symbol is replaced by a non-STT_TLS symbol, vice versa.
+    if (LLVM_UNLIKELY(sym.isTls()) && eSym.getType() != STT_TLS &&
+        eSym.getType() != STT_NOTYPE)
+      errorOrWarn("TLS attribute mismatch: " + toString(sym) + "\n>>> in " +
+                  toString(sym.file) + "\n>>> in " + toString(this));
+
     // !sym.file allows a symbol assignment redefines a symbol without an error.
     if (sym.file == this || !sym.file || !sym.isDefined() ||
         eSym.st_shndx == SHN_UNDEF || eSym.st_shndx == SHN_COMMON ||

diff  --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
index 77b1e62e1986d..89b52fec9a94d 100644
--- a/lld/ELF/Symbols.h
+++ b/lld/ELF/Symbols.h
@@ -523,21 +523,6 @@ size_t Symbol::getSymbolSize() const {
 // it over to "this". This function is called as a result of name
 // resolution, e.g. to replace an undefind symbol with a defined symbol.
 void Symbol::replace(const Symbol &other) {
-  using llvm::ELF::STT_TLS;
-
-  // st_value of STT_TLS represents the assigned offset, not the actual address
-  // which is used by STT_FUNC and STT_OBJECT. STT_TLS symbols can only be
-  // referenced by special TLS relocations. It is usually an error if a STT_TLS
-  // symbol is replaced by a non-STT_TLS symbol, vice versa. There are two
-  // exceptions: (a) a STT_NOTYPE lazy/undefined symbol can be replaced by a
-  // STT_TLS symbol, (b) a STT_TLS undefined symbol can be replaced by a
-  // STT_NOTYPE lazy symbol.
-  if (symbolKind != PlaceholderKind && !other.isLazy() &&
-      (type == STT_TLS) != (other.type == STT_TLS) &&
-      type != llvm::ELF::STT_NOTYPE)
-    error("TLS attribute mismatch: " + toString(*this) + "\n>>> defined in " +
-          toString(other.file) + "\n>>> defined in " + toString(file));
-
   Symbol old = *this;
   memcpy(this, &other, other.getSymbolSize());
 

diff  --git a/lld/test/ELF/tls-mismatch.s b/lld/test/ELF/tls-mismatch.s
index 5b7661268a949..479879934ac75 100644
--- a/lld/test/ELF/tls-mismatch.s
+++ b/lld/test/ELF/tls-mismatch.s
@@ -9,8 +9,7 @@
 ## The TLS definition mismatches a non-TLS reference.
 # RUN: echo '.type tls1, at object; movq tls1,%rax' | llvm-mc -filetype=obj -triple=x86_64 - -o %t2.o
 # RUN: not ld.lld %t2.o %t.o -o /dev/null 2>&1 | FileCheck %s
-## We fail to flag the swapped case.
-# RUN: ld.lld %t.o %t2.o -o /dev/null
+# RUN: not ld.lld %t.o %t2.o -o /dev/null 2>&1 | FileCheck %s
 
 ## We fail to flag the STT_NOTYPE reference. This usually happens with hand-written
 ## assembly because compiler-generated code properly sets symbol types.
@@ -18,10 +17,11 @@
 # RUN: ld.lld %t3.o %t.o -o /dev/null
 
 ## Overriding a TLS definition with a non-TLS definition does not make sense.
-# RUN: not ld.lld --defsym tls1=42 %t.o -o /dev/null 2>&1 | FileCheck %s
+## We fail to flag this case.
+# RUN: ld.lld --defsym tls1=42 %t.o -o /dev/null 2>&1 | count 0
 
 ## Part of PR36049: This should probably be allowed.
-# RUN: not ld.lld --defsym tls1=tls2 %t.o -o /dev/null 2>&1 | FileCheck %s
+# RUN: ld.lld --defsym tls1=tls2 %t.o -o /dev/null 2>&1 | count 0
 
 ## An undefined symbol in module-level inline assembly of a bitcode file
 ## is considered STT_NOTYPE. We should not error.
@@ -32,6 +32,8 @@
 # RUN: ld.lld %t.bc %t.o -o /dev/null
 
 # CHECK: error: TLS attribute mismatch: tls1
+# CHECK-NEXT: >>> in {{.*}}.tmp.o
+# CHECK-NEXT: >>> in {{.*}}
 
 .globl _start
 _start: