[llvm-branch-commits] [llvm-branch] r371376 - Merging r371262:
Hans Wennborg via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Sep 9 01:59:27 PDT 2019
Author: hans
Date: Mon Sep 9 01:59:27 2019
New Revision: 371376
URL: http://llvm.org/viewvc/llvm-project?rev=371376&view=rev
Log:
Merging r371262:
------------------------------------------------------------------------
r371262 | nickdesaulniers | 2019-09-06 23:50:11 +0200 (Fri, 06 Sep 2019) | 45 lines
[IR] CallBrInst: scan+update arg list when indirect dest list changes
Summary:
There's an unspoken invariant of callbr that the list of BlockAddress
Constants in the "function args" list match the BasicBlocks in the
"other labels" list. (This invariant is being added to the LangRef in
https://reviews.llvm.org/D67196).
When modifying the any of the indirect destinations of a callbr
instruction (possible jump targets), we need to update the function
arguments if the argument is a BlockAddress whose BasicBlock refers to
the indirect destination BasicBlock being replaced. Otherwise, many
transforms that modify successors will end up violating that invariant.
A recent change to the arm64 Linux kernel exposed this bug, which
prevents the kernel from booting.
I considered maintaining a mapping from indirect destination BasicBlock
to argument operand BlockAddress, but this ends up being a one to
potentially many (though usually one) mapping. Also, the list of
arguments to a function (or more typically inline assembly) ends up
being less than 10. The implementation is significantly simpler to just
rescan the full list of arguments. Because of the one to potentially
many relationship, the full arg list must be scanned (we can't stop at
the first instance).
Thanks to the following folks that reported the issue and helped debug
it:
* Nathan Chancellor
* Will Deacon
* Andrew Murray
* Craig Topper
Link: https://bugs.llvm.org/show_bug.cgi?id=43222
Link: https://github.com/ClangBuiltLinux/linux/issues/649
Link: https://lists.infradead.org/pipermail/linux-arm-kernel/2019-September/678330.html
Reviewers: craig.topper, chandlerc
Reviewed By: craig.topper
Subscribers: void, javed.absar, kristof.beyls, hiraditya, llvm-commits, nathanchance, srhines
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D67252
------------------------------------------------------------------------
Modified:
llvm/branches/release_90/ (props changed)
llvm/branches/release_90/include/llvm/IR/Instructions.h
llvm/branches/release_90/lib/IR/Instructions.cpp
llvm/branches/release_90/unittests/IR/InstructionsTest.cpp
Propchange: llvm/branches/release_90/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Sep 9 01:59:27 2019
@@ -1,3 +1,3 @@
/llvm/branches/Apple/Pertwee:110850,110961
/llvm/branches/type-system-rewrite:133420-134817
-/llvm/trunk:155241,366431,366447,366481,366487,366527,366570,366660,366868,366925,367019,367030,367062,367084,367124,367215,367292,367304,367306,367314,367340-367341,367394,367396,367398,367403,367412,367417,367429,367580,367662,367750,367753,367846-367847,367898,367941,368004,368164,368230,368300,368315,368324,368477-368478,368517-368519,368554,368572,368873,369011,369026,369084,369095,369097,369168,369199,369310,369426,369443,369886,370036,370176,370204,370271,370355,370404,370426,370430,370720-370721,370753,371048,371088,371095
+/llvm/trunk:155241,366431,366447,366481,366487,366527,366570,366660,366868,366925,367019,367030,367062,367084,367124,367215,367292,367304,367306,367314,367340-367341,367394,367396,367398,367403,367412,367417,367429,367580,367662,367750,367753,367846-367847,367898,367941,368004,368164,368230,368300,368315,368324,368477-368478,368517-368519,368554,368572,368873,369011,369026,369084,369095,369097,369168,369199,369310,369426,369443,369886,370036,370176,370204,370271,370355,370404,370426,370430,370720-370721,370753,371048,371088,371095,371262
Modified: llvm/branches/release_90/include/llvm/IR/Instructions.h
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_90/include/llvm/IR/Instructions.h?rev=371376&r1=371375&r2=371376&view=diff
==============================================================================
--- llvm/branches/release_90/include/llvm/IR/Instructions.h (original)
+++ llvm/branches/release_90/include/llvm/IR/Instructions.h Mon Sep 9 01:59:27 2019
@@ -3938,6 +3938,9 @@ class CallBrInst : public CallBase {
ArrayRef<BasicBlock *> IndirectDests, ArrayRef<Value *> Args,
ArrayRef<OperandBundleDef> Bundles, const Twine &NameStr);
+ /// Should the Indirect Destinations change, scan + update the Arg list.
+ void updateArgBlockAddresses(unsigned i, BasicBlock *B);
+
/// Compute the number of operands to allocate.
static int ComputeNumOperands(int NumArgs, int NumIndirectDests,
int NumBundleInputs = 0) {
@@ -4075,7 +4078,7 @@ public:
return cast<BasicBlock>(*(&Op<-1>() - getNumIndirectDests() - 1));
}
BasicBlock *getIndirectDest(unsigned i) const {
- return cast<BasicBlock>(*(&Op<-1>() - getNumIndirectDests() + i));
+ return cast_or_null<BasicBlock>(*(&Op<-1>() - getNumIndirectDests() + i));
}
SmallVector<BasicBlock *, 16> getIndirectDests() const {
SmallVector<BasicBlock *, 16> IndirectDests;
@@ -4087,6 +4090,7 @@ public:
*(&Op<-1>() - getNumIndirectDests() - 1) = reinterpret_cast<Value *>(B);
}
void setIndirectDest(unsigned i, BasicBlock *B) {
+ updateArgBlockAddresses(i, B);
*(&Op<-1>() - getNumIndirectDests() + i) = reinterpret_cast<Value *>(B);
}
@@ -4096,11 +4100,10 @@ public:
return i == 0 ? getDefaultDest() : getIndirectDest(i - 1);
}
- void setSuccessor(unsigned idx, BasicBlock *NewSucc) {
- assert(idx < getNumIndirectDests() + 1 &&
+ void setSuccessor(unsigned i, BasicBlock *NewSucc) {
+ assert(i < getNumIndirectDests() + 1 &&
"Successor # out of range for callbr!");
- *(&Op<-1>() - getNumIndirectDests() -1 + idx) =
- reinterpret_cast<Value *>(NewSucc);
+ return i == 0 ? setDefaultDest(NewSucc) : setIndirectDest(i - 1, NewSucc);
}
unsigned getNumSuccessors() const { return getNumIndirectDests() + 1; }
Modified: llvm/branches/release_90/lib/IR/Instructions.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_90/lib/IR/Instructions.cpp?rev=371376&r1=371375&r2=371376&view=diff
==============================================================================
--- llvm/branches/release_90/lib/IR/Instructions.cpp (original)
+++ llvm/branches/release_90/lib/IR/Instructions.cpp Mon Sep 9 01:59:27 2019
@@ -822,6 +822,17 @@ void CallBrInst::init(FunctionType *FTy,
setName(NameStr);
}
+void CallBrInst::updateArgBlockAddresses(unsigned i, BasicBlock *B) {
+ assert(getNumIndirectDests() > i && "IndirectDest # out of range for callbr");
+ if (BasicBlock *OldBB = getIndirectDest(i)) {
+ BlockAddress *Old = BlockAddress::get(OldBB);
+ BlockAddress *New = BlockAddress::get(B);
+ for (unsigned ArgNo = 0, e = getNumArgOperands(); ArgNo != e; ++ArgNo)
+ if (dyn_cast<BlockAddress>(getArgOperand(ArgNo)) == Old)
+ setArgOperand(ArgNo, New);
+ }
+}
+
CallBrInst::CallBrInst(const CallBrInst &CBI)
: CallBase(CBI.Attrs, CBI.FTy, CBI.getType(), Instruction::CallBr,
OperandTraits<CallBase>::op_end(this) - CBI.getNumOperands(),
Modified: llvm/branches/release_90/unittests/IR/InstructionsTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_90/unittests/IR/InstructionsTest.cpp?rev=371376&r1=371375&r2=371376&view=diff
==============================================================================
--- llvm/branches/release_90/unittests/IR/InstructionsTest.cpp (original)
+++ llvm/branches/release_90/unittests/IR/InstructionsTest.cpp Mon Sep 9 01:59:27 2019
@@ -1061,5 +1061,56 @@ TEST(InstructionsTest, FNegInstruction)
FNeg->deleteValue();
}
+TEST(InstructionsTest, CallBrInstruction) {
+ LLVMContext Context;
+ std::unique_ptr<Module> M = parseIR(Context, R"(
+define void @foo() {
+entry:
+ callbr void asm sideeffect "// XXX: ${0:l}", "X"(i8* blockaddress(@foo, %branch_test.exit))
+ to label %land.rhs.i [label %branch_test.exit]
+
+land.rhs.i:
+ br label %branch_test.exit
+
+branch_test.exit:
+ %0 = phi i1 [ true, %entry ], [ false, %land.rhs.i ]
+ br i1 %0, label %if.end, label %if.then
+
+if.then:
+ ret void
+
+if.end:
+ ret void
+}
+)");
+ Function *Foo = M->getFunction("foo");
+ auto BBs = Foo->getBasicBlockList().begin();
+ CallBrInst &CBI = cast<CallBrInst>(BBs->front());
+ ++BBs;
+ ++BBs;
+ BasicBlock &BranchTestExit = *BBs;
+ ++BBs;
+ BasicBlock &IfThen = *BBs;
+
+ // Test that setting the first indirect destination of callbr updates the dest
+ EXPECT_EQ(&BranchTestExit, CBI.getIndirectDest(0));
+ CBI.setIndirectDest(0, &IfThen);
+ EXPECT_EQ(&IfThen, CBI.getIndirectDest(0));
+
+ // Further, test that changing the indirect destination updates the arg
+ // operand to use the block address of the new indirect destination basic
+ // block. This is a critical invariant of CallBrInst.
+ BlockAddress *IndirectBA = BlockAddress::get(CBI.getIndirectDest(0));
+ BlockAddress *ArgBA = cast<BlockAddress>(CBI.getArgOperand(0));
+ EXPECT_EQ(IndirectBA, ArgBA)
+ << "After setting the indirect destination, callbr had an indirect "
+ "destination of '"
+ << CBI.getIndirectDest(0)->getName() << "', but a argument of '"
+ << ArgBA->getBasicBlock()->getName() << "'. These should always match:\n"
+ << CBI;
+ EXPECT_EQ(IndirectBA->getBasicBlock(), &IfThen);
+ EXPECT_EQ(ArgBA->getBasicBlock(), &IfThen);
+}
+
} // end anonymous namespace
} // end namespace llvm
More information about the llvm-branch-commits
mailing list