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

llvm-svn: 297332

llvm/include/llvm/MC/MCStreamer.h

@@ -489,6 +489,14 @@ class MCStreamer {
   ///  .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) {}

llvm/include/llvm/Object/ModuleSymbolTable.h

@@ -26,6 +26,7 @@
 namespace llvm {
 
 class GlobalValue;
+class RecordStreamer;
 
 class ModuleSymbolTable {
 public:
@@ -52,7 +53,7 @@ class ModuleSymbolTable {
   /// 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);
 };
 

llvm/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))

llvm/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);

llvm/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;
 }
 

llvm/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,

llvm/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;

llvm/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);
+}

llvm/lib/Object/RecordStreamer.h

@@ -20,6 +20,10 @@ class RecordStreamer : public MCStreamer {
 
 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 @@ class RecordStreamer : public MCStreamer {
                     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

llvm/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);

llvm/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
 }

llvm/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