From 230b256783e567266d9f07787c4fc1c5a667d227 Mon Sep 17 00:00:00 2001 From: Vlad Tsyrklevich Date: Fri, 20 Apr 2018 01:36:48 +0000 Subject: [PATCH] LowerTypeTests: Propagate symver directives 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%. Reviewers: pcc Reviewed By: pcc Subscribers: tejohnson, mehdi_amini, eraman, llvm-commits, eugenis, kcc Differential Revision: https://reviews.llvm.org/D45798 llvm-svn: 330387 --- llvm/include/llvm/Object/ModuleSymbolTable.h | 9 +++ llvm/lib/Object/ModuleSymbolTable.cpp | 81 +++++++++++++--------- llvm/lib/Object/RecordStreamer.cpp | 5 ++ llvm/lib/Object/RecordStreamer.h | 14 ++-- llvm/lib/Transforms/IPO/LowerTypeTests.cpp | 18 +++++ llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp | 23 +++++- .../Transforms/LowerTypeTests/export-symver.ll | 16 +++++ .../test/Transforms/ThinLTOBitcodeWriter/symver.ll | 22 ++++++ 8 files changed, 150 insertions(+), 38 deletions(-) create mode 100644 llvm/test/Transforms/LowerTypeTests/export-symver.ll create mode 100644 llvm/test/Transforms/ThinLTOBitcodeWriter/symver.ll diff --git a/llvm/include/llvm/Object/ModuleSymbolTable.h b/llvm/include/llvm/Object/ModuleSymbolTable.h index 9e93228..c3cbc27 100644 --- a/llvm/include/llvm/Object/ModuleSymbolTable.h +++ b/llvm/include/llvm/Object/ModuleSymbolTable.h @@ -57,6 +57,15 @@ public: static void CollectAsmSymbols( const Module &M, function_ref 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 AsmSymver); }; } // end namespace llvm diff --git a/llvm/lib/Object/ModuleSymbolTable.cpp b/llvm/lib/Object/ModuleSymbolTable.cpp index f0d70ae..b353ef3 100644 --- a/llvm/lib/Object/ModuleSymbolTable.cpp +++ b/llvm/lib/Object/ModuleSymbolTable.cpp @@ -68,9 +68,9 @@ void ModuleSymbolTable::addModule(Module *M) { }); } -void ModuleSymbolTable::CollectAsmSymbols( - const Module &M, - function_ref AsmSymbol) { +static void +initializeRecordStreamer(const Module &M, + function_ref Init) { StringRef InlineAsm = M.getModuleInlineAsm(); if (InlineAsm.empty()) return; @@ -119,36 +119,53 @@ void ModuleSymbolTable::CollectAsmSymbols( if (Parser->Run(false)) return; - 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; + Init(Streamer); +} + +void ModuleSymbolTable::CollectAsmSymbols( + const Module &M, + function_ref 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)); } - AsmSymbol(Key, BasicSymbolRef::Flags(Res)); - } + }); +} + +void ModuleSymbolTable::CollectAsmSymvers( + const Module &M, function_ref 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 { diff --git a/llvm/lib/Object/RecordStreamer.cpp b/llvm/lib/Object/RecordStreamer.cpp index 21577bb..f49c823 100644 --- a/llvm/lib/Object/RecordStreamer.cpp +++ b/llvm/lib/Object/RecordStreamer.cpp @@ -128,6 +128,11 @@ void RecordStreamer::emitELFSymverDirective(StringRef AliasName, SymverAliasMap[Aliasee].push_back(AliasName); } +iterator_range +RecordStreamer::symverAliases() { + return {SymverAliasMap.begin(), SymverAliasMap.end()}; +} + void RecordStreamer::flushSymverDirectives() { // Mapping from mangled name to GV. StringMap MangledNameMap; diff --git a/llvm/lib/Object/RecordStreamer.h b/llvm/lib/Object/RecordStreamer.h index 60b2d3e..13eac02 100644 --- a/llvm/lib/Object/RecordStreamer.h +++ b/llvm/lib/Object/RecordStreamer.h @@ -47,10 +47,6 @@ private: public: RecordStreamer(MCContext &Context, const Module &M); - using const_iterator = StringMap::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; @@ -63,9 +59,19 @@ public: /// 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::const_iterator; + const_iterator begin(); + const_iterator end(); + + // SymverAliasMap iterators + using const_symver_iterator = decltype(SymverAliasMap)::const_iterator; + iterator_range symverAliases(); }; } // end namespace llvm diff --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp index b568445..925d8f8 100644 --- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp +++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp @@ -1947,6 +1947,24 @@ bool LowerTypeTestsModule::lower() { } } + // 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(Symver->getOperand(0))->getString(); + StringRef Alias = cast(Symver->getOperand(1))->getString(); + + if (!ExportedFunctions.count(SymbolName)) + continue; + + M.appendModuleInlineAsm( + (llvm::Twine(".symver ") + SymbolName + ", " + Alias).str()); + } + } + } + return true; } diff --git a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp index 9bca88a..2ba27af 100644 --- a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp +++ b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp @@ -18,6 +18,7 @@ #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" @@ -301,13 +302,13 @@ void splitAndWriteThinLTOBitcode( promoteInternals(*MergedM, M, ModuleId, CfiFunctions); promoteInternals(M, *MergedM, ModuleId, CfiFunctions); + auto &Ctx = MergedM->getContext(); SmallVector CfiFunctionMDs; for (auto V : CfiFunctions) { Function &F = *cast(V); SmallVector Types; F.getMetadata(LLVMContext::MD_type, Types); - auto &Ctx = MergedM->getContext(); SmallVector Elts; Elts.push_back(MDString::get(Ctx, F.getName())); CfiFunctionLinkage Linkage; @@ -336,7 +337,6 @@ void splitAndWriteThinLTOBitcode( continue; auto *F = cast(A.getAliasee()); - auto &Ctx = MergedM->getContext(); SmallVector Elts; Elts.push_back(MDString::get(Ctx, A.getName())); @@ -355,6 +355,25 @@ void splitAndWriteThinLTOBitcode( NMD->addOperand(MD); } + SmallVector Symvers; + ModuleSymbolTable::CollectAsmSymvers(M, [&](StringRef Name, StringRef Alias) { + Function *F = M.getFunction(Name); + if (!F || F->use_empty()) + return; + + SmallVector 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. diff --git a/llvm/test/Transforms/LowerTypeTests/export-symver.ll b/llvm/test/Transforms/LowerTypeTests/export-symver.ll new file mode 100644 index 0000000..e2b58fc --- /dev/null +++ b/llvm/test/Transforms/LowerTypeTests/export-symver.ll @@ -0,0 +1,16 @@ +; 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"} diff --git a/llvm/test/Transforms/ThinLTOBitcodeWriter/symver.ll b/llvm/test/Transforms/ThinLTOBitcodeWriter/symver.ll new file mode 100644 index 0000000..8612f7e --- /dev/null +++ b/llvm/test/Transforms/ThinLTOBitcodeWriter/symver.ll @@ -0,0 +1,22 @@ +; 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"} -- 2.7.4