Perform symbol binding for .symver versioned symbols

Summary:
In a .symver assembler directive like:
.symver name, name2@@nodename
"name2@@nodename" should get the same symbol binding as "name".

While the ELF object writer is updating the symbol binding for .symver
aliases before emitting the object file, not doing so when the module
inline assembly is handled by the RecordStreamer is causing the wrong
behavior in *LTO mode.

E.g. when "name" is global, "name2@@nodename" must also be marked as
global. Otherwise, the symbol is skipped when iterating over the LTO
InputFile symbols (InputFile::Symbol::shouldSkip). So, for example,
when performing any *LTO via the gold-plugin, the versioned symbol
definition is not recorded by the plugin and passed back to the
linker. If the object was in an archive, and there were no other symbols
needed from that object, the object would not be included in the final
link and references to the versioned symbol are undefined.

The llvm-lto2 tests added will give an error about an unused symbol
resolution without the fix.

Reviewers: rafael, pcc

Reviewed By: pcc

Subscribers: mehdi_amini, llvm-commits

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@297332 91177308-0d34-0410-b5e6-96231b3b80d8

12 files changed, 190 insertions, 18 deletions

diff --git a/include/llvm/MC/MCStreamer.h b/include/llvm/MC/MCStreamer.h
index b477a608c26..c0d322e3ed3 100644
--- a/include/llvm/MC/MCStreamer.h
+++ b/include/llvm/MC/MCStreamer.h
@@ -489,6 +489,14 @@ public:
   ///  .size symbol, expression
   virtual void emitELFSize(MCSymbol *Symbol, const MCExpr *Value);
 
+  /// \brief Emit an ELF .symver directive.
+  ///
+  /// This corresponds to an assembler statement such as:
+  ///  .symver _start, foo@@SOME_VERSION
+  /// \param Alias - The versioned alias (i.e. "foo@@SOME_VERSION")
+  /// \param Aliasee - The aliased symbol (i.e. "_start")
+  virtual void emitELFSymverDirective(MCSymbol *Alias, const MCSymbol *Aliasee);
+
   /// \brief Emit a Linker Optimization Hint (LOH) directive.
   /// \param Args - Arguments of the LOH.
   virtual void EmitLOHDirective(MCLOHType Kind, const MCLOHArgs &Args) {}
diff --git a/include/llvm/Object/ModuleSymbolTable.h b/include/llvm/Object/ModuleSymbolTable.h
index 70775352d97..333301d5b45 100644
--- a/include/llvm/Object/ModuleSymbolTable.h
+++ b/include/llvm/Object/ModuleSymbolTable.h
@@ -26,6 +26,7 @@
 namespace llvm {
 
 class GlobalValue;
+class RecordStreamer;
 
 class ModuleSymbolTable {
 public:
@@ -52,7 +53,7 @@ public:
   /// For each found symbol, call \p AsmSymbol with the name of the symbol found
   /// and the associated flags.
   static void CollectAsmSymbols(
-      const Triple &TheTriple, StringRef InlineAsm,
+      const Module &M,
       function_ref<void(StringRef, object::BasicSymbolRef::Flags)> AsmSymbol);
 };
 
diff --git a/lib/Analysis/ModuleSummaryAnalysis.cpp b/lib/Analysis/ModuleSummaryAnalysis.cpp
index 9bc55034c14..87b6c0d18a0 100644
--- a/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -410,9 +410,8 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
     // be listed on the llvm.used or llvm.compiler.used global and marked as
     // referenced from there.
     ModuleSymbolTable::CollectAsmSymbols(
-        Triple(M.getTargetTriple()), M.getModuleInlineAsm(),
-        [&M, &Index, &CantBePromoted](StringRef Name,
-                                      object::BasicSymbolRef::Flags Flags) {
+        M, [&M, &Index, &CantBePromoted](StringRef Name,
+                                         object::BasicSymbolRef::Flags Flags) {
           // Symbols not marked as Weak or Global are local definitions.
           if (Flags & (object::BasicSymbolRef::SF_Weak |
                        object::BasicSymbolRef::SF_Global))
diff --git a/lib/LTO/LTOBackend.cpp b/lib/LTO/LTOBackend.cpp
index 25795b1332e..605a32d7331 100644
--- a/lib/LTO/LTOBackend.cpp
+++ b/lib/LTO/LTOBackend.cpp
@@ -354,7 +354,7 @@ static void handleAsmUndefinedRefs(Module &Mod, TargetMachine &TM) {
   // llvm.compiler.used to prevent optimization to drop these from the output.
   StringSet<> AsmUndefinedRefs;
   ModuleSymbolTable::CollectAsmSymbols(
-      Triple(Mod.getTargetTriple()), Mod.getModuleInlineAsm(),
+      Mod,
       [&AsmUndefinedRefs](StringRef Name, object::BasicSymbolRef::Flags Flags) {
         if (Flags & object::BasicSymbolRef::SF_Undefined)
           AsmUndefinedRefs.insert(Name);
diff --git a/lib/MC/MCParser/ELFAsmParser.cpp b/lib/MC/MCParser/ELFAsmParser.cpp
index b9426566b5c..fff081f0445 100644
--- a/lib/MC/MCParser/ELFAsmParser.cpp
+++ b/lib/MC/MCParser/ELFAsmParser.cpp
@@ -743,6 +743,7 @@ bool ELFAsmParser::ParseDirectiveSymver(StringRef, SMLoc) {
   const MCExpr *Value = MCSymbolRefExpr::create(Sym, getContext());
 
   getStreamer().EmitAssignment(Alias, Value);
+  getStreamer().emitELFSymverDirective(Alias, Sym);
   return false;
 }
 
diff --git a/lib/MC/MCStreamer.cpp b/lib/MC/MCStreamer.cpp
index 860c1bd6d87..b9c01c66f31 100644
--- a/lib/MC/MCStreamer.cpp
+++ b/lib/MC/MCStreamer.cpp
@@ -821,6 +821,8 @@ void MCStreamer::EmitCOFFSymbolType(int Type) {
   llvm_unreachable("this directive only supported on COFF targets");
 }
 void MCStreamer::emitELFSize(MCSymbol *Symbol, const MCExpr *Value) {}
+void MCStreamer::emitELFSymverDirective(MCSymbol *Alias,
+                                        const MCSymbol *Aliasee) {}
 void MCStreamer::EmitLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                                        unsigned ByteAlignment) {}
 void MCStreamer::EmitTBSSSymbol(MCSection *Section, MCSymbol *Symbol,
diff --git a/lib/Object/ModuleSymbolTable.cpp b/lib/Object/ModuleSymbolTable.cpp
index 90488007ff5..21709b5c223 100644
--- a/lib/Object/ModuleSymbolTable.cpp
+++ b/lib/Object/ModuleSymbolTable.cpp
@@ -50,20 +50,95 @@ void ModuleSymbolTable::addModule(Module *M) {
   for (GlobalAlias &GA : M->aliases())
     SymTab.push_back(&GA);
 
-  CollectAsmSymbols(Triple(M->getTargetTriple()), M->getModuleInlineAsm(),
-                    [this](StringRef Name, BasicSymbolRef::Flags Flags) {
-                      SymTab.push_back(new (AsmSymbols.Allocate())
-                                           AsmSymbol(Name, Flags));
-                    });
+  CollectAsmSymbols(*M, [this](StringRef Name, BasicSymbolRef::Flags Flags) {
+    SymTab.push_back(new (AsmSymbols.Allocate()) AsmSymbol(Name, Flags));
+  });
+}
+
+// Ensure ELF .symver aliases get the same binding as the defined symbol
+// they alias with.
+static void handleSymverAliases(const Module &M, RecordStreamer &Streamer) {
+  if (Streamer.symverAliases().empty())
+    return;
+
+  // The name in the assembler will be mangled, but the name in the IR
+  // might not, so we first compute a mapping from mangled name to GV.
+  Mangler Mang;
+  SmallString<64> MangledName;
+  StringMap<const GlobalValue *> MangledNameMap;
+  auto GetMangledName = [&](const GlobalValue &GV) {
+    if (!GV.hasName())
+      return;
+
+    MangledName.clear();
+    MangledName.reserve(GV.getName().size() + 1);
+    Mang.getNameWithPrefix(MangledName, &GV, /*CannotUsePrivateLabel=*/false);
+    MangledNameMap[MangledName] = &GV;
+  };
+  for (const Function &F : M)
+    GetMangledName(F);
+  for (const GlobalVariable &GV : M.globals())
+    GetMangledName(GV);
+  for (const GlobalAlias &GA : M.aliases())
+    GetMangledName(GA);
+
+  // Walk all the recorded .symver aliases, and set up the binding
+  // for each alias.
+  for (auto &Symver : Streamer.symverAliases()) {
+    const MCSymbol *Aliasee = Symver.first;
+    MCSymbolAttr Attr = MCSA_Invalid;
+
+    // First check if the aliasee binding was recorded in the asm.
+    RecordStreamer::State state = Streamer.getSymbolState(Aliasee);
+    switch (state) {
+    case RecordStreamer::Global:
+    case RecordStreamer::DefinedGlobal:
+      Attr = MCSA_Global;
+      break;
+    case RecordStreamer::UndefinedWeak:
+    case RecordStreamer::DefinedWeak:
+      Attr = MCSA_Weak;
+      break;
+    default:
+      break;
+    }
+
+    // If we don't have a symbol attribute from assembly, then check if
+    // the aliasee was defined in the IR.
+    if (Attr == MCSA_Invalid) {
+      const auto *GV = M.getNamedValue(Aliasee->getName());
+      if (!GV) {
+        auto MI = MangledNameMap.find(Aliasee->getName());
+        if (MI != MangledNameMap.end())
+          GV = MI->second;
+        else
+          continue;
+      }
+      if (GV->hasExternalLinkage())
+        Attr = MCSA_Global;
+      else if (GV->hasLocalLinkage())
+        Attr = MCSA_Local;
+      else if (GV->isWeakForLinker())
+        Attr = MCSA_Weak;
+    }
+    if (Attr == MCSA_Invalid)
+      continue;
+
+    // Set the detected binding on each alias with this aliasee.
+    for (auto &Alias : Symver.second)
+      Streamer.EmitSymbolAttribute(Alias, Attr);
+  }
 }
 
 void ModuleSymbolTable::CollectAsmSymbols(
-    const Triple &TT, StringRef InlineAsm,
+    const Module &M,
     function_ref<void(StringRef, BasicSymbolRef::Flags)> AsmSymbol) {
+  StringRef InlineAsm = M.getModuleInlineAsm();
   if (InlineAsm.empty())
     return;
 
   std::string Err;
+  const Triple TT(M.getTargetTriple());
   const Target *T = TargetRegistry::lookupTarget(TT.str(), Err);
   assert(T && T->hasMCAsmParser());
 
@@ -106,6 +181,8 @@ void ModuleSymbolTable::CollectAsmSymbols(
   if (Parser->Run(false))
     return;
 
+  handleSymverAliases(M, Streamer);
+
   for (auto &KV : Streamer) {
     StringRef Key = KV.first();
     RecordStreamer::State Value = KV.second;
diff --git a/lib/Object/RecordStreamer.cpp b/lib/Object/RecordStreamer.cpp
index da99b3fea00..8be6d0b60cf 100644
--- a/lib/Object/RecordStreamer.cpp
+++ b/lib/Object/RecordStreamer.cpp
@@ -110,3 +110,8 @@ void RecordStreamer::EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                                       unsigned ByteAlignment) {
   markDefined(*Symbol);
 }
+
+void RecordStreamer::emitELFSymverDirective(MCSymbol *Alias,
+                                            const MCSymbol *Aliasee) {
+  SymverAliasMap[Aliasee].push_back(Alias);
+}
diff --git a/lib/Object/RecordStreamer.h b/lib/Object/RecordStreamer.h
index c795dbb2e43..c3bd5b09a9b 100644
--- a/lib/Object/RecordStreamer.h
+++ b/lib/Object/RecordStreamer.h
@@ -20,6 +20,10 @@ public:
 
 private:
   StringMap<State> Symbols;
+  // Map of aliases created by .symver directives, saved so we can update
+  // their symbol binding after parsing complete. This maps from each
+  // aliasee to its list of aliases.
+  DenseMap<const MCSymbol *, std::vector<MCSymbol *>> SymverAliasMap;
   void markDefined(const MCSymbol &Symbol);
   void markGlobal(const MCSymbol &Symbol, MCSymbolAttr Attribute);
   void markUsed(const MCSymbol &Symbol);
@@ -38,6 +42,20 @@ public:
                     unsigned ByteAlignment) override;
   void EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         unsigned ByteAlignment) override;
+  /// Record .symver aliases for later processing.
+  void emitELFSymverDirective(MCSymbol *Alias,
+                              const MCSymbol *Aliasee) override;
+  /// Return the map of .symver aliasee to associated aliases.
+  DenseMap<const MCSymbol *, std::vector<MCSymbol *>> &symverAliases() {
+    return SymverAliasMap;
+  }
+  /// Get the state recorded for the given symbol.
+  State getSymbolState(const MCSymbol *Sym) {
+    auto SI = Symbols.find(Sym->getName());
+    if (SI == Symbols.end())
+      return NeverSeen;
+    return SI->second;
+  }
 };
 }
 #endif
diff --git a/lib/Transforms/IPO/FunctionImport.cpp b/lib/Transforms/IPO/FunctionImport.cpp
index 76cfbf81bd1..2c6dbb2a4ee 100644
--- a/lib/Transforms/IPO/FunctionImport.cpp
+++ b/lib/Transforms/IPO/FunctionImport.cpp
@@ -598,7 +598,7 @@ void llvm::thinLTOInternalizeModule(Module &TheModule,
   // the current module.
   StringSet<> AsmUndefinedRefs;
   ModuleSymbolTable::CollectAsmSymbols(
-      Triple(TheModule.getTargetTriple()), TheModule.getModuleInlineAsm(),
+      TheModule,
       [&AsmUndefinedRefs](StringRef Name, object::BasicSymbolRef::Flags Flags) {
         if (Flags & object::BasicSymbolRef::SF_Undefined)
           AsmUndefinedRefs.insert(Name);
diff --git a/test/LTO/X86/symver-asm.ll b/test/LTO/X86/symver-asm.ll
index 03dda2bedd9..0b0b3334896 100644
--- a/test/LTO/X86/symver-asm.ll
+++ b/test/LTO/X86/symver-asm.ll
@@ -1,16 +1,47 @@
 ; RUN: llvm-as < %s >%t1
-; RUN: llvm-lto -o %t2 %t1
+; RUN: llvm-lto -exported-symbol=io_cancel_0_4 -exported-symbol=io_cancel_weak_0_4 -exported-symbol=foo -o %t2 %t1
 ; RUN: llvm-nm %t2 | FileCheck %s
+; RUN: llvm-lto2 -r %t1,io_cancel_0_4,plx -r %t1,io_cancel_0_4,plx -r %t1,io_cancel_local_0_4,plx -r %t1,io_cancel_weak_0_4,plx -r %t1,io_cancel_weak_0_4,plx -r %t1,io_cancel@@LIBAIO_0.4,plx -r %t1,io_cancel_weak@@LIBAIO_0.4,plx -r %t1,io_cancel_weak@@LIBAIO_0.4.1,plx -r %t1,foo,plx -r %t1,foo,plx -r %t1,foo@@VER1,plx -o %t3 %t1 -save-temps
+; RUN: llvm-nm %t3.0 | FileCheck %s
+; RUN: llvm-dis %t3.0.2.internalize.bc -o - | FileCheck %s --check-prefix=INTERN
 
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 module asm ".symver io_cancel_0_4,io_cancel@@LIBAIO_0.4"
+module asm ".symver io_cancel_local_0_4,io_cancel_local@@LIBAIO_0.4"
+module asm ".symver io_cancel_weak_0_4,io_cancel_weak@@LIBAIO_0.4"
+; Ensure we handle case of same aliasee with two version aliases.
+module asm ".symver io_cancel_weak_0_4,io_cancel_weak@@LIBAIO_0.4.1"
+module asm ".symver foo,foo@@VER1"
+
+; Local values used in inline assembly must be specified on the
+; llvm.compiler.used so they aren't incorrectly DCE'd during module linking.
+@llvm.compiler.used = appending global [1 x i8*] [i8* bitcast (i32 ()* @io_cancel_local_0_4 to i8*)], section "llvm.metadata"
 
-; Even without -exported-symbol, io_cancel_0_4 should be noticed by LTOModule's
-; RecordStreamer, so it shouldn't get eliminated. However, the object file will
-; contain the aliased symver as well as the original.
 define i32 @io_cancel_0_4() {
-; CHECK: io_cancel@@LIBAIO_0.4
-; CHECK: io_cancel_0_4
+; CHECK-DAG: T io_cancel@@LIBAIO_0.4
+; CHECK-DAG: T io_cancel_0_4
+  ret i32 0
+}
+
+define internal i32 @io_cancel_local_0_4() {
+; INTERN: llvm.compiler.used {{.*}} @io_cancel_local_0_4
+; INTERN: define internal i32 @io_cancel_local_0_4()
+; CHECK-DAG: t io_cancel_local@@LIBAIO_0.4
+; CHECK-DAG: t io_cancel_local_0_4
+  ret i32 0
+}
+
+define weak i32 @io_cancel_weak_0_4() {
+; CHECK-DAG: W io_cancel_weak@@LIBAIO_0.4
+; CHECK-DAG: W io_cancel_weak@@LIBAIO_0.4.1
+; CHECK-DAG: W io_cancel_weak_0_4
+ret i32 0
+}
+
+define i32 @"\01foo"() {
+; CHECK-DAG: T foo@@VER1
+; CHECK-DAG: T foo
   ret i32 0
 }
diff --git a/test/LTO/X86/symver-asm2.ll b/test/LTO/X86/symver-asm2.ll
new file mode 100644
index 00000000000..f02f5f7938d
--- /dev/null
+++ b/test/LTO/X86/symver-asm2.ll
@@ -0,0 +1,30 @@
+; Test to ensure symbol binding works correctly for symver directives,
+; when the aliased symbols are defined in inline assembly, including
+; cases when the symbol attributes are provided after the .symver
+; directive.
+
+; RUN: llvm-as < %s >%t1
+; RUN: llvm-lto -o %t2 %t1
+; RUN: llvm-nm %t2 | FileCheck %s
+; RUN: llvm-lto2 -r %t1,_start,plx -r %t1,_start3,plx -r %t1,foo@@SOME_VERSION -r %t1,foo@SOME_VERSION3 -o %t3 %t1 -save-temps
+; RUN: llvm-nm %t3.0 | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+module asm ".global _start"
+module asm "_start:"
+module asm "_start2:"
+module asm "_start3:"
+module asm ".symver _start, foo@@SOME_VERSION"
+module asm ".symver _start2, foo@SOME_VERSION2"
+module asm ".symver _start3, foo@SOME_VERSION3"
+module asm ".local _start2"
+module asm ".weak _start3"
+
+; CHECK-DAG: T _start
+; CHECK-DAG: t _start2
+; CHECK-DAG: W _start3
+; CHECK-DAG: T foo@@SOME_VERSION
+; CHECK-DAG: t foo@SOME_VERSION2
+; CHECK-DAG: W foo@SOME_VERSION3