Fix an issue that common symbols are not internalized under some condition.

[ruiu]

Summary

r360841 introduced CommonSymbol class. An unintended behavioral change
introduced by that change was that common symbols are not internalized
by LTO under some condition, as reported in
https://bugs.llvm.org/show_bug.cgi?id=41978.

This patch fixes that issue.

The issue occurred under the following condition:

1. There exists a common symbol
2. At least one DSO is given to lld

If the above conditions are met, Symbol::includeInDynsym() returned a
wrong value for a common symbol.

[MaskRay]

compileBitcodeFiles<ELFT>(); is executed before replaceCommonSymbols().
When includeInDynsym() is called to compute VisibileToRegularObj, it unnecessarily returned true for a common symbol.

The description can be generalized a bit:

The issue occurred under the following condition:

• There exists a common symbol.
• At least one DSO is given to lld, or -pie (i.e. ld.lld t.o t1.so or ld.lld t.o -pie)

lld/test/ELF/lto/common4.ll

2 ↗

(On Diff #206363)

llvm-mc -filetype=obj -triple=x86_64-unknown-linux /dev/null -o %t.so.o

/dev/null has been used in many other tests.

6 ↗

(On Diff #206363)

Nit: put input files together, %t.o %t.so

8 ↗

(On Diff #206363)

You may add another RUN+CHECK lines: ld.lld -pie %t.o -o %t.exe -save-temps

The purpose of the input DSO is to make HasDynSymTab true.

[MaskRay]

lld/test/ELF/lto/common4.ll

13 ↗

(On Diff #206363)

Probably add a comment saying @a does not interpose a symbol in a DSO so it does not have to be exported.

[ruiu]

• address review coments

[ruiu]

• update the test

[ruiu]

• add a comment to the test file

[MaskRay]

lld/test/ELF/lto/common4.ll

4 ↗

(On Diff #206367)

I am not sure if module-local is a proper term here.

@a does not have to exported (ExportDynamic) because there is no DSO has an undefined symbol named a.

lld/trunk/ELF/Symbols.cpp

uint8_t Symbol::computeBinding() const {
   return Binding;
 }
 
 bool Symbol::includeInDynsym() const {
   if (!Config->HasDynSymTab)
     return false;
   if (computeBinding() == STB_LOCAL)
     return false;
+
   // If a PIE binary was not linked against any shared libraries, then we can
   // safely drop weak undef symbols from .dynsym.
   if (isUndefWeak() && Config->Pie && SharedFiles.empty())
     return false;
-  if (!isDefined())
-    return true;
-  return ExportDynamic;
+
+  return isUndefined() || isShared() || ExportDynamic;
 }
 
 // Print out a log message for --trace-symbol.
 void elf::printTraceSymbol(const Symbol *Sym) {
   std::string S;
   if (Sym->isUndefined())
     S = ": reference to ";
   else if (Sym->isLazy())

lld/trunk/test/ELF/lto/common4.ll

+; REQUIRES: x86
+
+;; Make sure that common symbols are properly internalized.
+;; In this file, @a does not interpose any symbol in a DSO,
+;; so LTO should be able to internelize it.
+
+; RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux /dev/null -o %t.so.o
+; RUN: ld.lld -shared -o %t.so %t.so.o
+
+; RUN: llvm-as %s -o %t.o
+; RUN: ld.lld -o %t.exe -save-temps %t.o %t.so
+; RUN: llvm-dis < %t.exe.0.2.internalize.bc | FileCheck %s
+
+; RUN: ld.lld -pie -o %t.exe -save-temps %t.o
+; RUN: llvm-dis < %t.exe.0.2.internalize.bc | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@a = common dso_local local_unnamed_addr global i32 0, align 4
+; CHECK-DAG: @a = internal global i32 0, align 4