LowerTypeTests: Propagate symver directives

[vlad.tsyrklevich]

Summary

This change fixes https://crbug.com/834474, a build failure caused by
LowerTypeTests not preserving .symver symbol versioning directives for
exported functions. Emit symver information to ThinLTO summary data and
then propagate symver directives for exported functions to the merged
module.

Emitting symver information to the summaries increases the size of
intermediate build artifacts for a Chromium build by less than 0.2%.

[vlad.tsyrklevich]

lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

361 ↗

(On Diff #143042)

@pcc This should probably be F->hasAddressTaken() instead of F->use_empty(), but I seem to recall that that would be actually be to restrictive for which functions we currently export--is that right?

[pcc]

LGTM

I don't think this will do the right thing if two modules have different .symver targets, but then again that would seem to be problematic even without CFI (consider the case where the modules with different targets get imported into one another).

Probably something to consider in the future would be to rename functions at the IR level in the ThinLTOBitcodeWriter according to the .symver directives.

lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

361 ↗

(On Diff #143042)

I think so, in the importer we would end up redirecting all references (including direct calls) to go through the jump table, so we need to make sure that the jump table points to the right function.

[vlad.tsyrklevich]

I started thinking about renaming but it's complicated by the fact that there are different usages of the symver directive that slightly change the renaming logic. For now I'm going to keep it as is but I've also submitted https://reviews.llvm.org/D45845 to make sure that builds fail correctly if multiple mismatched symvers are emitted.

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

public:
   /// 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 Module &M,
       function_ref<void(StringRef, object::BasicSymbolRef::Flags)> AsmSymbol);
+
+  /// Parse inline ASM and collect the symvers directives that are defined in
+  /// the current module.
+  ///
+  /// For each found symbol, call \p AsmSymver with the name of the symbol and
+  /// its alias.
+  static void
+  CollectAsmSymvers(const Module &M,
+                    function_ref<void(StringRef, StringRef)> AsmSymver);
 };
 
 } // end namespace llvm
 
 #endif // LLVM_OBJECT_MODULESYMBOLTABLE_H

llvm/trunk/lib/Object/ModuleSymbolTable.cpp

void ModuleSymbolTable::addModule(Module *M) {
   for (GlobalValue &GV : M->global_values())
     SymTab.push_back(&GV);
 
   CollectAsmSymbols(*M, [this](StringRef Name, BasicSymbolRef::Flags Flags) {
     SymTab.push_back(new (AsmSymbols.Allocate()) AsmSymbol(Name, Flags));
   });
 }
 
-void ModuleSymbolTable::CollectAsmSymbols(
-    const Module &M,
-    function_ref<void(StringRef, BasicSymbolRef::Flags)> AsmSymbol) {
+static void
+initializeRecordStreamer(const Module &M,
+                         function_ref<void(RecordStreamer &)> Init) {
   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<MCTargetAsmParser> TAP(
       T->createMCAsmParser(*STI, *Parser, *MCII, MCOptions));
   if (!TAP)
     return;
 
   Parser->setTargetParser(*TAP);
   if (Parser->Run(false))
     return;
 
+  Init(Streamer);
+}
+
+void ModuleSymbolTable::CollectAsmSymbols(
+    const Module &M,
+    function_ref<void(StringRef, BasicSymbolRef::Flags)> AsmSymbol) {
+  initializeRecordStreamer(M, [&](RecordStreamer &Streamer) {
     Streamer.flushSymverDirectives();
 
     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");
       case RecordStreamer::DefinedGlobal:
         Res |= BasicSymbolRef::SF_Global;
         break;
       case RecordStreamer::Defined:
         break;
       case RecordStreamer::Global:
       case RecordStreamer::Used:
         Res |= BasicSymbolRef::SF_Undefined;
         Res |= BasicSymbolRef::SF_Global;
         break;
       case RecordStreamer::DefinedWeak:
         Res |= BasicSymbolRef::SF_Weak;
         Res |= BasicSymbolRef::SF_Global;
         break;
       case RecordStreamer::UndefinedWeak:
         Res |= BasicSymbolRef::SF_Weak;
         Res |= BasicSymbolRef::SF_Undefined;
       }
       AsmSymbol(Key, BasicSymbolRef::Flags(Res));
     }
+  });
+}
+
+void ModuleSymbolTable::CollectAsmSymvers(
+    const Module &M, function_ref<void(StringRef, StringRef)> AsmSymver) {
+  initializeRecordStreamer(M, [&](RecordStreamer &Streamer) {
+    for (auto &KV : Streamer.symverAliases())
+      for (auto &Alias : KV.second)
+        AsmSymver(KV.first->getName(), Alias);
+  });
 }
 
 void ModuleSymbolTable::printSymbolName(raw_ostream &OS, Symbol S) const {
   if (S.is<AsmSymbol *>()) {
     OS << S.get<AsmSymbol *>()->first;
     return;
   }
 

llvm/trunk/lib/Object/RecordStreamer.h

private:
   void markDefined(const MCSymbol &Symbol);
   void markGlobal(const MCSymbol &Symbol, MCSymbolAttr Attribute);
   void markUsed(const MCSymbol &Symbol);
   void visitUsedSymbol(const MCSymbol &Sym) override;
 
 public:
   RecordStreamer(MCContext &Context, const Module &M);
 
-  using const_iterator = StringMap<State>::const_iterator;
-
-  const_iterator begin();
-  const_iterator end();
   void EmitInstruction(const MCInst &Inst, const MCSubtargetInfo &STI,
                        bool) 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(StringRef AliasName,
                               const MCSymbol *Aliasee) override;
+
   // Emit ELF .symver aliases and ensure they have the same binding as the
   // defined symbol they alias with.
   void flushSymverDirectives();
+
+  // Symbols iterators
+  using const_iterator = StringMap<State>::const_iterator;
+  const_iterator begin();
+  const_iterator end();
+
+  // SymverAliasMap iterators
+  using const_symver_iterator = decltype(SymverAliasMap)::const_iterator;
+  iterator_range<const_symver_iterator> symverAliases();
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_OBJECT_RECORDSTREAMER_H

llvm/trunk/lib/Object/RecordStreamer.cpp

RecordStreamer::State RecordStreamer::getSymbolState(const MCSymbol *Sym) {
   return SI->second;
 }
 
 void RecordStreamer::emitELFSymverDirective(StringRef AliasName,
                                             const MCSymbol *Aliasee) {
   SymverAliasMap[Aliasee].push_back(AliasName);
 }
 
+iterator_range<RecordStreamer::const_symver_iterator>
+RecordStreamer::symverAliases() {
+  return {SymverAliasMap.begin(), SymverAliasMap.end()};
+}
+
 void RecordStreamer::flushSymverDirectives() {
   // Mapping from mangled name to GV.
   StringMap<const GlobalValue *> MangledNameMap;
   // 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;
   for (const GlobalValue &GV : M.global_values()) {

llvm/trunk/lib/Transforms/IPO/LowerTypeTests.cpp

if (NamedMDNode *AliasesMD = M.getNamedMetadata("aliases")) {
           F->eraseFromParent();
         } else {
           Alias->setName(AliasName);
         }
       }
     }
   }
 
+  // Emit .symver directives for exported functions, if they exist.
+  if (ExportSummary) {
+    if (NamedMDNode *SymversMD = M.getNamedMetadata("symvers")) {
+      for (auto Symver : SymversMD->operands()) {
+        assert(Symver->getNumOperands() >= 2);
+        StringRef SymbolName =
+            cast<MDString>(Symver->getOperand(0))->getString();
+        StringRef Alias = cast<MDString>(Symver->getOperand(1))->getString();
+
+        if (!ExportedFunctions.count(SymbolName))
+          continue;
+
+        M.appendModuleInlineAsm(
+            (llvm::Twine(".symver ") + SymbolName + ", " + Alias).str());
+      }
+    }
+  }
+
   return true;
 }
 
 PreservedAnalyses LowerTypeTestsPass::run(Module &M,
                                           ModuleAnalysisManager &AM) {
   bool Changed = LowerTypeTestsModule(M, /*ExportSummary=*/nullptr,
                                       /*ImportSummary=*/nullptr)
                      .lower();
   if (!Changed)
     return PreservedAnalyses::all();
   return PreservedAnalyses::none();
 }

llvm/trunk/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

 #include "llvm/Analysis/ProfileSummaryInfo.h"
 #include "llvm/Analysis/TypeMetadataUtils.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/PassManager.h"
+#include "llvm/Object/ModuleSymbolTable.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/IPO.h"
 #include "llvm/Transforms/IPO/FunctionAttrs.h"
 #include "llvm/Transforms/IPO/FunctionImport.h"
 #include "llvm/Transforms/Utils/Cloning.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
if (const auto *C = GV->getComdat())
       if (MergedMComdats.count(C))
         return false;
     return true;
   });
 
   promoteInternals(*MergedM, M, ModuleId, CfiFunctions);
   promoteInternals(M, *MergedM, ModuleId, CfiFunctions);
 
+  auto &Ctx = MergedM->getContext();
   SmallVector<MDNode *, 8> CfiFunctionMDs;
   for (auto V : CfiFunctions) {
     Function &F = *cast<Function>(V);
     SmallVector<MDNode *, 2> Types;
     F.getMetadata(LLVMContext::MD_type, Types);
 
-    auto &Ctx = MergedM->getContext();
     SmallVector<Metadata *, 4> Elts;
     Elts.push_back(MDString::get(Ctx, F.getName()));
     CfiFunctionLinkage Linkage;
     if (!F.isDeclarationForLinker())
       Linkage = CFL_Definition;
     else if (F.isWeakForLinker())
       Linkage = CFL_WeakDeclaration;
     else
void splitAndWriteThinLTOBitcode(
   }
 
   SmallVector<MDNode *, 8> FunctionAliases;
   for (auto &A : M.aliases()) {
     if (!isa<Function>(A.getAliasee()))
       continue;
 
     auto *F = cast<Function>(A.getAliasee());
-    auto &Ctx = MergedM->getContext();
     SmallVector<Metadata *, 4> Elts;
 
     Elts.push_back(MDString::get(Ctx, A.getName()));
     Elts.push_back(MDString::get(Ctx, F->getName()));
     Elts.push_back(ConstantAsMetadata::get(
         llvm::ConstantInt::get(Type::getInt8Ty(Ctx), A.getVisibility())));
     Elts.push_back(ConstantAsMetadata::get(
         llvm::ConstantInt::get(Type::getInt8Ty(Ctx), A.isWeakForLinker())));
 
     FunctionAliases.push_back(MDTuple::get(Ctx, Elts));
   }
 
   if (!FunctionAliases.empty()) {
     NamedMDNode *NMD = MergedM->getOrInsertNamedMetadata("aliases");
     for (auto MD : FunctionAliases)
       NMD->addOperand(MD);
   }
 
+  SmallVector<MDNode *, 8> Symvers;
+  ModuleSymbolTable::CollectAsmSymvers(M, [&](StringRef Name, StringRef Alias) {
+    Function *F = M.getFunction(Name);
+    if (!F || F->use_empty())
+      return;
+
+    SmallVector<Metadata *, 2> Elts;
+    Elts.push_back(MDString::get(Ctx, Name));
+    Elts.push_back(MDString::get(Ctx, Alias));
+
+    Symvers.push_back(MDTuple::get(Ctx, Elts));
+  });
+
+  if (!Symvers.empty()) {
+    NamedMDNode *NMD = MergedM->getOrInsertNamedMetadata("symvers");
+    for (auto MD : Symvers)
+      NMD->addOperand(MD);
+  }
+
   simplifyExternals(*MergedM);
 
   // FIXME: Try to re-use BSI and PFI from the original module here.
   ProfileSummaryInfo PSI(M);
   ModuleSummaryIndex Index = buildModuleSummaryIndex(M, nullptr, &PSI);
 
   // Mark the merged module as requiring full LTO. We still want an index for
   // it though, so that it can participate in summary-based dead stripping.

llvm/trunk/test/Transforms/LowerTypeTests/export-symver.ll

+; RUN: opt -S %s -lowertypetests -lowertypetests-summary-action=export -lowertypetests-read-summary=%S/Inputs/use-typeid1-typeid2.yaml | FileCheck %s
+;
+; CHECK: module asm ".symver exported_and_symver, alias1"
+; CHECK-NOT: .symver exported
+; CHECK-NOT: .symver symver
+
+target triple = "x86_64-unknown-linux"
+
+!cfi.functions = !{!0, !1}
+!symvers = !{!3, !4}
+
+!0 = !{!"exported_and_symver", i8 2, !2}
+!1 = !{!"exported", i8 2, !2}
+!2 = !{i64 0, !"typeid1"}
+!3 = !{!"exported_and_symver", !"alias1"}
+!4 = !{!"symver", !"alias2"}

llvm/trunk/test/Transforms/ThinLTOBitcodeWriter/symver.ll

+; RUN: opt -thinlto-bc -o %t %s
+; RUN: llvm-modextract -n 1 -o - %t | llvm-dis | FileCheck %s
+
+target triple = "x86_64-unknown-linux-gnu"
+
+module asm ".symver used, used@VER"
+module asm ".symver unused, unused@VER"
+module asm ".symver variable, variable@VER"
+
+declare !type !0 void @used()
+declare !type !0 void @unused()
+@variable = global i32 0
+
+define i32* @use() {
+  call void @used()
+  ret i32* @variable
+}
+
+; CHECK: !symvers = !{![[SYMVER:[0-9]+]]}
+; CHECK: ![[SYMVER]] = !{!"used", !"used@VER"}
+
+!0 = !{i64 0, !"_ZTSFvvE"}