llvm.org GIT mirror llvm / 860838c
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 ------------------------------------------------------------------------ git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_90@371376 91177308-0d34-0410-b5e6-96231b3b80d8 Hans Wennborg a month ago
3 changed file(s) with 70 addition(s) and 5 deletion(s). Raw diff Collapse all Expand all
39373937 ArrayRef IndirectDests, ArrayRef Args,
39383938 ArrayRef Bundles, const Twine &NameStr);
39393939
3940 /// Should the Indirect Destinations change, scan + update the Arg list.
3941 void updateArgBlockAddresses(unsigned i, BasicBlock *B);
3942
39403943 /// Compute the number of operands to allocate.
39413944 static int ComputeNumOperands(int NumArgs, int NumIndirectDests,
39423945 int NumBundleInputs = 0) {
40744077 return cast(*(&Op<-1>() - getNumIndirectDests() - 1));
40754078 }
40764079 BasicBlock *getIndirectDest(unsigned i) const {
4077 return cast(*(&Op<-1>() - getNumIndirectDests() + i));
4080 return cast_or_null(*(&Op<-1>() - getNumIndirectDests() + i));
40784081 }
40794082 SmallVector getIndirectDests() const {
40804083 SmallVector IndirectDests;
40864089 *(&Op<-1>() - getNumIndirectDests() - 1) = reinterpret_cast(B);
40874090 }
40884091 void setIndirectDest(unsigned i, BasicBlock *B) {
4092 updateArgBlockAddresses(i, B);
40894093 *(&Op<-1>() - getNumIndirectDests() + i) = reinterpret_cast(B);
40904094 }
40914095
40954099 return i == 0 ? getDefaultDest() : getIndirectDest(i - 1);
40964100 }
40974101
4098 void setSuccessor(unsigned idx, BasicBlock *NewSucc) {
4099 assert(idx < getNumIndirectDests() + 1 &&
4102 void setSuccessor(unsigned i, BasicBlock *NewSucc) {
4103 assert(i < getNumIndirectDests() + 1 &&
41004104 "Successor # out of range for callbr!");
4101 *(&Op<-1>() - getNumIndirectDests() -1 + idx) =
4102 reinterpret_cast(NewSucc);
4105 return i == 0 ? setDefaultDest(NewSucc) : setIndirectDest(i - 1, NewSucc);
41034106 }
41044107
41054108 unsigned getNumSuccessors() const { return getNumIndirectDests() + 1; }
819819 assert(It + 2 + IndirectDests.size() == op_end() && "Should add up!");
820820
821821 setName(NameStr);
822 }
823
824 void CallBrInst::updateArgBlockAddresses(unsigned i, BasicBlock *B) {
825 assert(getNumIndirectDests() > i && "IndirectDest # out of range for callbr");
826 if (BasicBlock *OldBB = getIndirectDest(i)) {
827 BlockAddress *Old = BlockAddress::get(OldBB);
828 BlockAddress *New = BlockAddress::get(B);
829 for (unsigned ArgNo = 0, e = getNumArgOperands(); ArgNo != e; ++ArgNo)
830 if (dyn_cast(getArgOperand(ArgNo)) == Old)
831 setArgOperand(ArgNo, New);
832 }
822833 }
823834
824835 CallBrInst::CallBrInst(const CallBrInst &CBI)
10601060 FNeg->deleteValue();
10611061 }
10621062
1063 TEST(InstructionsTest, CallBrInstruction) {
1064 LLVMContext Context;
1065 std::unique_ptr M = parseIR(Context, R"(
1066 define void @foo() {
1067 entry:
1068 callbr void asm sideeffect "// XXX: ${0:l}", "X"(i8* blockaddress(@foo, %branch_test.exit))
1069 to label %land.rhs.i [label %branch_test.exit]
1070
1071 land.rhs.i:
1072 br label %branch_test.exit
1073
1074 branch_test.exit:
1075 %0 = phi i1 [ true, %entry ], [ false, %land.rhs.i ]
1076 br i1 %0, label %if.end, label %if.then
1077
1078 if.then:
1079 ret void
1080
1081 if.end:
1082 ret void
1083 }
1084 )");
1085 Function *Foo = M->getFunction("foo");
1086 auto BBs = Foo->getBasicBlockList().begin();
1087 CallBrInst &CBI = cast(BBs->front());
1088 ++BBs;
1089 ++BBs;
1090 BasicBlock &BranchTestExit = *BBs;
1091 ++BBs;
1092 BasicBlock &IfThen = *BBs;
1093
1094 // Test that setting the first indirect destination of callbr updates the dest
1095 EXPECT_EQ(&BranchTestExit, CBI.getIndirectDest(0));
1096 CBI.setIndirectDest(0, &IfThen);
1097 EXPECT_EQ(&IfThen, CBI.getIndirectDest(0));
1098
1099 // Further, test that changing the indirect destination updates the arg
1100 // operand to use the block address of the new indirect destination basic
1101 // block. This is a critical invariant of CallBrInst.
1102 BlockAddress *IndirectBA = BlockAddress::get(CBI.getIndirectDest(0));
1103 BlockAddress *ArgBA = cast(CBI.getArgOperand(0));
1104 EXPECT_EQ(IndirectBA, ArgBA)
1105 << "After setting the indirect destination, callbr had an indirect "
1106 "destination of '"
1107 << CBI.getIndirectDest(0)->getName() << "', but a argument of '"
1108 << ArgBA->getBasicBlock()->getName() << "'. These should always match:\n"
1109 << CBI;
1110 EXPECT_EQ(IndirectBA->getBasicBlock(), &IfThen);
1111 EXPECT_EQ(ArgBA->getBasicBlock(), &IfThen);
1112 }
1113
10631114 } // end anonymous namespace
10641115 } // end namespace llvm