[lld] ce45c95 - [ELF] Remove obscure -dp and GNU ld incompatible --[no-]define-common, ignore -d/-dc

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 9 10:35:57 PST 2022


Author: Fangrui Song
Date: 2022-02-09T10:35:53-08:00
New Revision: ce45c956942e4fe301102ed807efc726399623f9

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

LOG: [ELF] Remove obscure -dp and GNU ld incompatible --[no-]define-common, ignore -d/-dc

https://maskray.me/blog/2022-02-06-all-about-common-symbols#no-define-common

In GNU ld, -dc only affects -r links and causes COMMON symbols to be allocated.
--no-define-common is defined to make COMMON symbols undefined for -shared.
AIUI --no-define-common is a workaround around glibc 2.1 time and not really useful.

gold confuses --define-common with -d/FORCE_COMMON_ALLOCATION and implements
--define-common with -d semantics. Its --no-define-common is incompatible with
GNU ld.

In ld.lld, b2a23cf3c08cee45614f27eb2c6d044e506aa6a6 fixed the default -r
behavior for COMMON symbols but ported the incompatible gold
--[no-]define-common. To the best of my knowledge, no project uses -dp
--[no-]define-common. So just remove these options.

-d/-dc are used by the following projects:

* grub grub-core/genmod.sh.in uses -Wl,-r,-d (https://lists.gnu.org/archive/html/grub-devel/2022-02/msg00088.html)
* FreeBSD crunchgen uses -Wl,-dc (https://reviews.freebsd.org/D34215)

A no-op implementation works for them. Only when a program inspects relocatable
output by itself and does not recognize COMMON symbols, there may be a problem.
This is an extremely unlikely case.

Reviewed By: peter.smith

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

Added: 
    

Modified: 
    lld/ELF/Config.h
    lld/ELF/Driver.cpp
    lld/ELF/Options.td
    lld/ELF/SyntheticSections.cpp
    lld/docs/ReleaseNotes.rst
    lld/docs/ld.lld.1
    lld/test/ELF/relocatable-common.s
    lld/test/ELF/silent-ignore.test

Removed: 
    


################################################################################
diff  --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index 6d5c4cb592878..4b2ad7eb15db5 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -160,7 +160,6 @@ struct Configuration {
   bool compressDebugSections;
   bool cref;
   std::vector<std::pair<llvm::GlobPattern, uint64_t>> deadRelocInNonAlloc;
-  bool defineCommon;
   bool demangle = true;
   bool dependentLibraries;
   bool disableVerify;

diff  --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index e6a7731f01cd4..d9d9911987654 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -334,9 +334,6 @@ static void checkOptions() {
   if (!config->shared && !config->auxiliaryList.empty())
     error("-f may not be used without -shared");
 
-  if (!config->relocatable && !config->defineCommon)
-    error("-no-define-common not supported in non relocatable output");
-
   if (config->strip == StripPolicy::All && config->emitRelocs)
     error("--strip-all and --emit-relocs may not be used together");
 
@@ -996,8 +993,6 @@ static void readConfigs(opt::InputArgList &args) {
   config->chroot = args.getLastArgValue(OPT_chroot);
   config->compressDebugSections = getCompressDebugSections(args);
   config->cref = args.hasArg(OPT_cref);
-  config->defineCommon = args.hasFlag(OPT_define_common, OPT_no_define_common,
-                                      !args.hasArg(OPT_relocatable));
   config->optimizeBBJumps =
       args.hasFlag(OPT_optimize_bb_jumps, OPT_no_optimize_bb_jumps, false);
   config->demangle = args.hasFlag(OPT_demangle, OPT_no_demangle, true);

diff  --git a/lld/ELF/Options.td b/lld/ELF/Options.td
index ca9fdcde791f0..40b064c4c5f5f 100644
--- a/lld/ELF/Options.td
+++ b/lld/ELF/Options.td
@@ -129,10 +129,6 @@ def color_diagnostics_eq: JJ<"color-diagnostics=">,
 def cref: FF<"cref">,
   HelpText<"Output cross reference table. If -Map is specified, print to the map file">;
 
-defm define_common: B<"define-common",
-    "Assign space to common symbols",
-    "Do not assign space to common symbols">;
-
 defm demangle: B<"demangle",
     "Demangle symbol names (default)",
     "Do not demangle symbol names">;
@@ -512,9 +508,6 @@ def: F<"dy">, Alias<Bdynamic>, HelpText<"Alias for --Bdynamic">;
 def: F<"dn">, Alias<Bstatic>, HelpText<"Alias for --Bstatic">;
 def: F<"non_shared">, Alias<Bstatic>, HelpText<"Alias for --Bstatic">;
 def: F<"static">, Alias<Bstatic>, HelpText<"Alias for --Bstatic">;
-def: Flag<["-"], "d">, Alias<define_common>, HelpText<"Alias for --define-common">;
-def: F<"dc">, Alias<define_common>, HelpText<"Alias for --define-common">;
-def: F<"dp">, Alias<define_common>, HelpText<"Alias for --define-common">;
 def: Flag<["-"], "x">, Alias<discard_all>, HelpText<"Alias for --discard-all">;
 def: Flag<["-"], "X">, Alias<discard_locals>, HelpText<"Alias for --discard-locals">;
 def: Flag<["-"], "q">, Alias<emit_relocs>, HelpText<"Alias for --emit-relocs">;
@@ -693,6 +686,8 @@ def plugin_opt_eq : J<"plugin-opt=">;
 
 // Options listed below are silently ignored for now for compatibility.
 def: F<"detect-odr-violations">;
+def: Flag<["-"], "d">;
+def: Flag<["-"], "dc">;
 def: Flag<["-"], "g">;
 def: F<"long-plt">;
 def: F<"no-copy-dt-needed-entries">;

diff  --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index 5fcfd3bbf38fc..0cc2cfb62b2ca 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -2192,7 +2192,7 @@ SymbolTableSection<ELFT>::SymbolTableSection(StringTableSection &strTabSec)
 }
 
 static BssSection *getCommonSec(Symbol *sym) {
-  if (!config->defineCommon)
+  if (config->relocatable)
     if (auto *d = dyn_cast<Defined>(sym))
       return dyn_cast_or_null<BssSection>(d->section);
   return nullptr;
@@ -2234,8 +2234,8 @@ template <class ELFT> void SymbolTableSection<ELFT>::writeTo(uint8_t *buf) {
       eSym->st_other |= sym->stOther & STO_AARCH64_VARIANT_PCS;
 
     if (BssSection *commonSec = getCommonSec(sym)) {
-      // st_value is usually an address of a symbol, but that has a special
-      // meaning for uninstantiated common symbols (--no-define-common).
+      // When -r is specified, a COMMON symbol is not allocated. Its st_shndx
+      // holds SHN_COMMON and st_value holds the alignment.
       eSym->st_shndx = SHN_COMMON;
       eSym->st_value = commonSec->alignment;
       eSym->st_size = cast<Defined>(sym)->size;

diff  --git a/lld/docs/ReleaseNotes.rst b/lld/docs/ReleaseNotes.rst
index c4554bf76a262..1a31c13eb8905 100644
--- a/lld/docs/ReleaseNotes.rst
+++ b/lld/docs/ReleaseNotes.rst
@@ -29,7 +29,9 @@ ELF Improvements
 Breaking changes
 ----------------
 
-* ...
+* The GNU ld incompatible ``--no-define-common`` has been removed.
+* The obscure ``-dc``/``-dp`` options have been removed.
+* ``-d`` is now ignored.
 
 COFF Improvements
 -----------------

diff  --git a/lld/docs/ld.lld.1 b/lld/docs/ld.lld.1
index da43cf0ef7abf..58b3b8c05d9f5 100644
--- a/lld/docs/ld.lld.1
+++ b/lld/docs/ld.lld.1
@@ -144,8 +144,9 @@ to set the compression level to 6.
 Output cross reference table. If
 .Fl Map
 is specified, print to the map file.
-.It Fl -define-common , Fl d
-Assign space to common symbols.
+.It Fl dc
+.Fl r
+specific: assign space to COMMON symbols like building an executable or shared object.
 .It Fl -defsym Ns = Ns Ar symbol Ns = Ns Ar expression
 Define a symbol alias.
 .Ar expression
@@ -326,8 +327,6 @@ Always set
 for shared libraries.
 .It Fl -no-color-diagnostics
 Do not use colors in diagnostics.
-.It Fl -no-define-common
-Do not assign space to common symbols.
 .It Fl -no-demangle
 Do not demangle symbol names.
 .It Fl -no-dynamic-linker

diff  --git a/lld/test/ELF/relocatable-common.s b/lld/test/ELF/relocatable-common.s
index 0c89cea9be406..7f81ec4177dcd 100644
--- a/lld/test/ELF/relocatable-common.s
+++ b/lld/test/ELF/relocatable-common.s
@@ -2,16 +2,6 @@
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1.o
 # RUN: ld.lld -r %t1.o -o %t
 # RUN: llvm-readobj --symbols -r %t | FileCheck %s
-# RUN: ld.lld -r --no-define-common %t1.o -o %t
-# RUN: llvm-readobj --symbols -r %t | FileCheck %s
-# RUN: ld.lld -r --define-common %t1.o -o %t
-# RUN: llvm-readobj --symbols -r %t | FileCheck -check-prefix=DEFCOMM %s
-# RUN: ld.lld -r -d %t1.o -o %t
-# RUN: llvm-readobj --symbols -r %t | FileCheck -check-prefix=DEFCOMM %s
-# RUN: ld.lld -r -dc %t1.o -o %t
-# RUN: llvm-readobj --symbols -r %t | FileCheck -check-prefix=DEFCOMM %s
-# RUN: ld.lld -r -dp %t1.o -o %t
-# RUN: llvm-readobj --symbols -r %t | FileCheck -check-prefix=DEFCOMM %s
 
 # CHECK:        Symbol {
 # CHECK:          Name: common
@@ -23,17 +13,4 @@
 # CHECK-NEXT:     Section: Common (0xFFF2)
 # CHECK-NEXT:   }
 
-# DEFCOMM:      Symbol {
-# DEFCOMM:        Name: common
-# DEFCOMM-NEXT:   Value: 0x0
-# DEFCOMM-NEXT:   Size: 4
-# DEFCOMM-NEXT:   Binding: Global
-# DEFCOMM-NEXT:   Type: Object
-# DEFCOMM-NEXT:   Other: 0
-# DEFCOMM-NEXT:   Section: COMMON
-# DEFCOMM-NEXT: }
-
-# RUN: not ld.lld -shared --no-define-common %t1.o -o /dev/null 2>&1 | FileCheck --check-prefix=ERROR %s
-# ERROR: error: -no-define-common not supported in non relocatable output
-
 .comm common,4,4

diff  --git a/lld/test/ELF/silent-ignore.test b/lld/test/ELF/silent-ignore.test
index 91b57434765f2..c9289b5ab716e 100644
--- a/lld/test/ELF/silent-ignore.test
+++ b/lld/test/ELF/silent-ignore.test
@@ -1,4 +1,6 @@
 RUN: ld.lld --version \
+RUN:   -d \
+RUN:   -dc \
 RUN:   -detect-odr-violations \
 RUN:   -g \
 RUN:   -long-plt \


        


More information about the llvm-commits mailing list