Skip to content

Commit

Permalink
LowerTypeTests: Propagate symver directives
Browse files Browse the repository at this point in the history
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
  • Loading branch information
vlad902 committed Apr 20, 2018
1 parent a59aacf commit 230b256
Show file tree
Hide file tree
Showing 8 changed files with 150 additions and 38 deletions.
9 changes: 9 additions & 0 deletions llvm/include/llvm/Object/ModuleSymbolTable.h
Expand Up @@ -57,6 +57,15 @@ class ModuleSymbolTable {
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
Expand Down
81 changes: 49 additions & 32 deletions llvm/lib/Object/ModuleSymbolTable.cpp
Expand Up @@ -68,9 +68,9 @@ void ModuleSymbolTable::addModule(Module *M) {
});
}

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;
Expand Down Expand Up @@ -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<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));
}
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 {
Expand Down
5 changes: 5 additions & 0 deletions llvm/lib/Object/RecordStreamer.cpp
Expand Up @@ -128,6 +128,11 @@ void RecordStreamer::emitELFSymverDirective(StringRef AliasName,
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;
Expand Down
14 changes: 10 additions & 4 deletions llvm/lib/Object/RecordStreamer.h
Expand Up @@ -47,10 +47,6 @@ class RecordStreamer : public MCStreamer {
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;
Expand All @@ -63,9 +59,19 @@ class RecordStreamer : public MCStreamer {
/// 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
Expand Down
18 changes: 18 additions & 0 deletions llvm/lib/Transforms/IPO/LowerTypeTests.cpp
Expand Up @@ -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<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;
}

Expand Down
23 changes: 21 additions & 2 deletions llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
Expand Up @@ -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"
Expand Down Expand Up @@ -301,13 +302,13 @@ void splitAndWriteThinLTOBitcode(
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;
Expand Down Expand Up @@ -336,7 +337,6 @@ void splitAndWriteThinLTOBitcode(
continue;

auto *F = cast<Function>(A.getAliasee());
auto &Ctx = MergedM->getContext();
SmallVector<Metadata *, 4> Elts;

Elts.push_back(MDString::get(Ctx, A.getName()));
Expand All @@ -355,6 +355,25 @@ void splitAndWriteThinLTOBitcode(
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.
Expand Down
16 changes: 16 additions & 0 deletions 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"}
22 changes: 22 additions & 0 deletions 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"}

0 comments on commit 230b256

Please sign in to comment.