1
0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2025-01-31 12:41:49 +01:00

[ThinLTO] Drop dso_local if a GlobalVariable satisfies isDeclarationForLinker()

dso_local leads to direct access even if the definition is not within this compilation unit (it is
still in the same linkage unit). On ELF, such a relocation (e.g. R_X86_64_PC32) referencing a
STB_GLOBAL STV_DEFAULT object can cause a linker error in a -shared link.

If the linkage is changed to available_externally, the dso_local flag should be dropped, so that no
direct access will be generated.

The current behavior is benign, because -fpic does not assume dso_local
(clang/lib/CodeGen/CodeGenModule.cpp:shouldAssumeDSOLocal).
If we do that for -fno-semantic-interposition (D73865), there will be an
R_X86_64_PC32 linker error without this patch.

Reviewed By: tejohnson

Differential Revision: https://reviews.llvm.org/D74751
This commit is contained in:
Fangrui Song 2020-02-15 17:23:18 -08:00
parent c3d83ed19e
commit d41ca54775
19 changed files with 156 additions and 50 deletions

View File

@ -105,8 +105,10 @@ public:
std::function<Expected<std::unique_ptr<Module>>(StringRef Identifier)>;
/// Create a Function Importer.
FunctionImporter(const ModuleSummaryIndex &Index, ModuleLoaderTy ModuleLoader)
: Index(Index), ModuleLoader(std::move(ModuleLoader)) {}
FunctionImporter(const ModuleSummaryIndex &Index, ModuleLoaderTy ModuleLoader,
bool ClearDSOLocalOnDeclarations)
: Index(Index), ModuleLoader(std::move(ModuleLoader)),
ClearDSOLocalOnDeclarations(ClearDSOLocalOnDeclarations) {}
/// Import functions in Module \p M based on the supplied import list.
Expected<bool> importFunctions(Module &M, const ImportMapTy &ImportList);
@ -117,6 +119,10 @@ private:
/// Factory function to load a Module for a given identifier
ModuleLoaderTy ModuleLoader;
/// See the comment of ClearDSOLocalOnDeclarations in
/// Utils/FunctionImportUtils.h.
bool ClearDSOLocalOnDeclarations;
};
/// The function importing pass

View File

@ -39,6 +39,19 @@ class FunctionImportGlobalProcessing {
/// as part of a different backend compilation process.
bool HasExportedFunctions = false;
/// Set to true (only applicatable to ELF -fpic) if dso_local should be
/// dropped for a declaration.
///
/// On ELF, the assembler is conservative and assumes a global default
/// visibility symbol can be interposable. No direct access relocation is
/// allowed, if the definition is not in the translation unit, even if the
/// definition is available in the linkage unit. Thus we need to clear
/// dso_local to disable direct access.
///
/// This flag should not be set for -fno-pic or -fpie, which would
/// unnecessarily disable direct access.
bool ClearDSOLocalOnDeclarations;
/// Set of llvm.*used values, in order to validate that we don't try
/// to promote any non-renamable values.
SmallPtrSet<GlobalValue *, 8> Used;
@ -85,10 +98,11 @@ class FunctionImportGlobalProcessing {
GlobalValue::LinkageTypes getLinkage(const GlobalValue *SGV, bool DoPromote);
public:
FunctionImportGlobalProcessing(
Module &M, const ModuleSummaryIndex &Index,
SetVector<GlobalValue *> *GlobalsToImport = nullptr)
: M(M), ImportIndex(Index), GlobalsToImport(GlobalsToImport) {
FunctionImportGlobalProcessing(Module &M, const ModuleSummaryIndex &Index,
SetVector<GlobalValue *> *GlobalsToImport,
bool ClearDSOLocalOnDeclarations)
: M(M), ImportIndex(Index), GlobalsToImport(GlobalsToImport),
ClearDSOLocalOnDeclarations(ClearDSOLocalOnDeclarations) {
// If we have a ModuleSummaryIndex but no function to import,
// then this is the primary module being compiled in a ThinLTO
// backend compilation, and we need to see if it has functions that
@ -111,6 +125,7 @@ public:
/// exported local functions renamed and promoted for ThinLTO.
bool renameModuleForThinLTO(
Module &M, const ModuleSummaryIndex &Index,
bool ClearDSOLocalOnDeclarations,
SetVector<GlobalValue *> *GlobalsToImport = nullptr);
/// Compute synthetic function entry counts.

View File

@ -521,7 +521,13 @@ Error lto::thinBackend(const Config &Conf, unsigned Task, AddStreamFn AddStream,
if (Conf.PreOptModuleHook && !Conf.PreOptModuleHook(Task, Mod))
return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
renameModuleForThinLTO(Mod, CombinedIndex);
// When linking an ELF shared object, dso_local should be dropped. We
// conservatively do this for -fpic.
bool ClearDSOLocalOnDeclarations =
TM->getTargetTriple().isOSBinFormatELF() &&
TM->getRelocationModel() != Reloc::Static &&
Mod.getPIELevel() == PIELevel::Default;
renameModuleForThinLTO(Mod, CombinedIndex, ClearDSOLocalOnDeclarations);
dropDeadSymbols(Mod, DefinedGlobals, CombinedIndex);
@ -547,7 +553,8 @@ Error lto::thinBackend(const Config &Conf, unsigned Task, AddStreamFn AddStream,
/*IsImporting*/ true);
};
FunctionImporter Importer(CombinedIndex, ModuleLoader);
FunctionImporter Importer(CombinedIndex, ModuleLoader,
ClearDSOLocalOnDeclarations);
if (Error Err = Importer.importFunctions(Mod, ImportList).takeError())
return Err;

View File

@ -152,8 +152,9 @@ generateModuleMap(std::vector<std::unique_ptr<lto::InputFile>> &Modules) {
return ModuleMap;
}
static void promoteModule(Module &TheModule, const ModuleSummaryIndex &Index) {
if (renameModuleForThinLTO(TheModule, Index))
static void promoteModule(Module &TheModule, const ModuleSummaryIndex &Index,
bool ClearDSOLocalOnDeclarations) {
if (renameModuleForThinLTO(TheModule, Index, ClearDSOLocalOnDeclarations))
report_fatal_error("renameModuleForThinLTO failed");
}
@ -205,15 +206,16 @@ static std::unique_ptr<Module> loadModuleFromInput(lto::InputFile *Input,
static void
crossImportIntoModule(Module &TheModule, const ModuleSummaryIndex &Index,
StringMap<lto::InputFile*> &ModuleMap,
const FunctionImporter::ImportMapTy &ImportList) {
StringMap<lto::InputFile *> &ModuleMap,
const FunctionImporter::ImportMapTy &ImportList,
bool ClearDSOLocalOnDeclarations) {
auto Loader = [&](StringRef Identifier) {
auto &Input = ModuleMap[Identifier];
return loadModuleFromInput(Input, TheModule.getContext(),
/*Lazy=*/true, /*IsImporting*/ true);
};
FunctionImporter Importer(Index, Loader);
FunctionImporter Importer(Index, Loader, ClearDSOLocalOnDeclarations);
Expected<bool> Result = Importer.importFunctions(TheModule, ImportList);
if (!Result) {
handleAllErrors(Result.takeError(), [&](ErrorInfoBase &EIB) {
@ -411,8 +413,15 @@ ProcessThinLTOModule(Module &TheModule, ModuleSummaryIndex &Index,
// "Benchmark"-like optimization: single-source case
bool SingleModule = (ModuleMap.size() == 1);
// When linking an ELF shared object, dso_local should be dropped. We
// conservatively do this for -fpic.
bool ClearDSOLocalOnDeclarations =
TM.getTargetTriple().isOSBinFormatELF() &&
TM.getRelocationModel() != Reloc::Static &&
TheModule.getPIELevel() == PIELevel::Default;
if (!SingleModule) {
promoteModule(TheModule, Index);
promoteModule(TheModule, Index, ClearDSOLocalOnDeclarations);
// Apply summary-based prevailing-symbol resolution decisions.
thinLTOResolvePrevailingInModule(TheModule, DefinedGlobals);
@ -432,7 +441,8 @@ ProcessThinLTOModule(Module &TheModule, ModuleSummaryIndex &Index,
saveTempBitcode(TheModule, SaveTempsDir, count, ".2.internalized.bc");
if (!SingleModule) {
crossImportIntoModule(TheModule, Index, ModuleMap, ImportList);
crossImportIntoModule(TheModule, Index, ModuleMap, ImportList,
ClearDSOLocalOnDeclarations);
// Save temps: after cross-module import.
saveTempBitcode(TheModule, SaveTempsDir, count, ".3.imported.bc");
@ -673,7 +683,8 @@ void ThinLTOCodeGenerator::promote(Module &TheModule, ModuleSummaryIndex &Index,
Index, IsExported(ExportLists, GUIDPreservedSymbols),
IsPrevailing(PrevailingCopy));
promoteModule(TheModule, Index);
// FIXME Set ClearDSOLocalOnDeclarations.
promoteModule(TheModule, Index, /*ClearDSOLocalOnDeclarations=*/false);
}
/**
@ -705,7 +716,9 @@ void ThinLTOCodeGenerator::crossModuleImport(Module &TheModule,
ExportLists);
auto &ImportList = ImportLists[TheModule.getModuleIdentifier()];
crossImportIntoModule(TheModule, Index, ModuleMap, ImportList);
// FIXME Set ClearDSOLocalOnDeclarations.
crossImportIntoModule(TheModule, Index, ModuleMap, ImportList,
/*ClearDSOLocalOnDeclarations=*/false);
}
/**
@ -832,7 +845,8 @@ void ThinLTOCodeGenerator::internalize(Module &TheModule,
Index, IsExported(ExportLists, GUIDPreservedSymbols),
IsPrevailing(PrevailingCopy));
promoteModule(TheModule, Index);
// FIXME Set ClearDSOLocalOnDeclarations.
promoteModule(TheModule, Index, /*ClearDSOLocalOnDeclarations=*/false);
// Internalization
thinLTOResolvePrevailingInModule(

View File

@ -1233,7 +1233,8 @@ Expected<bool> FunctionImporter::importFunctions(
UpgradeDebugInfo(*SrcModule);
// Link in the specified functions.
if (renameModuleForThinLTO(*SrcModule, Index, &GlobalsToImport))
if (renameModuleForThinLTO(*SrcModule, Index, ClearDSOLocalOnDeclarations,
&GlobalsToImport))
return true;
if (PrintImports) {
@ -1302,7 +1303,8 @@ static bool doImportingForModule(Module &M) {
// Next we need to promote to global scope and rename any local values that
// are potentially exported to other modules.
if (renameModuleForThinLTO(M, *Index, nullptr)) {
if (renameModuleForThinLTO(M, *Index, /*clearDSOOnDeclarations=*/false,
/*GlobalsToImport=*/nullptr)) {
errs() << "Error renaming module\n";
return false;
}
@ -1311,7 +1313,8 @@ static bool doImportingForModule(Module &M) {
auto ModuleLoader = [&M](StringRef Identifier) {
return loadFile(std::string(Identifier), M.getContext());
};
FunctionImporter Importer(*Index, ModuleLoader);
FunctionImporter Importer(*Index, ModuleLoader,
/*ClearDSOLocalOnDeclarations=*/false);
Expected<bool> Result = Importer.importFunctions(M, ImportList);
// FIXME: Probably need to propagate Errors through the pass manager.

View File

@ -212,13 +212,6 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) {
}
}
}
// Check the summaries to see if the symbol gets resolved to a known local
// definition.
if (VI && VI.isDSOLocal()) {
GV.setDSOLocal(true);
if (GV.hasDLLImportStorageClass())
GV.setDLLStorageClass(GlobalValue::DefaultStorageClass);
}
}
// We should always have a ValueInfo (i.e. GV in index) for definitions when
@ -280,6 +273,20 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) {
} else
GV.setLinkage(getLinkage(&GV, /* DoPromote */ false));
// When ClearDSOLocalOnDeclarations is true, clear dso_local if GV is
// converted to a declaration, to disable direct access. Don't do this if GV
// is implicitly dso_local due to a non-default visibility.
if (ClearDSOLocalOnDeclarations && GV.isDeclarationForLinker() &&
!GV.isImplicitDSOLocal()) {
GV.setDSOLocal(false);
} else if (VI && VI.isDSOLocal()) {
// If all summaries are dso_local, symbol gets resolved to a known local
// definition.
GV.setDSOLocal(true);
if (GV.hasDLLImportStorageClass())
GV.setDLLStorageClass(GlobalValue::DefaultStorageClass);
}
// Remove functions imported as available externally defs from comdats,
// as this is a declaration for the linker, and will be dropped eventually.
// It is illegal for comdats to contain declarations.
@ -319,7 +326,9 @@ bool FunctionImportGlobalProcessing::run() {
}
bool llvm::renameModuleForThinLTO(Module &M, const ModuleSummaryIndex &Index,
bool ClearDSOLocalOnDeclarations,
SetVector<GlobalValue *> *GlobalsToImport) {
FunctionImportGlobalProcessing ThinLTOProcessing(M, Index, GlobalsToImport);
FunctionImportGlobalProcessing ThinLTOProcessing(M, Index, GlobalsToImport,
ClearDSOLocalOnDeclarations);
return ThinLTOProcessing.run();
}

View File

@ -13,9 +13,7 @@ target triple = "x86_64-unknown-linux-gnu"
$g = comdat any
@g = global i8 42, comdat, !type !0
; CHECK: define
; CHECK-NOT: dllimport
; CHECK-SAME: @f
; CHECK: define available_externally dllimport i8* @f()
define available_externally dllimport i8* @f() {
ret i8* @g
}

View File

@ -1,5 +1,5 @@
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
@b = global i32* @a, align 8
@a = global i32 42, align 4
@b = dso_local global i32* @a, align 8
@a = dso_local global i32 42, align 4

View File

@ -11,7 +11,7 @@
; foo() being always_inline should be imported irrespective of the
; instruction limit
; CHECK1: define available_externally dso_local void @foo()
; CHECK1: define available_externally void @foo()
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

View File

@ -16,15 +16,15 @@
; RUN: llvm-dis %t5.1.3.import.bc -o - | FileCheck %s --check-prefix=PRESERVED
; We currently don't support importing aliases
; IMPORT: @g.alias = external dso_local global i32
; IMPORT: @g.alias = external global i32
; IMPORT-NEXT: @g = internal global i32 42, align 4 #0
; IMPORT: attributes #0 = { "thinlto-internalize" }
; CODEGEN: define dso_local i32 @main
; CODEGEN-NEXT: ret i32 42
; PRESERVED: @g.alias = external dso_local global i32
; PRESERVED-NEXT: @g = available_externally dso_local global i32 42, align 4
; PRESERVED: @g.alias = external global i32
; PRESERVED-NEXT: @g = available_externally global i32 42, align 4
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

View File

@ -4,7 +4,7 @@
; RUN: llvm-dis %t3.2.3.import.bc -o - | FileCheck %s
; Comdats are not internalized even if they are read only.
; CHECK: @g = available_externally dso_local global i32 42
; CHECK: @g = available_externally global i32 42
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

View File

@ -6,7 +6,7 @@
; Dead globals are converted to declarations by ThinLTO in dropDeadSymbols
; If we try to internalize such we'll get a broken module.
; CHECK: @g = external dso_local global i32
; CHECK: @g = external global i32
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

View File

@ -8,7 +8,7 @@
; All references from functions in full LTO module are not constant.
; We cannot internalize @g
; CHECK: @g = available_externally dso_local global i32 42
; CHECK: @g = available_externally global i32 42
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

View File

@ -0,0 +1,28 @@
;; The same as index-const-prop-gvref.ll, except for PIE.
; RUN: opt -module-summary %s -o %t1.bc
; RUN: opt -module-summary %p/Inputs/index-const-prop-gvref.ll -o %t2.bc
; RUN: llvm-lto2 run -save-temps %t2.bc -r=%t2.bc,b,pl -r=%t2.bc,a,pl \
; RUN: %t1.bc -r=%t1.bc,main,plx -r=%t1.bc,a, -r=%t1.bc,b, -o %t3
; RUN: llvm-dis %t3.2.3.import.bc -o - | FileCheck %s --check-prefix=DEST
;; For PIE, keep dso_local for declarations to enable direct access.
; DEST: @b = external dso_local global i32*
; DEST-NEXT: @a = available_externally dso_local global i32 42, align 4
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
@a = external global i32
@b = external global i32*
define i32 @main() {
%p = load i32*, i32** @b, align 8
store i32 33, i32* %p, align 4
%v = load i32, i32* @a, align 4
ret i32 %v
}
!llvm.module.flags = !{!0}
!0 = !{i32 7, !"PIE Level", i32 2}
!1 = !{i32 7, !"PIC Level", i32 2}

View File

@ -1,17 +1,41 @@
; RUN: opt -module-summary %s -o %t1.bc
; RUN: opt -module-summary %p/Inputs/index-const-prop-gvref.ll -o %t2.bc
; RUN: llvm-lto2 run -save-temps %t2.bc -r=%t2.bc,b,pl -r=%t2.bc,a,pl \
; RUN: %t1.bc -r=%t1.bc,main,plx -r=%t1.bc,a, -r=%t1.bc,b, -o %t3
; RUN: llvm-lto2 run -relocation-model=static -save-temps %t2.bc -r=%t2.bc,b,pl -r=%t2.bc,a,pl \
; RUN: %t1.bc -r=%t1.bc,main,plx -r=%t1.bc,a, -r=%t1.bc,b, -o %t3
; RUN: llvm-dis %t3.1.3.import.bc -o - | FileCheck %s --check-prefix=SRC
; RUN: llvm-dis %t3.2.3.import.bc -o - | FileCheck %s --check-prefix=DEST
;; When producing an ELF DSO, clear dso_local for declarations to avoid direct access.
; RUN: llvm-lto2 run -relocation-model=pic -save-temps %t2.bc -r=%t2.bc,b,pl -r=%t2.bc,a,pl \
; RUN: %t1.bc -r=%t1.bc,main,plx -r=%t1.bc,a, -r=%t1.bc,b, -o %t4
; RUN: llvm-dis %t4.1.3.import.bc -o - | FileCheck %s --check-prefix=SRC
; RUN: llvm-dis %t4.2.3.import.bc -o - | FileCheck %s --check-prefix=DEST_DSO
; No variable in the source module should have been internalized
; SRC: @b = dso_local global i32* @a
; SRC-NEXT: @a = dso_local global i32 42
; We can't internalize globals referenced by other live globals
; DEST: @b = external dso_local global i32*
; DEST-NEXT: @a = available_externally dso_local global i32 42, align 4
; DEST: @b = external dso_local global i32*
; DEST-NEXT: @a = available_externally dso_local global i32 42, align 4
; DEST_DSO: @b = external global i32*
; DEST_DSO-NEXT: @a = available_externally global i32 42, align 4
;; Test old API.
;; When producing an ELF DSO, clear dso_local for declarations to avoid direct access.
; RUN: llvm-lto -thinlto-action=run %t2.bc %t1.bc -relocation-model=static -thinlto-save-temps=%t5.
; RUN: llvm-dis < %t5.0.3.imported.bc | FileCheck %s --check-prefix=OLDAPI_SRC
; RUN: llvm-dis < %t5.1.3.imported.bc | FileCheck %s --check-prefix=OLDAPI_DST
; RUN: llvm-lto -thinlto-action=run %t2.bc %t1.bc -relocation-model=pic -thinlto-save-temps=%t6.
; RUN: llvm-dis < %t6.0.3.imported.bc | FileCheck %s --check-prefix=OLDAPI_SRC
; RUN: llvm-dis < %t6.1.3.imported.bc | FileCheck %s --check-prefix=OLDAPI_DST_DSO
; OLDAPI_SRC: @b = internal global i32* @a, align 8
; OLDAPI_SRC-NEXT: @a = dso_local global i32 42, align 4
; OLDAPI_DST: @b = external dso_local global i32*
; OLDAPI_DST-NEXT: @a = available_externally dso_local global i32 42, align 4
; OLDAPI_DST_DSO: @b = external global i32*
; OLDAPI_DST_DSO-NEXT: @a = available_externally global i32 42, align 4
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

View File

@ -5,7 +5,7 @@
; The 'store' instruction in @main should prevent internalization
; even when there is 'load' instruction before it.
; CHECK: @g = available_externally dso_local global i32 42
; CHECK: @g = available_externally global i32 42
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

View File

@ -11,7 +11,7 @@
; - reference from @llvm.used
; CHECK: @llvm.used = appending global [1 x i32*] [i32* @g2]
; CHECK-NEXT: @g1 = external dso_local global i32, align 4
; CHECK-NEXT: @g2 = available_externally dso_local global i32 42, align 4
; CHECK-NEXT: @g2 = available_externally global i32 42, align 4
; CHECK-NEXT: @g3 = available_externally global i32 42, align 4
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"

View File

@ -68,7 +68,7 @@
; CODEGEN: i32 @main()
; CODEGEN-NEXT: ret i32 3
; IMPORT2: @gBar = available_externally dso_local local_unnamed_addr global i32 2, align 4
; IMPORT2: @gBar = available_externally local_unnamed_addr global i32 2, align 4
; CODEGEN2: i32 @main2
; CODEGEN2-NEXT: %1 = tail call i32 @rand()

View File

@ -266,7 +266,8 @@ static bool importFunctions(const char *argv0, Module &DestModule) {
auto CachedModuleLoader = [&](StringRef Identifier) {
return ModuleLoaderCache.takeModule(std::string(Identifier));
};
FunctionImporter Importer(*Index, CachedModuleLoader);
FunctionImporter Importer(*Index, CachedModuleLoader,
/*ClearDSOLocalOnDeclarations=*/false);
ExitOnErr(Importer.importFunctions(DestModule, ImportList));
return true;
@ -313,7 +314,8 @@ static bool linkFiles(const char *argv0, LLVMContext &Context, Linker &L,
}
// Promotion
if (renameModuleForThinLTO(*M, *Index))
if (renameModuleForThinLTO(*M, *Index,
/*ClearDSOLocalOnDeclarations=*/false))
return true;
}