[PATCH] D105945: [ELF] --fortran-common: prefer STB_WEAK to COMMON

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: peter.smith, sfertile.
Herald added subscribers: arichardson, emaste.
MaskRay requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

The ELF specification says "The link editor honors the common definition and
ignores the weak ones." GNU ld and our Symbol::compare follow this, but the
--fortran-common code (D86142 <https://reviews.llvm.org/D86142>) made a mistake on the precedence.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105945

Files:
  lld/ELF/InputFiles.cpp
  lld/test/ELF/common-archive-lookup.s


Index: lld/test/ELF/common-archive-lookup.s
===================================================================
--- lld/test/ELF/common-archive-lookup.s
+++ lld/test/ELF/common-archive-lookup.s
@@ -20,6 +20,7 @@
 ## Bitcode files.
 # RUN: llvm-as -o 1.bc commonblock.ll
 # RUN: llvm-as -o 2.bc blockdata.ll
+# RUN: llvm-as -o 3.bc weak.ll
 
 ## Bitcode archive.
 # RUN: llvm-ar crs 4.a 1.bc 2.bc
@@ -31,10 +32,10 @@
 # RUN: llvm-objdump -D -j .data 2 | FileCheck --check-prefix=TEST1 %s
 
 # RUN: ld.lld -o 3 main.o 2.a
-# RUN: llvm-objdump -D -j .data 3 | FileCheck --check-prefix=TEST1 %s
+# RUN: llvm-objdump -t 3 | FileCheck --check-prefix=BSS %s
 
 # RUN: ld.lld -o 4 main.o --start-lib 1.o weak_data_only.o --end-lib
-# RUN: llvm-objdump -D -j .data 4 | FileCheck --check-prefix=TEST1 %s
+# RUN: llvm-objdump -t 4 | FileCheck --check-prefix=BSS %s
 
 # RUN: ld.lld -o 5 main.o 3.a --print-map | FileCheck --check-prefix=MAP %s
 
@@ -63,6 +64,9 @@
 # RUN: ld.lld -o - main.o  --start-lib 1.bc 2.bc --end-lib --lto-emit-asm | \
 # RUN:   FileCheck --check-prefix=ASM %s
 
+## COMMON overrides weak. Don't extract 3.bc which provides a weak definition.
+# RUN: ld.lld -o /dev/null main.o --start-lib 1.bc 3.bc --end-lib -y block | FileCheck --check-prefix=LTO_WEAK %s
+
 ## Old FORTRAN that mixes use of COMMON blocks and BLOCK DATA requires that we
 ## search through archives for non-tentative definitions (from the BLOCK DATA)
 ## to replace the tentative definitions (from the COMMON block(s)).
@@ -75,6 +79,7 @@
 # TEST1-NEXT:       fb 21 09 40
 # TEST1-NEXT:       ...
 
+# BSS:       [[#%x,]] g     O .bss   0000000000000028 block
 
 # NFC:       Name: block
 # NFC-NEXT:  Value:
@@ -100,6 +105,10 @@
 # ASM-NEXT:    .long 5
 # ASM:         .size   block, 20
 
+# LTO_WEAK:     1.bc: common definition of block
+# LTO_WEAK:     <internal>: reference to block
+# LTO_WEAK-NOT: {{.}}
+
 #--- ref.s
   .text
   .abiversion 2
@@ -167,6 +176,12 @@
 
 @block = dso_local local_unnamed_addr global [5 x i32] [i32 5, i32 0, i32 0, i32 0, i32 0], align 4
 
+#--- weak.ll
+target datalayout = "e-m:e-i64:64-n32:64-S128-v256:256:256-v512:512:512"
+target triple = "powerpc64le-unknown-linux-gnu"
+
+ at block = weak dso_local global [5 x i32] [i32 5, i32 0, i32 0, i32 0, i32 0], align 4
+
 #--- commonblock.ll
 target datalayout = "e-m:e-i64:64-n32:64-S128-v256:256:256-v512:512:512"
 target triple = "powerpc64le-unknown-linux-gnu"
Index: lld/ELF/InputFiles.cpp
===================================================================
--- lld/ELF/InputFiles.cpp
+++ lld/ELF/InputFiles.cpp
@@ -1272,7 +1272,7 @@
 //
 // 2) Consider the tentative definition as still undefined (ie the promotion to
 //    a real definition happens only after all symbol resolution is done).
-//    The linker searches archive members for global or weak definitions to
+//    The linker searches archive members for STB_GLOBAL definitions to
 //    replace the tentative definition with. This is the behavior used by
 //    GNU ld.
 //
@@ -1288,7 +1288,7 @@
   for (const irsymtab::Reader::SymbolRef &sym :
        symtabFile.TheReader.symbols()) {
     if (sym.isGlobal() && sym.getName() == symName)
-      return !sym.isUndefined() && !sym.isCommon();
+      return !sym.isUndefined() && !sym.isWeak() && !sym.isCommon();
   }
   return false;
 }
@@ -1302,7 +1302,8 @@
   for (auto sym : obj->template getGlobalELFSyms<ELFT>()) {
     Expected<StringRef> name = sym.getName(stringtable);
     if (name && name.get() == symName)
-      return sym.isDefined() && !sym.isCommon();
+      return sym.isDefined() && sym.getBinding() == STB_GLOBAL &&
+             !sym.isCommon();
   }
   return false;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D105945.358447.patch
Type: text/x-patch
Size: 3677 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210713/3a9baf74/attachment.bin>