[lld] c7cf960 - [ELF] Set the priority of STB_GNU_UNIQUE the same as STB_WEAK

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2022-03-14T12:00:15-07:00
New Revision: c7cf960d859bcfe166860fc1c4eba9665693ad51

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

LOG: [ELF] Set the priority of STB_GNU_UNIQUE the same as STB_WEAK

In GCC -fgnu-unique output, STB_GNU_UNIQUE symbols are always defined relative
to a section in a COMDAT group. Currently `other` cannot be STB_GNU_UNIQUE for
valid input, so this patch is NFC.

If we switch to the model that ignores COMDAT resolution when performing symbol
resolution (D120626), this will fix bogus `relocation refers to a symbol in a
discarded section` errors when mixing -fno-gnu-unique objects with -fgnu-unique
objects.

Differential Revision: https://reviews.llvm.org/D120640

Added: 
    lld/test/ELF/comdat-binding.s

Modified: 
    lld/ELF/Symbols.cpp
    lld/ELF/Symbols.h

Removed: 
    


################################################################################
diff  --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp
index 8b74725c42cff..e8ead0d81cbd0 100644
--- a/lld/ELF/Symbols.cpp
+++ b/lld/ELF/Symbols.cpp
@@ -528,7 +528,13 @@ bool Symbol::shouldReplace(const Defined &other) const {
       return false;
   }
 
-  return isWeak() && !other.isWeak();
+  // Incoming STB_GLOBAL overrides STB_WEAK/STB_GNU_UNIQUE. -fgnu-unique changes
+  // some vague linkage data in COMDAT from STB_WEAK to STB_GNU_UNIQUE. Treat
+  // STB_GNU_UNIQUE like STB_WEAK so that we prefer the first among all
+  // STB_WEAK/STB_GNU_UNIQUE copies. If we prefer an incoming STB_GNU_UNIQUE to
+  // an existing STB_WEAK, there may be discarded section errors because the
+  // selected copy may be in a non-prevailing COMDAT.
+  return !isGlobal() && other.isGlobal();
 }
 
 void elf::reportDuplicate(const Symbol &sym, InputFile *newFile,

diff  --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
index 661e0cb5cb526..cd0e54ddf3d30 100644
--- a/lld/ELF/Symbols.h
+++ b/lld/ELF/Symbols.h
@@ -141,6 +141,7 @@ class Symbol {
 
   bool includeInDynsym() const;
   uint8_t computeBinding() const;
+  bool isGlobal() const { return binding == llvm::ELF::STB_GLOBAL; }
   bool isWeak() const { return binding == llvm::ELF::STB_WEAK; }
 
   bool isUndefined() const { return symbolKind == UndefinedKind; }

diff  --git a/lld/test/ELF/comdat-binding.s b/lld/test/ELF/comdat-binding.s
new file mode 100644
index 0000000000000..6c6fd89b57495
--- /dev/null
+++ b/lld/test/ELF/comdat-binding.s
@@ -0,0 +1,32 @@
+# REQUIRES: x86
+# RUN: rm -rf %t && split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %t/g.s -o %t/g.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %t/w.s -o %t/w.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %t/u.s -o %t/u.o
+# RUN: ld.lld -e 0 %t/w.o %t/u.o -o %t/w
+# RUN: llvm-readelf -s %t/w | FileCheck %s --check-prefix=WEAK
+# RUN: ld.lld -e 0 %t/u.o %t/w.o -o %t/u
+# RUN: llvm-readelf -s %t/u | FileCheck %s --check-prefix=UNIQUE
+
+# RUN: ld.lld -e 0 %t/w.o %t/g.o -o %t/w
+# RUN: llvm-readelf -s %t/w | FileCheck %s --check-prefix=WEAK
+
+# WEAK:   NOTYPE WEAK   DEFAULT [[#]] _ZZ1fvE1x
+# UNIQUE: OBJECT UNIQUE DEFAULT [[#]] _ZZ1fvE1x
+
+#--- g.s
+movq _ZZ1fvE1x at gotpcrel(%rip), %rax
+
+.section .bss._ZZ1fvE1x,"awG", at nobits,_ZZ1fvE1x,comdat
+.globl _ZZ1fvE1x
+_ZZ1fvE1x:
+
+#--- w.s
+.section .bss._ZZ1fvE1x,"awG", at nobits,_ZZ1fvE1x,comdat
+.weak _ZZ1fvE1x
+_ZZ1fvE1x:
+
+#--- u.s
+.section .bss._ZZ1fvE1x,"awG", at nobits,_ZZ1fvE1x,comdat
+.type _ZZ1fvE1x, @gnu_unique_object
+_ZZ1fvE1x: