Perform symbol binding for .symver versioned symbols

[tejohnson]

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.

[pcc]

lib/MC/MCParser/ELFAsmParser.cpp

748 ↗

(On Diff #90115)

I don't think this is right. See ELFObjectWriter::executePostLayoutBinding: .symver symbols inherit their binding from the symbol they alias.

I think we may need something equivalent to the executePostLayoutBinding logic in ModuleSymbolTable::CollectAsmSymbols.

[tejohnson]

lib/MC/MCParser/ELFAsmParser.cpp

748 ↗

(On Diff #90115)

Thanks for pointing me to that, I was having trouble finding good documentation on the .symver directive, but I see that if I make the aliased symbol local that gas makes the versioned symbol also local.

However, I'm not sure I agree that the handling in executePostLayoutBinding, which deals with .symver specifically, should be added to CollectAsmSymbols, which is doing more general handling of the symbols returned from the streamer. It seems like we should do this in the .symver handling here within the ELFAsmParser, and instead set the appropriate symbol attribute based on the symbol binding of the aliased Sym (instead of unconditionally setting to global as I have done here).

[pcc]

lib/MC/MCParser/ELFAsmParser.cpp

748 ↗

(On Diff #90115)

The problem is that we cannot resolve the binding until we have seen the entire input because a later directive may change the binding. For example:

foo:
.symver foo, bar@@baz

.weak foo

In this case bar@@baz has weak binding but that is not known until the end of the input.

It may be that we can handle this within the parser but I don't think it would be right to do it entirely at the point where we see the directive.

[tejohnson]

lib/MC/MCParser/ELFAsmParser.cpp

748 ↗

(On Diff #90115)

Good point.

Sigh, my test case expanded to include the local and weak cases hit https://bugs.llvm.org//show_bug.cgi?id=25679 for the local case. I think as Rafael said there we should relax this assertion, although I am not sure what the right conditions for relaxing this are - we no longer have the local aliased symbol defined anywhere so can't check that its binding was local.

For the original bug I am fixing here, getting this right unfortunately seems tricky - the binding of the aliased symbol could be set in the module asm via a directive, or as is probably more likely, in the IR, e.g.:

module asm ".symver io_cancel_local_0_4,io_cancel_local@@LIBAIO_0.4"

define internal i32 @io_cancel_local_0_4() {

ret i32 0

}

In that case, we don't have the information in the asm parser about the aliased symbol's binding. We don't even currently have this in CollectAsmSymbols, although I suppose we could pass in the Module. But it seems complex to get this right - we need to identify these .symver aliases, and get the binding of the aliased symbol, either from the asm or from the IR. Not sure of a cleaner way to do this right...

Ultimately the emitted object has the right bindings, because the binding is set on the alias in ELFObjectWriter::executePostLayoutBinding. But the issue is ensuring the symbol iterator in the InputFile skips the right symbols.

[pcc]

lib/MC/MCParser/ELFAsmParser.cpp

748 ↗

(On Diff #90115)

Yes, this is tricky. I think we will have to change ModuleSymbolTable to keep track of the set of defined symbol names (and as part of that use the aliased symbol to compute the binding of the alias). I think that is something we'll need to do anyway to deduplicate symbols when we create the bitcode symbol table.

If we also keep track of which symbols are aliased with .symver we can inhibit internalization for those symbols, which would seem to fix pr25679.

[tejohnson]

lib/MC/MCParser/ELFAsmParser.cpp

748 ↗

(On Diff #90115)

It's also tricky because CollectAsmSymbols is target agnostic, and the symver directive is specific to ELF. Even the symbol binding is only accessible via the ELF symbol type.

After looking at this some more, I think the most straightforward way is to use a callback from the parser that is defined in CollectAsmSymbols, that would a) add the aliased symbol to the llvm.compiler.used set to avoid internalization; and b) return the binding info based on the linkage type (assuming it is defined in the IR).

Then have a virtual method probably on the MCAsmParserExtension that can be invoked at the end of parsing and would be overridden for the ELFAsmParser to do the right thing for symver aliases (which would need to be tracked), and which uses the callback.

[tejohnson]

lib/MC/MCParser/ELFAsmParser.cpp

748 ↗

(On Diff #90115)

I have fixes for these issues but they just need to be cleaned up a bit. I also discovered that the issue with the incorrect internalization was even easier to fix. We just needed to invoke the mechanism that updates the llvm.compiler.used with undefined asm symbols earlier for regular LTO - both in the new API and in the old legacy LTO. I'll clean that up and send it to fix that original bug first. Tomorrow though as I need to finish up something unrelated today...

[tejohnson]

Rework to get symbol binding from aliased symbol.

[pcc]

The parser doesn't seem like the correct place to handle this. If you introduce an EmitELFSymverDirective method on MCStreamer, and extend the AsmSymbol callback to CollectAsmSymbols with something that lets the caller know that this is a symver symbol and the name of the aliasee, I think you can move all handling of this directive into RecordStreamer and the AsmSymbol callback provided by ModuleSymbolTable.

include/llvm/Object/ModuleSymbolTable.h

55 ↗

(On Diff #90707)

The signature change seems like a separately useful cleanup.

lib/Object/ModuleSymbolTable.cpp

112 ↗

(On Diff #90707)

I don't think this is right, as Name may be mangled.

[tejohnson]

In D30485#693665, @pcc wrote:

The parser doesn't seem like the correct place to handle this.

Why? The ELFAsmParser already has .symver handling and is specific to ELF. The benefit of handling there is that all clients get the correct binding. My concern with doing this in the MCStreamer & RecordStreamer is that the RecordStreamer is currently format-agnostic. The MCStreamer has some format specific Emit* methods on it, but they are empty and overridden by the various format-specific object file writers derived via MCObjectStreamer.

If you introduce an EmitELFSymverDirective method on MCStreamer, and extend the AsmSymbol callback to CollectAsmSymbols with something that lets the caller know that this is a symver symbol and the name of the aliasee,

I think what would need to be done is to add this virtual method on the MCStreamer, which by default doesn't do anything, and only implement it for the derived RecordStreamer class. For RecordStreamer, it would need to save the mapping between the alias and aliasee. Then in CollectAsmSymbols, we'd have to walk this saved mapping, and get and apply the same symbol attribute to the alias as applied to its aliasee (either by invoking RecordStreamer::EmitSymbolAttribute, or by applying the same RecordStreamer::State to the aliasee when iterating through the RecordStreamer's Syymbols string map). We also need to get the symbol binding from the IR in the case where the aliasee is defined in IR.

I think you can move all handling of this directive into RecordStreamer and the AsmSymbol callback provided by ModuleSymbolTable.

Do you mean move it into the invocation of CollectAsmSymbols from ModuleSymbolTable::addModule (i.e. into it's definition of AsmSymbol)? This will only benefit that one client of CollectAsmSymbols (although that is the one that matters for this bug).

lib/Object/ModuleSymbolTable.cpp

112 ↗

(On Diff #90707)

Won't the name in IR also be mangled?

[pcc]

In D30485#694812, @tejohnson wrote:

In D30485#693665, @pcc wrote:

The parser doesn't seem like the correct place to handle this.

Why? The ELFAsmParser already has .symver handling and is specific to ELF. The benefit of handling there is that all clients get the correct binding. My concern with doing this in the MCStreamer & RecordStreamer is that the RecordStreamer is currently format-agnostic. The MCStreamer has some format specific Emit* methods on it, but they are empty and overridden by the various format-specific object file writers derived via MCObjectStreamer.

The parser is supposed to be just a parser. The streamer is meant to control what to do with each directive. It is fine if it contains special handling for ELF because the RecordStreamer is supposed to be handling all directives that can introduce a symbol reference or definition in the object file, even those which happen to be specific to one object format (in fact, I suspect that we may need to override methods like BeginCOFFSymbolDef in the future).

If you introduce an EmitELFSymverDirective method on MCStreamer, and extend the AsmSymbol callback to CollectAsmSymbols with something that lets the caller know that this is a symver symbol and the name of the aliasee,

I think what would need to be done is to add this virtual method on the MCStreamer, which by default doesn't do anything, and only implement it for the derived RecordStreamer class. For RecordStreamer, it would need to save the mapping between the alias and aliasee. Then in CollectAsmSymbols, we'd have to walk this saved mapping, and get and apply the same symbol attribute to the alias as applied to its aliasee (either by invoking RecordStreamer::EmitSymbolAttribute, or by applying the same RecordStreamer::State to the aliasee when iterating through the RecordStreamer's Syymbols string map). We also need to get the symbol binding from the IR in the case where the aliasee is defined in IR.

Yes, you could even handle all non-symver symbols first and then handle the symver symbols by walking the partially computed ModuleSymbolTable. (I think it is fine if it uses a linear scan because module asm symbols are uncommon.)

I think you can move all handling of this directive into RecordStreamer and the AsmSymbol callback provided by ModuleSymbolTable.

Do you mean move it into the invocation of CollectAsmSymbols from ModuleSymbolTable::addModule (i.e. into it's definition of AsmSymbol)? This will only benefit that one client of CollectAsmSymbols (although that is the one that matters for this bug).

If a caller of CollectAsmSymbols needs 100% accurate binding information, it will effectively need to create a ModuleSymbolTable. I haven't looked very closely at what the other callers are doing though. It may be that they can get by with inaccurate bindings for symvers.

lib/Object/ModuleSymbolTable.cpp

112 ↗

(On Diff #90707)

In ELF it could be something like @"\01foo". In that case the mangled name is foo.

[tejohnson]

In D30485#694931, @pcc wrote:

In D30485#694812, @tejohnson wrote:

In D30485#693665, @pcc wrote:

The parser doesn't seem like the correct place to handle this.

Why? The ELFAsmParser already has .symver handling and is specific to ELF. The benefit of handling there is that all clients get the correct binding. My concern with doing this in the MCStreamer & RecordStreamer is that the RecordStreamer is currently format-agnostic. The MCStreamer has some format specific Emit* methods on it, but they are empty and overridden by the various format-specific object file writers derived via MCObjectStreamer.

The parser is supposed to be just a parser. The streamer is meant to control what to do with each directive. It is fine if it contains special handling for ELF because the RecordStreamer is supposed to be handling all directives that can introduce a symbol reference or definition in the object file, even those which happen to be specific to one object format (in fact, I suspect that we may need to override methods like BeginCOFFSymbolDef in the future).

Fair enough.

If you introduce an EmitELFSymverDirective method on MCStreamer, and extend the AsmSymbol callback to CollectAsmSymbols with something that lets the caller know that this is a symver symbol and the name of the aliasee,

I think what would need to be done is to add this virtual method on the MCStreamer, which by default doesn't do anything, and only implement it for the derived RecordStreamer class. For RecordStreamer, it would need to save the mapping between the alias and aliasee. Then in CollectAsmSymbols, we'd have to walk this saved mapping, and get and apply the same symbol attribute to the alias as applied to its aliasee (either by invoking RecordStreamer::EmitSymbolAttribute, or by applying the same RecordStreamer::State to the aliasee when iterating through the RecordStreamer's Syymbols string map). We also need to get the symbol binding from the IR in the case where the aliasee is defined in IR.

Yes, you could even handle all non-symver symbols first and then handle the symver symbols by walking the partially computed ModuleSymbolTable. (I think it is fine if it uses a linear scan because module asm symbols are uncommon.)

I changed to handling this in the RecordStreamer and CollectAsmSymbols.

I think you can move all handling of this directive into RecordStreamer and the AsmSymbol callback provided by ModuleSymbolTable.

Do you mean move it into the invocation of CollectAsmSymbols from ModuleSymbolTable::addModule (i.e. into it's definition of AsmSymbol)? This will only benefit that one client of CollectAsmSymbols (although that is the one that matters for this bug).

If a caller of CollectAsmSymbols needs 100% accurate binding information, it will effectively need to create a ModuleSymbolTable. I haven't looked very closely at what the other callers are doing though. It may be that they can get by with inaccurate bindings for symvers.

All the handling is in CollectAsmSymbols, none needed in the AsmSymbol callbacks.

lib/Object/ModuleSymbolTable.cpp

112 ↗

(On Diff #90707)

Ah, wasn't familiar with cases like that. Since I need to go from the mangled name (in asm) to a potentially unmangled name (in IR), I ended up needing to precompute a map. Added a case like this to the test.

[tejohnson]

Address review comments

[pcc]

LGTM with a few nits.

lib/Object/ModuleSymbolTable.cpp

61 ↗

(On Diff #91040)

if (Streamer.symverAliases().empty()) return;

74 ↗

(On Diff #91040)

You can make the code a little simpler by putting all globals in the map.

lib/Object/RecordStreamer.h

26 ↗

(On Diff #91040)

Can just be a vector of (alias, aliasee) pairs.

test/LTO/X86/symver-asm.ll

19 ↗

(On Diff #91040)

You mean so they aren't DCE'd?

[tejohnson]

Incidentally, this is no longer dependent on D30657, since the handling moved out of the ELFAsmParser. Will update this patch to remove the dependence, but that is still a good standalone fix.

lib/Object/ModuleSymbolTable.cpp

61 ↗

(On Diff #91040)

Good idea, will add

74 ↗

(On Diff #91040)

I was trying to reduce the size of the map we create, since in most cases we won't need an entry. I'll go ahead and remove the guard, the map creation shouldn't be common in the first place.

lib/Object/RecordStreamer.h

26 ↗

(On Diff #91040)

I prefer to keep it this way so the processing in handleSymverAliases can stay the same (i.e. only look for the Aliasee in the IR once for all aliases).

test/LTO/X86/symver-asm.ll

19 ↗

(On Diff #91040)

Right, or more specifically, not linked into the combined module. Will change to "DCE'd during module linking".

[tejohnson]

Address review comments

llvm/trunk/include/llvm/MC/MCStreamer.h

public:
   virtual void EmitCOFFSecRel32(MCSymbol const *Symbol, uint64_t Offset);
 
   /// \brief Emit an ELF .size directive.
   ///
   /// This corresponds to an assembler statement such as:
   ///  .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) {}
 
   /// \brief Emit a common symbol.
   ///
   /// \param Symbol - The common symbol to emit.
   /// \param Size - The size of the common symbol.

llvm/trunk/include/llvm/Object/ModuleSymbolTable.h

 #include "llvm/IR/Mangler.h"
 #include "llvm/Object/SymbolicFile.h"
 #include <string>
 #include <utility>
 
 namespace llvm {
 
 class GlobalValue;
+class RecordStreamer;
 
 class ModuleSymbolTable {
 public:
   typedef std::pair<std::string, uint32_t> AsmSymbol;
   typedef PointerUnion<GlobalValue *, AsmSymbol *> Symbol;
 
 private:
   Module *FirstMod = nullptr;
public:
   uint32_t getSymbolFlags(Symbol S) const;
 
   /// Parse inline ASM and collect the symbols that are defined or referenced in
   /// the current module.
   ///
   /// 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);
 };
 
 }
 
 #endif

llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp

if (!M.getModuleInlineAsm().empty()) {
     // to prevent export of any use of them in regular IR that would require
     // renaming within the module level asm. Note we don't need to create a
     // summary for weak or global defs, as they don't need to be flagged as
     // NoRename, and defs in module level asm can't be imported anyway.
     // Also, any values used but not defined within module level asm should
     // 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,
+        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))
             return;
           GlobalValue *GV = M.getNamedValue(Name);
           if (!GV)
             return;
           assert(GV->isDeclaration() && "Def in module asm already has definition");

llvm/trunk/lib/LTO/LTOBackend.cpp

finalizeOptimizationRemarks(std::unique_ptr<tool_output_file> DiagOutputFile) {
   DiagOutputFile->os().flush();
 }
 
 static void handleAsmUndefinedRefs(Module &Mod, TargetMachine &TM) {
   // Collect the list of undefined symbols used in asm and update
   // 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);
       });
   updateCompilerUsed(Mod, TM, AsmUndefinedRefs);
 }
 
 Error lto::backend(Config &C, AddStreamFn AddStream,

llvm/trunk/lib/MC/MCParser/ELFAsmParser.cpp

bool ELFAsmParser::ParseDirectiveSymver(StringRef, SMLoc) {
   if (AliasName.find('@') == StringRef::npos)
     return TokError("expected a '@' in the name");
 
   MCSymbol *Alias = getContext().getOrCreateSymbol(AliasName);
   MCSymbol *Sym = getContext().getOrCreateSymbol(Name);
   const MCExpr *Value = MCSymbolRefExpr::create(Sym, getContext());
 
   getStreamer().EmitAssignment(Alias, Value);
+  getStreamer().emitELFSymverDirective(Alias, Sym);
   return false;
 }
 
 /// ParseDirectiveVersion
 ///  ::= .version string
 bool ELFAsmParser::ParseDirectiveVersion(StringRef, SMLoc) {
   if (getLexer().isNot(AsmToken::String))
     return TokError("unexpected token in '.version' directive");

llvm/trunk/lib/MC/MCStreamer.cpp

 void MCStreamer::EmitFileDirective(StringRef Filename) {}
 void MCStreamer::EmitCOFFSymbolStorageClass(int StorageClass) {
   llvm_unreachable("this directive only supported on COFF targets");
 }
 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,
                                 uint64_t Size, unsigned ByteAlignment) {}
 void MCStreamer::ChangeSection(MCSection *, const MCExpr *) {}
 void MCStreamer::EmitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) {}
 void MCStreamer::EmitBytes(StringRef Data) {}
 void MCStreamer::EmitBinaryData(StringRef Data) { EmitBytes(Data); }

llvm/trunk/lib/Object/ModuleSymbolTable.cpp

void ModuleSymbolTable::addModule(Module *M) {
 
   for (Function &F : *M)
     SymTab.push_back(&F);
   for (GlobalVariable &GV : M->globals())
     SymTab.push_back(&GV);
   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());
 
   std::unique_ptr<MCRegisterInfo> MRI(T->createMCRegInfo(TT.str()));
   if (!MRI)
     return;
 
   std::unique_ptr<MCAsmInfo> MAI(T->createMCAsmInfo(*MRI, TT.str()));
std::unique_ptr<MCTargetAsmParser> TAP(
       T->createMCAsmParser(*STI, *Parser, *MCII, MCOptions));
   if (!TAP)
     return;
 
   Parser->setTargetParser(*TAP);
   if (Parser->Run(false))
     return;
 
+  handleSymverAliases(M, Streamer);
+
   for (auto &KV : Streamer) {
     StringRef Key = KV.first();
     RecordStreamer::State Value = KV.second;
     // FIXME: For now we just assume that all asm symbols are executable.
     uint32_t Res = BasicSymbolRef::SF_Executable;
     switch (Value) {
     case RecordStreamer::NeverSeen:
       llvm_unreachable("NeverSeen should have been replaced earlier");

llvm/trunk/lib/Object/RecordStreamer.h

 namespace llvm {
 class RecordStreamer : public MCStreamer {
 public:
   enum State { NeverSeen, Global, Defined, DefinedGlobal, DefinedWeak, Used,
                UndefinedWeak};
 
 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);
   void visitUsedSymbol(const MCSymbol &Sym) override;
 
 public:
   typedef StringMap<State>::const_iterator const_iterator;
   const_iterator begin();
   const_iterator end();
   RecordStreamer(MCContext &Context);
   void EmitInstruction(const MCInst &Inst, const MCSubtargetInfo &STI) override;
   void EmitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override;
   void EmitAssignment(MCSymbol *Symbol, const MCExpr *Value) override;
   bool EmitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
   void EmitZerofill(MCSection *Section, MCSymbol *Symbol, uint64_t Size,
                     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/trunk/lib/Object/RecordStreamer.cpp

void RecordStreamer::EmitZerofill(MCSection *Section, MCSymbol *Symbol,
                                   uint64_t Size, unsigned ByteAlignment) {
   markDefined(*Symbol);
 }
 
 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/trunk/lib/Transforms/IPO/FunctionImport.cpp

 
 /// Run internalization on \p TheModule based on symmary analysis.
 void llvm::thinLTOInternalizeModule(Module &TheModule,
                                     const GVSummaryMapTy &DefinedGlobals) {
   // Parse inline ASM and collect the list of symbols that are not defined in
   // 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);
       });
 
   // Declare a callback for the internalize pass that will ask for every
   // candidate GlobalValue if it can be internalized or not.
   auto MustPreserveGV = [&](const GlobalValue &GV) -> bool {

llvm/trunk/test/LTO/X86/symver-asm.ll

 ; 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.
[e-mail] = 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/trunk/test/LTO/X86/symver-asm2.ll

+; 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