From a364825f51384b1a5a532e4759551219549bd6bd Mon Sep 17 00:00:00 2001 From: Mehdi Amini Date: Wed, 14 Sep 2016 22:29:59 +0000 Subject: [PATCH] Fix auto-upgrade of TBAA tags in Bitcode Reader If TBAA is on an intrinsic and it gets upgraded, it'll delete the call instruction that we collected in a vector. Even if we were to use WeakVH, it'll drop the TBAA and we'll hit the assert on the upgrade path. r263673 gave a shot to make sure the TBAA upgrade happens before intrinsics upgrade, but failed to account for all cases. Instead of collecting instructions in a vector, this patch makes it just upgrade the TBAA on the fly, because metadata are always already loaded at this point. Differential Revision: https://reviews.llvm.org/D24533 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@281549 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/AutoUpgrade.h | 7 +++--- lib/AsmParser/LLParser.cpp | 9 +++++-- lib/Bitcode/Reader/BitcodeReader.cpp | 13 +++------- lib/IR/AutoUpgrade.cpp | 32 ++++++++++++------------- test/LTO/X86/Inputs/remangle_intrinsics_tbaa.ll | 8 +++++++ test/LTO/X86/remangle_intrinsics_tbaa.ll | 23 ++++++++++++++++++ tools/llvm-link/llvm-link.cpp | 15 ++++++++---- 7 files changed, 71 insertions(+), 36 deletions(-) create mode 100644 test/LTO/X86/Inputs/remangle_intrinsics_tbaa.ll create mode 100644 test/LTO/X86/remangle_intrinsics_tbaa.ll diff --git a/include/llvm/IR/AutoUpgrade.h b/include/llvm/IR/AutoUpgrade.h index 9eb358682c6..b42a3d3ad95 100644 --- a/include/llvm/IR/AutoUpgrade.h +++ b/include/llvm/IR/AutoUpgrade.h @@ -51,9 +51,10 @@ namespace llvm { /// module is modified. bool UpgradeModuleFlags(Module &M); - /// If the TBAA tag for the given instruction uses the scalar TBAA format, - /// we upgrade it to the struct-path aware TBAA format. - void UpgradeInstWithTBAATag(Instruction *I); + /// If the given TBAA tag uses the scalar TBAA format, create a new node + /// corresponding to the upgrade to the struct-path aware TBAA format. + /// Otherwise return the \p TBAANode itself. + MDNode *UpgradeTBAANode(MDNode &TBAANode); /// This is an auto-upgrade for bitcast between pointers with different /// address spaces: the instruction is replaced by a pair ptrtoint+inttoptr. diff --git a/lib/AsmParser/LLParser.cpp b/lib/AsmParser/LLParser.cpp index 15327a1d065..0e154b26578 100644 --- a/lib/AsmParser/LLParser.cpp +++ b/lib/AsmParser/LLParser.cpp @@ -222,8 +222,13 @@ bool LLParser::ValidateEndOfModule() { N.second->resolveCycles(); } - for (unsigned I = 0, E = InstsWithTBAATag.size(); I < E; I++) - UpgradeInstWithTBAATag(InstsWithTBAATag[I]); + for (auto *Inst : InstsWithTBAATag) { + MDNode *MD = Inst->getMetadata(LLVMContext::MD_tbaa); + assert(MD && "UpgradeInstWithTBAATag should have a TBAA tag"); + auto *UpgradedMD = UpgradeTBAANode(*MD); + if (MD != UpgradedMD) + Inst->setMetadata(LLVMContext::MD_tbaa, UpgradedMD); + } // Look for intrinsic functions and CallInst that need to be upgraded for (Module::iterator FI = M->begin(), FE = M->end(); FI != FE; ) diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index 2504ce78cc6..33f1a7e454e 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -257,8 +257,6 @@ class BitcodeReader : public GVMaterializer { std::vector > FunctionPrologues; std::vector > FunctionPersonalityFns; - SmallVector InstsWithTBAATag; - bool HasSeenOldLoopTags = false; /// The set of attributes by index. Index zero in the file is for null, and @@ -4425,11 +4423,11 @@ std::error_code BitcodeReader::parseMetadataAttachment(Function &F) { if (HasSeenOldLoopTags && I->second == LLVMContext::MD_loop) MD = upgradeInstructionLoopAttachment(*MD); - Inst->setMetadata(I->second, MD); if (I->second == LLVMContext::MD_tbaa) { - InstsWithTBAATag.push_back(Inst); - continue; + assert(!MD->isTemporary() && "should load MDs before attachments"); + MD = UpgradeTBAANode(*MD); } + Inst->setMetadata(I->second, MD); } break; } @@ -5842,11 +5840,6 @@ std::error_code BitcodeReader::materializeModule() { if (!BasicBlockFwdRefs.empty()) return error("Never resolved function from blockaddress"); - // Upgrading intrinsic calls before TBAA can cause TBAA metadata to be lost, - // to prevent this instructions with TBAA tags should be upgraded first. - for (unsigned I = 0, E = InstsWithTBAATag.size(); I < E; I++) - UpgradeInstWithTBAATag(InstsWithTBAATag[I]); - // Upgrade any intrinsic calls that slipped through (should not happen!) and // delete the old functions to clean up. We can't do this unless the entire // module is materialized because there could always be another function body diff --git a/lib/IR/AutoUpgrade.cpp b/lib/IR/AutoUpgrade.cpp index 96c34c0ad8c..a68247be973 100644 --- a/lib/IR/AutoUpgrade.cpp +++ b/lib/IR/AutoUpgrade.cpp @@ -1488,28 +1488,26 @@ void llvm::UpgradeCallsToIntrinsic(Function *F) { } } -void llvm::UpgradeInstWithTBAATag(Instruction *I) { - MDNode *MD = I->getMetadata(LLVMContext::MD_tbaa); - assert(MD && "UpgradeInstWithTBAATag should have a TBAA tag"); +MDNode *llvm::UpgradeTBAANode(MDNode &MD) { // Check if the tag uses struct-path aware TBAA format. - if (isa(MD->getOperand(0)) && MD->getNumOperands() >= 3) - return; + if (isa(MD.getOperand(0)) && MD.getNumOperands() >= 3) + return &MD; - if (MD->getNumOperands() == 3) { - Metadata *Elts[] = {MD->getOperand(0), MD->getOperand(1)}; - MDNode *ScalarType = MDNode::get(I->getContext(), Elts); + auto &Context = MD.getContext(); + if (MD.getNumOperands() == 3) { + Metadata *Elts[] = {MD.getOperand(0), MD.getOperand(1)}; + MDNode *ScalarType = MDNode::get(Context, Elts); // Create a MDNode Metadata *Elts2[] = {ScalarType, ScalarType, - ConstantAsMetadata::get(Constant::getNullValue( - Type::getInt64Ty(I->getContext()))), - MD->getOperand(2)}; - I->setMetadata(LLVMContext::MD_tbaa, MDNode::get(I->getContext(), Elts2)); - } else { - // Create a MDNode - Metadata *Elts[] = {MD, MD, ConstantAsMetadata::get(Constant::getNullValue( - Type::getInt64Ty(I->getContext())))}; - I->setMetadata(LLVMContext::MD_tbaa, MDNode::get(I->getContext(), Elts)); + ConstantAsMetadata::get( + Constant::getNullValue(Type::getInt64Ty(Context))), + MD.getOperand(2)}; + return MDNode::get(Context, Elts2); } + // Create a MDNode + Metadata *Elts[] = {&MD, &MD, ConstantAsMetadata::get(Constant::getNullValue( + Type::getInt64Ty(Context)))}; + return MDNode::get(Context, Elts); } Instruction *llvm::UpgradeBitCastInst(unsigned Opc, Value *V, Type *DestTy, diff --git a/test/LTO/X86/Inputs/remangle_intrinsics_tbaa.ll b/test/LTO/X86/Inputs/remangle_intrinsics_tbaa.ll new file mode 100644 index 00000000000..b216ed04cef --- /dev/null +++ b/test/LTO/X86/Inputs/remangle_intrinsics_tbaa.ll @@ -0,0 +1,8 @@ +target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-apple-macosx10.11.0" + +%some_named_struct = type { i8, i8 } + +define void @bar(%some_named_struct*) { + ret void +} diff --git a/test/LTO/X86/remangle_intrinsics_tbaa.ll b/test/LTO/X86/remangle_intrinsics_tbaa.ll new file mode 100644 index 00000000000..189674b5b06 --- /dev/null +++ b/test/LTO/X86/remangle_intrinsics_tbaa.ll @@ -0,0 +1,23 @@ +; RUN: llvm-as %s -o %t1.bc +; RUN: llvm-as %p/Inputs/remangle_intrinsics_tbaa.ll -o %t2.bc +; RUN: llvm-link -disable-lazy-loading %t2.bc %t1.bc -S | FileCheck %s + +; Verify that we correctly rename the intrinsic and don't crash +; CHECK: @llvm.masked.store.v4p0some_named_struct.0.p0v4p0some_named_struct.0 + +target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-apple-macosx10.11.0" + +%some_named_struct = type { i8 } + +define void @foo(%some_named_struct*) { + call void @llvm.masked.store.v4p0some_named_struct.p0v4p0some_named_struct(<4 x %some_named_struct*> undef, <4 x %some_named_struct*>* undef, i32 8, <4 x i1> undef), !tbaa !5 + ret void +} + +declare void @llvm.masked.store.v4p0some_named_struct.p0v4p0some_named_struct(<4 x %some_named_struct*>, <4 x %some_named_struct*>*, i32, <4 x i1>) #1 + +!5 = !{!6, !6, i64 0} +!6 = !{!"any pointer", !7, i64 0} +!7 = !{!"omnipotent char", !8, i64 0} +!8 = !{!"Simple C/C++ TBAA"} diff --git a/tools/llvm-link/llvm-link.cpp b/tools/llvm-link/llvm-link.cpp index 185ae2a82a1..80749605bd1 100644 --- a/tools/llvm-link/llvm-link.cpp +++ b/tools/llvm-link/llvm-link.cpp @@ -82,8 +82,11 @@ static cl::opt Force("f", cl::desc("Enable binary output on terminals")); static cl::opt -OutputAssembly("S", - cl::desc("Write output as LLVM assembly"), cl::Hidden); + DisableLazyLoad("disable-lazy-loading", + cl::desc("Enable binary output on terminals")); + +static cl::opt + OutputAssembly("S", cl::desc("Write output as LLVM assembly"), cl::Hidden); static cl::opt Verbose("v", cl::desc("Print information about actions taken")); @@ -114,8 +117,12 @@ static std::unique_ptr loadFile(const char *argv0, bool MaterializeMetadata = true) { SMDiagnostic Err; if (Verbose) errs() << "Loading '" << FN << "'\n"; - std::unique_ptr Result = - getLazyIRFileModule(FN, Err, Context, !MaterializeMetadata); + std::unique_ptr Result; + if (DisableLazyLoad) + Result = parseIRFile(FN, Err, Context); + else + Result = getLazyIRFileModule(FN, Err, Context, !MaterializeMetadata); + if (!Result) { Err.print(argv0, errs()); return nullptr; -- 2.11.0