From b35cc691ea93d9408f76736170359fd7ce604b53 Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Wed, 20 Apr 2016 14:39:45 +0000 Subject: [PATCH] [ThinLTO] Prevent importing of "llvm.used" values Summary: This patch prevents importing from (and therefore exporting from) any module with a "llvm.used" local value. Local values need to be promoted and renamed when importing, and their presense on the llvm.used variable indicates that there are opaque uses that won't see the rename. One such example is a use in inline assembly. See also the discussion at: http://lists.llvm.org/pipermail/llvm-dev/2016-April/098047.html As part of this, move collectUsedGlobalVariables out of Transforms/Utils and into IR/Module so that it can be used more widely. There are several other places in LLVM that used copies of this code that can be cleaned up as a follow on NFC patch. Reviewers: joker.eph Subscribers: pcc, llvm-commits, joker.eph Differential Revision: http://reviews.llvm.org/D18986 llvm-svn: 266877 --- llvm/include/llvm/IR/Module.h | 7 ++++++ .../llvm/Transforms/Utils/ModuleUtils.h | 7 ------ llvm/lib/Analysis/ModuleSummaryAnalysis.cpp | 25 +++++++++++++++++++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp | 3 +++ llvm/lib/IR/Module.cpp | 16 ++++++++++++ llvm/lib/Transforms/IPO/GlobalOpt.cpp | 1 - llvm/lib/Transforms/IPO/Internalize.cpp | 1 - .../Transforms/Utils/FunctionImportUtils.cpp | 20 +++++++++++++++ llvm/lib/Transforms/Utils/ModuleUtils.cpp | 17 ------------- .../FunctionImport/Inputs/inlineasm.ll | 11 ++++++++ .../Transforms/FunctionImport/inlineasm.ll | 19 ++++++++++++++ llvm/tools/gold/gold-plugin.cpp | 1 - 12 files changed, 101 insertions(+), 27 deletions(-) create mode 100644 llvm/test/Transforms/FunctionImport/Inputs/inlineasm.ll create mode 100644 llvm/test/Transforms/FunctionImport/inlineasm.ll diff --git a/llvm/include/llvm/IR/Module.h b/llvm/include/llvm/IR/Module.h index 05e704978ae5..0a6a127bb0dc 100644 --- a/llvm/include/llvm/IR/Module.h +++ b/llvm/include/llvm/IR/Module.h @@ -35,6 +35,7 @@ class GVMaterializer; class LLVMContext; class RandomNumberGenerator; class StructType; +template class SmallPtrSetImpl; template<> struct ilist_traits : public ilist_default_traits { @@ -748,6 +749,12 @@ public: /// @} }; +/// \brief Given "llvm.used" or "llvm.compiler.used" as a global name, collect +/// the initializer elements of that global in Set and return the global itself. +GlobalVariable *collectUsedGlobalVariables(const Module &M, + SmallPtrSetImpl &Set, + bool CompilerUsed); + /// An raw_ostream inserter for modules. inline raw_ostream &operator<<(raw_ostream &O, const Module &M) { M.print(O, nullptr); diff --git a/llvm/include/llvm/Transforms/Utils/ModuleUtils.h b/llvm/include/llvm/Transforms/Utils/ModuleUtils.h index 4781bd623b77..2eb2b1363b0b 100644 --- a/llvm/include/llvm/Transforms/Utils/ModuleUtils.h +++ b/llvm/include/llvm/Transforms/Utils/ModuleUtils.h @@ -28,7 +28,6 @@ class Constant; class StringRef; class Value; class Type; -template class SmallPtrSetImpl; /// Append F to the list of global ctors of module M with the given Priority. /// This wraps the function in the appropriate structure and stores it along @@ -41,12 +40,6 @@ void appendToGlobalCtors(Module &M, Function *F, int Priority, void appendToGlobalDtors(Module &M, Function *F, int Priority, Constant *Data = nullptr); -/// \brief Given "llvm.used" or "llvm.compiler.used" as a global name, collect -/// the initializer elements of that global in Set and return the global itself. -GlobalVariable *collectUsedGlobalVariables(Module &M, - SmallPtrSetImpl &Set, - bool CompilerUsed); - // Validate the result of Module::getOrInsertFunction called for an interface // function of given sanitizer. If the instrumented module defines a function // with the same name, their prototypes must match, otherwise diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp index 5c8b0aab193a..ebc17214e2ac 100644 --- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp +++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp @@ -120,6 +120,31 @@ ModuleSummaryIndexBuilder::ModuleSummaryIndexBuilder( const Module *M, std::function Ftor) : Index(llvm::make_unique()), M(M) { + // We cannot currently promote or rename anything that is in llvm.used, + // since any such value may have a use that won't see the new name. + // Specifically, any uses within inline assembly are not visible to the + // compiler. Prevent importing of any modules containing these uses by + // suppressing generation of the index. This also prevents importing + // into this module, which is also necessary to avoid needing to rename + // in case of a name clash between a local in this module and an imported + // global. + // FIXME: If we find we need a finer-grained approach of preventing promotion + // and renaming of just the functions using inline assembly we will need to: + // - Add flag in the function summaries to identify those with inline asm. + // - Prevent importing of any functions with flag set. + // - Prevent importing of any global function with the same name as a + // function in current module that has the flag set. + // - For any llvm.used value that is exported and promoted, add a private + // alias to the original name in the current module (even if we don't + // export the function using those values in inline asm, another function + // with a reference could be exported). + SmallPtrSet Used; + collectUsedGlobalVariables(*M, Used, /*CompilerUsed*/ false); + for (GlobalValue *V : Used) { + if (V->hasLocalLinkage()) + return; + } + // Compute summaries for all functions defined in module, and save in the // index. for (auto &F : *M) { diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp index d7eda00f5796..6e8d6e80bd71 100644 --- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -2889,6 +2889,9 @@ static void WritePerModuleGlobalValueSummary(const Module *M, if (M->empty()) return; + if (Index.begin() == Index.end()) + return; + Stream.EnterSubblock(bitc::GLOBALVAL_SUMMARY_BLOCK_ID, 3); // Abbrev for FS_PERMODULE. diff --git a/llvm/lib/IR/Module.cpp b/llvm/lib/IR/Module.cpp index 64528d2815a1..101103678419 100644 --- a/llvm/lib/IR/Module.cpp +++ b/llvm/lib/IR/Module.cpp @@ -14,6 +14,7 @@ #include "llvm/IR/Module.h" #include "SymbolTableListTraitsImpl.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringExtras.h" #include "llvm/IR/Constants.h" @@ -515,3 +516,18 @@ void Module::setProfileSummary(Metadata *M) { Metadata *Module::getProfileSummary() { return getModuleFlag("ProfileSummary"); } + +GlobalVariable *llvm::collectUsedGlobalVariables( + const Module &M, SmallPtrSetImpl &Set, bool CompilerUsed) { + const char *Name = CompilerUsed ? "llvm.compiler.used" : "llvm.used"; + GlobalVariable *GV = M.getGlobalVariable(Name); + if (!GV || !GV->hasInitializer()) + return GV; + + const ConstantArray *Init = cast(GV->getInitializer()); + for (Value *Op : Init->operands()) { + GlobalValue *G = cast(Op->stripPointerCastsNoFollowAliases()); + Set.insert(G); + } + return GV; +} diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp index 7c400d62307d..60d1e7ccbe6c 100644 --- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp +++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp @@ -43,7 +43,6 @@ #include "llvm/Transforms/Utils/CtorUtils.h" #include "llvm/Transforms/Utils/Evaluator.h" #include "llvm/Transforms/Utils/GlobalStatus.h" -#include "llvm/Transforms/Utils/ModuleUtils.h" #include using namespace llvm; diff --git a/llvm/lib/Transforms/IPO/Internalize.cpp b/llvm/lib/Transforms/IPO/Internalize.cpp index c01a5e1c8abd..ca868e9d224b 100644 --- a/llvm/lib/Transforms/IPO/Internalize.cpp +++ b/llvm/lib/Transforms/IPO/Internalize.cpp @@ -31,7 +31,6 @@ #include "llvm/Support/Debug.h" #include "llvm/Support/raw_ostream.h" #include "llvm/Transforms/Utils/GlobalStatus.h" -#include "llvm/Transforms/Utils/ModuleUtils.h" #include #include using namespace llvm; diff --git a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp index c0628b3bd49c..f1d12bce06c9 100644 --- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp +++ b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp @@ -206,6 +206,26 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) { } void FunctionImportGlobalProcessing::processGlobalsForThinLTO() { + // We cannot currently promote or rename anything that is in llvm.used, + // since any such value may have a use that won't see the new name. + // Specifically, any uses within inline assembly are not visible to the + // compiler. Prevent changing any such values on the exporting side, + // since we would already have guarded against an import from this module by + // suppressing its index generation. See comments on what is required + // in order to implement a finer grained solution in + // ModuleSummaryIndexBuilder::ModuleSummaryIndexBuilder(). + SmallPtrSet Used; + collectUsedGlobalVariables(M, Used, /*CompilerUsed*/ false); + for (GlobalValue *V : Used) { + if (!V->hasLocalLinkage()) + continue; + // We would have blocked importing from this module by suppressing index + // generation. + assert(!isPerformingImport() && + "Should have blocked importing from module with local used"); + return; + } + for (GlobalVariable &GV : M.globals()) processGlobalForThinLTO(GV); for (Function &SF : M) diff --git a/llvm/lib/Transforms/Utils/ModuleUtils.cpp b/llvm/lib/Transforms/Utils/ModuleUtils.cpp index 4d73a6bf0375..eb9188518624 100644 --- a/llvm/lib/Transforms/Utils/ModuleUtils.cpp +++ b/llvm/lib/Transforms/Utils/ModuleUtils.cpp @@ -12,7 +12,6 @@ //===----------------------------------------------------------------------===// #include "llvm/Transforms/Utils/ModuleUtils.h" -#include "llvm/ADT/SmallPtrSet.h" #include "llvm/IR/DerivedTypes.h" #include "llvm/IR/Function.h" #include "llvm/IR/IRBuilder.h" @@ -90,22 +89,6 @@ void llvm::appendToGlobalDtors(Module &M, Function *F, int Priority, Constant *D appendToGlobalArray("llvm.global_dtors", M, F, Priority, Data); } -GlobalVariable * -llvm::collectUsedGlobalVariables(Module &M, SmallPtrSetImpl &Set, - bool CompilerUsed) { - const char *Name = CompilerUsed ? "llvm.compiler.used" : "llvm.used"; - GlobalVariable *GV = M.getGlobalVariable(Name); - if (!GV || !GV->hasInitializer()) - return GV; - - const ConstantArray *Init = cast(GV->getInitializer()); - for (Value *Op : Init->operands()) { - GlobalValue *G = cast(Op->stripPointerCastsNoFollowAliases()); - Set.insert(G); - } - return GV; -} - Function *llvm::checkSanitizerInterfaceFunction(Constant *FuncOrBitcast) { if (isa(FuncOrBitcast)) return cast(FuncOrBitcast); diff --git a/llvm/test/Transforms/FunctionImport/Inputs/inlineasm.ll b/llvm/test/Transforms/FunctionImport/Inputs/inlineasm.ll new file mode 100644 index 000000000000..1ffc5db5f8b0 --- /dev/null +++ b/llvm/test/Transforms/FunctionImport/Inputs/inlineasm.ll @@ -0,0 +1,11 @@ +@myvar = internal constant i8 1, align 1 +@llvm.used = appending global [1 x i8*] [i8* @myvar], section "llvm.metadata" + +define void @foo(i64* %v) #0 { +entry: + %v.addr = alloca i64*, align 8 + store i64* %v, i64** %v.addr, align 8 + %0 = load i64*, i64** %v.addr, align 8 + call void asm sideeffect "movzbl myvar(%rip), %eax\0A\09movq %rax, $0\0A\09", "=*m,~{eax},~{dirflag},~{fpsr},~{flags}"(i64* %0) #1 + ret void +} diff --git a/llvm/test/Transforms/FunctionImport/inlineasm.ll b/llvm/test/Transforms/FunctionImport/inlineasm.ll new file mode 100644 index 000000000000..d0516f146534 --- /dev/null +++ b/llvm/test/Transforms/FunctionImport/inlineasm.ll @@ -0,0 +1,19 @@ +; Do setup work for all below tests: generate bitcode and combined index +; RUN: opt -module-summary %s -o %t.bc +; RUN: opt -module-summary %p/Inputs/inlineasm.ll -o %t2.bc +; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc + +; Attempt the import now, ensure below that file containing inline assembly +; is not imported from. Otherwise we would need to promote its local variable +; used in the inline assembly, which would not see the rename. +; RUN: opt -function-import -summary-file %t3.thinlto.bc %t.bc -S 2>&1 | FileCheck %s --check-prefix=CHECK + +define i32 @main() #0 { +entry: + %f = alloca i64, align 8 + call void @foo(i64* %f) + ret i32 0 +} + +; CHECK: declare void @foo(i64*) +declare void @foo(i64*) #1 diff --git a/llvm/tools/gold/gold-plugin.cpp b/llvm/tools/gold/gold-plugin.cpp index 1c78d2fc7457..42e93b1c58ff 100644 --- a/llvm/tools/gold/gold-plugin.cpp +++ b/llvm/tools/gold/gold-plugin.cpp @@ -44,7 +44,6 @@ #include "llvm/Transforms/IPO/PassManagerBuilder.h" #include "llvm/Transforms/Utils/FunctionImportUtils.h" #include "llvm/Transforms/Utils/GlobalStatus.h" -#include "llvm/Transforms/Utils/ModuleUtils.h" #include "llvm/Transforms/Utils/ValueMapper.h" #include #include