[ELF] implement --warn-common/--no-warn-common

[grimar]

Summary

-warn-common

Warn when a common symbol is combined with another common symbol
or with a symbol definition.  Unix linkers allow  this  somewhat
sloppy  practice, but linkers on some other operating systems do
not.  This option allows you to  find  potential  problems  from
combining global symbols.

This should fix the the https://llvm.org/bugs/show_bug.cgi?id=26737

[ruiu]

ELF/Config.h

77 ↗

(On Diff #50153)

Remove = false.

ELF/Symbols.cpp

145–152 ↗

(On Diff #50153)

I wouldn't add new code to this function. Instead, please try to make this a new function or refactor this function to make it shorter.

[grimar]

ELF/Symbols.cpp

145–152 ↗

(On Diff #50153)

My version of refactor is here: http://reviews.llvm.org/D18004

[grimar]

• Rebased after refactoring

[grimar]

• Addressed review comment

[ruiu]

What about the Joerg's comment? Doesn't this always warn on two common symbol who have the same name?

[grimar]

In D17998#373261, @ruiu wrote:

What about the Joerg's comment? Doesn't this always warn on two common symbol who have the same name?

Yea, I answered already via llvm-mails, but will duplicate answer here for history of patch.
Current logic always warn about such symbols. I did that intentionaly because gold do the same,
consider 2 files:

test.s:

.globl _start
_start:

.type arr,@object
.comm arr,4,4

test2.s:

.type arr,@object
.comm arr,4,4

So 2 commons have the same size and gold produces a warning in that case:
++ /home/LLVM/build/bin/llvm-mc -filetype=obj test.s -o test.o
++ /home/LLVM/build/bin/llvm-mc -filetype=obj test2.s -o test2.o
++ gold -warn-common test.o test2.o -o test
gold: warning: test2.o: multiple common of 'arr'
gold: test.o: previous definition here

[ruiu]

LGTM

lld/trunk/ELF/Config.h

struct Configuration {
   bool Relocatable;
   bool SaveTemps;
   bool Shared;
   bool Static = false;
   bool StripAll;
   bool SysvHash = true;
   bool Threads;
   bool Verbose;
+  bool WarnCommon;
   bool ZExecStack;
   bool ZNodelete;
   bool ZNow;
   bool ZOrigin;
   bool ZRelro;
   ELFKind EKind = ELFNoneKind;
   uint16_t EMachine = llvm::ELF::EM_NONE;
   uint64_t EntryAddr = -1;

lld/trunk/ELF/Driver.cpp

void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   Config->NoinhibitExec = Args.hasArg(OPT_noinhibit_exec);
   Config->PrintGcSections = Args.hasArg(OPT_print_gc_sections);
   Config->Relocatable = Args.hasArg(OPT_relocatable);
   Config->SaveTemps = Args.hasArg(OPT_save_temps);
   Config->Shared = Args.hasArg(OPT_shared);
   Config->StripAll = Args.hasArg(OPT_strip_all);
   Config->Threads = Args.hasArg(OPT_threads);
   Config->Verbose = Args.hasArg(OPT_verbose);
+  Config->WarnCommon = Args.hasArg(OPT_warn_common);
 
   Config->DynamicLinker = getString(Args, OPT_dynamic_linker);
   Config->Entry = getString(Args, OPT_entry);
   Config->Fini = getString(Args, OPT_fini, "_fini");
   Config->Init = getString(Args, OPT_init, "_init");
   Config->OutputFile = getString(Args, OPT_o);
   Config->SoName = getString(Args, OPT_soname);
   Config->Sysroot = getString(Args, OPT_sysroot);

lld/trunk/ELF/Options.td

 def undefined : Joined<["--"], "undefined=">,
   HelpText<"Force undefined symbol during linking">;
 
 def verbose : Flag<["--"], "verbose">;
 
 def version : Flag<["--", "-"], "version">,
   HelpText<"Display the version number">;
 
+def warn_common : Flag<["--", "-"], "warn-common">,
+  HelpText<"Warn about duplicate common symbols">;
+
 def whole_archive : Flag<["--", "-"], "whole-archive">,
   HelpText<"Force load of all members in a static library">;
 
 def wrap : Separate<["--", "-"], "wrap">, MetaVarName<"<symbol>">,
   HelpText<"Use wrapper functions for symbol">;
 
 def z : JoinedOrSeparate<["-"], "z">, MetaVarName<"<option>">,
   HelpText<"Linker option extensions">;

 def end_group_paren: Flag<["-"], ")">;
 def start_group : Flag<["--"], "start-group">;
 def start_group_paren: Flag<["-"], "(">;
 
 // Options listed below are silently ignored for now for compatibility.
 def fatal_warnings : Flag<["--"], "fatal-warnings">;
 def no_add_needed : Flag<["--"], "no-add-needed">;
 def no_fatal_warnings : Flag<["--"], "no-fatal-warnings">;
+def no_warn_common : Flag<["--", "-"], "no-warn-common">;
 def no_warn_mismatch : Flag<["--"], "no-warn-mismatch">;
 def version_script : Separate<["--"], "version-script">;
-def warn_common : Flag<["--"], "warn-common">;
 def warn_shared_textrel : Flag<["--"], "warn-shared-textrel">;
 def G : Separate<["-"], "G">;
 
 // Aliases for ignored options
 def alias_version_script_version_script : Joined<["--"], "version-script=">, Alias<version_script>;
 
 // Debugging options
 def save_temps : Flag<["-"], "save-temps">;

lld/trunk/ELF/Symbols.cpp

static uint8_t getMinVisibility(uint8_t VA, uint8_t VB) {
   if (VA == STV_DEFAULT)
     return VB;
   if (VB == STV_DEFAULT)
     return VA;
   return std::min(VA, VB);
 }
 
 static int compareCommons(DefinedCommon *A, DefinedCommon *B) {
+  if (Config->WarnCommon)
+    warning("multiple common of " + A->getName());
   A->Alignment = B->Alignment = std::max(A->Alignment, B->Alignment);
   if (A->Size < B->Size)
     return -1;
   return 1;
 }
 
 // Returns 1, 0 or -1 if this symbol should take precedence
 // over the Other, tie or lose, respectively.
if (L != R)
     return -1;
   if (!isDefined() || isShared() || isWeak())
     return 1;
   if (!isCommon() && !Other->isCommon())
     return 0;
   if (isCommon() && Other->isCommon())
     return compareCommons(cast<DefinedCommon>(this),
                           cast<DefinedCommon>(Other));
+  if (Config->WarnCommon)
+    warning("common " + this->getName() + " is overridden");
   return isCommon() ? -1 : 1;
 }
 
 Defined::Defined(Kind K, StringRef Name, bool IsWeak, bool IsLocal,
                  uint8_t Visibility, uint8_t Type)
     : SymbolBody(K, Name, IsWeak, IsLocal, Visibility, Type) {}
 
 DefinedBitcode::DefinedBitcode(StringRef Name, bool IsWeak, uint8_t Visibility)

lld/trunk/test/ELF/Inputs/warn-common.s

+.type arr,@object
+.comm arr,8,4

lld/trunk/test/ELF/Inputs/warn-common2.s

+.type arr,@object
+.data
+.globl arr
+.p2align 2
+arr:
+ .long 1
+ .long 0
+ .size arr, 8

lld/trunk/test/ELF/warn-common.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t1.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/warn-common.s -o %t2.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/warn-common2.s -o %t3.o
+
+## Report multiple commons if warn-common is specified
+# RUN: ld.lld --warn-common %t1.o %t2.o -o %t.out 2>&1 | FileCheck %s --check-prefix=WARN
+# WARN: multiple common of arr
+
+## no-warn-common is ignored
+# RUN: ld.lld --no-warn-common %t1.o %t2.o -o %t.out
+# RUN: llvm-readobj %t.out > /dev/null
+
+## Report if common is overridden
+# RUN: ld.lld --warn-common %t1.o %t3.o -o %t.out 2>&1 | FileCheck %s --check-prefix=OVER
+# OVER: common arr is overridden
+
+## Report if common is overridden, but in different order
+# RUN: ld.lld --warn-common %t3.o %t1.o -o %t.out 2>&1 | FileCheck %s --check-prefix=OVER
+
+.globl _start
+_start:
+
+.type arr,@object
+.comm arr,4,4