llvm.org GIT mirror llvm / d773cb7
Fix a bug w/inbounds invalidation in LFTR (recommit) Recommit r363289 with a bug fix for crash identified in pr42279. Issue was that a loop exit test does not have to be an icmp, leading to a null dereference crash when new logic was exercised for that case. Test case previously committed in r363601. Original commit comment follows: This contains fixes for two cases where we might invalidate inbounds and leave it stale in the IR (a miscompile). Case 1 is when switching to an IV with no dynamically live uses, and case 2 is when doing pre-to-post conversion on the same pointer type IV. The basic scheme used is to prove that using the given IV (pre or post increment forms) would have to already trigger UB on the path to the test we're modifying. As such, our potential UB triggering use does not change the semantics of the original program. As was pointed out in the review thread by Nikita, this is defending against a separate issue from the hasConcreteDef case. This is about poison, that's about undef. Unfortunately, the two are different, see Nikita's comment for a fuller explanation, he explains it well. (Note: I'm going to address Nikita's last style comment in a separate commit just to minimize chance of subtle bugs being introduced due to typos.) Differential Revision: https://reviews.llvm.org/D62939 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@363613 91177308-0d34-0410-b5e6-96231b3b80d8 Philip Reames 3 months ago
5 changed file(s) with 140 addition(s) and 64 deletion(s). Raw diff Collapse all Expand all
3030 #include "llvm/ADT/None.h"
3131 #include "llvm/ADT/Optional.h"
3232 #include "llvm/ADT/STLExtras.h"
33 #include "llvm/ADT/SmallSet.h"
3334 #include "llvm/ADT/SmallPtrSet.h"
3435 #include "llvm/ADT/SmallVector.h"
3536 #include "llvm/ADT/Statistic.h"
4142 #include "llvm/Analysis/ScalarEvolutionExpressions.h"
4243 #include "llvm/Analysis/TargetLibraryInfo.h"
4344 #include "llvm/Analysis/TargetTransformInfo.h"
45 #include "llvm/Analysis/ValueTracking.h"
4446 #include "llvm/Transforms/Utils/Local.h"
4547 #include "llvm/IR/BasicBlock.h"
4648 #include "llvm/IR/Constant.h"
20762078 return Phi != getLoopPhiForCounter(IncV, L);
20772079 }
20782080
2081 /// Return true if undefined behavior would provable be executed on the path to
2082 /// OnPathTo if Root produced a posion result. Note that this doesn't say
2083 /// anything about whether OnPathTo is actually executed or whether Root is
2084 /// actually poison. This can be used to assess whether a new use of Root can
2085 /// be added at a location which is control equivalent with OnPathTo (such as
2086 /// immediately before it) without introducing UB which didn't previously
2087 /// exist. Note that a false result conveys no information.
2088 static bool mustExecuteUBIfPoisonOnPathTo(Instruction *Root,
2089 Instruction *OnPathTo,
2090 DominatorTree *DT) {
2091 // Basic approach is to assume Root is poison, propagate poison forward
2092 // through all users we can easily track, and then check whether any of those
2093 // users are provable UB and must execute before out exiting block might
2094 // exit.
2095
2096 // The set of all recursive users we've visited (which are assumed to all be
2097 // poison because of said visit)
2098 SmallSet KnownPoison;
2099 SmallVector Worklist;
2100 Worklist.push_back(Root);
2101 while (!Worklist.empty()) {
2102 const Instruction *I = Worklist.pop_back_val();
2103
2104 // If we know this must trigger UB on a path leading our target.
2105 if (mustTriggerUB(I, KnownPoison) && DT->dominates(I, OnPathTo))
2106 return true;
2107
2108 // If we can't analyze propagation through this instruction, just skip it
2109 // and transitive users. Safe as false is a conservative result.
2110 if (!propagatesFullPoison(I) && I != Root)
2111 continue;
2112
2113 if (KnownPoison.insert(I).second)
2114 for (const User *User : I->users())
2115 Worklist.push_back(cast(User));
2116 }
2117
2118 // Might be non-UB, or might have a path we couldn't prove must execute on
2119 // way to exiting bb.
2120 return false;
2121 }
2122
20792123 /// Recursive helper for hasConcreteDef(). Unfortunately, this currently boils
20802124 /// down to checking that all operands are constant and listing instructions
20812125 /// that may hide undef.
21642208 /// valid count without scaling the address stride, so it remains a pointer
21652209 /// expression as far as SCEV is concerned.
21662210 static PHINode *FindLoopCounter(Loop *L, BasicBlock *ExitingBB,
2167 const SCEV *BECount, ScalarEvolution *SE) {
2211 const SCEV *BECount,
2212 ScalarEvolution *SE, DominatorTree *DT) {
21682213 uint64_t BCWidth = SE->getTypeSizeInBits(BECount->getType());
21692214
21702215 Value *Cond = cast(ExitingBB->getTerminator())->getCondition();
22092254 continue;
22102255 }
22112256
2257 // Avoid introducing undefined behavior due to poison which didn't exist in
2258 // the original program. (Annoyingly, the rules for poison and undef
2259 // propagation are distinct, so this does NOT cover the undef case above.)
2260 // We have to ensure that we don't introduce UB by introducing a use on an
2261 // iteration where said IV produces poison. Our strategy here differs for
2262 // pointers and integer IVs. For integers, we strip and reinfer as needed,
2263 // see code in linearFunctionTestReplace. For pointers, we restrict
2264 // transforms as there is no good way to reinfer inbounds once lost.
2265 if (!Phi->getType()->isIntegerTy() &&
2266 !mustExecuteUBIfPoisonOnPathTo(Phi, ExitingBB->getTerminator(), DT))
2267 continue;
2268
22122269 const SCEV *Init = AR->getStart();
22132270
22142271 if (BestPhi && !AlmostDeadIV(BestPhi, LatchBlock, Cond)) {
23372394 // compare against the post-incremented value, otherwise we must compare
23382395 // against the preincremented value.
23392396 if (ExitingBB == L->getLoopLatch()) {
2340 // Add one to the "backedge-taken" count to get the trip count.
2341 // This addition may overflow, which is valid as long as the comparison is
2342 // truncated to BackedgeTakenCount->getType().
2343 IVCount = SE->getAddExpr(BackedgeTakenCount,
2344 SE->getOne(BackedgeTakenCount->getType()));
2345 // The BackedgeTaken expression contains the number of times that the
2346 // backedge branches to the loop header. This is one less than the
2347 // number of times the loop executes, so use the incremented indvar.
2348 CmpIndVar = IndVar->getIncomingValueForBlock(ExitingBB);
2397 bool SafeToPostInc = IndVar->getType()->isIntegerTy();
2398 if (!SafeToPostInc) {
2399 // For pointer IVs, we chose to not strip inbounds which requires us not
2400 // to add a potentially UB introducing use. We need to either a) show
2401 // the loop test we're modifying is already in post-inc form, or b) show
2402 // that adding a use must not introduce UB.
2403 Instruction *Inc =
2404 cast(IndVar->getIncomingValueForBlock(L->getLoopLatch()));
2405 if (ICmpInst *LoopTest = getLoopTest(L, ExitingBB))
2406 SafeToPostInc = LoopTest->getOperand(0) == Inc ||
2407 LoopTest->getOperand(1) == Inc;
2408 if (!SafeToPostInc)
2409 SafeToPostInc =
2410 mustExecuteUBIfPoisonOnPathTo(Inc, ExitingBB->getTerminator(), DT);
2411 }
2412 if (SafeToPostInc) {
2413 // Add one to the "backedge-taken" count to get the trip count.
2414 // This addition may overflow, which is valid as long as the comparison
2415 // is truncated to BackedgeTakenCount->getType().
2416 IVCount = SE->getAddExpr(BackedgeTakenCount,
2417 SE->getOne(BackedgeTakenCount->getType()));
2418 // The BackedgeTaken expression contains the number of times that the
2419 // backedge branches to the loop header. This is one less than the
2420 // number of times the loop executes, so use the incremented indvar.
2421 CmpIndVar = IndVar->getIncomingValueForBlock(ExitingBB);
2422 }
23492423 }
23502424
23512425 // It may be necessary to drop nowrap flags on the incrementing instruction
26452719 if (BETakenCount->isZero())
26462720 continue;
26472721
2648 PHINode *IndVar = FindLoopCounter(L, ExitingBB, BETakenCount, SE);
2722 PHINode *IndVar = FindLoopCounter(L, ExitingBB, BETakenCount, SE, DT);
26492723 if (!IndVar)
26502724 continue;
26512725
2727 ; CHECK-NEXT: br label [[FOR_BODY21_I:%.*]]
2828 ; CHECK: for.body21.i:
2929 ; CHECK-NEXT: [[DESTYPIXELPTR_010_I:%.*]] = phi i8* [ null, [[FOR_BODY21_LR_PH_I]] ], [ [[INCDEC_PTR_I:%.*]], [[IF_END_I126:%.*]] ]
30 ; CHECK-NEXT: [[X_09_I:%.*]] = phi i32 [ 0, [[FOR_BODY21_LR_PH_I]] ], [ [[INC_I125:%.*]], [[IF_END_I126]] ]
3031 ; CHECK-NEXT: br i1 undef, label [[IF_END_I126]], label [[IF_ELSE_I124:%.*]]
3132 ; CHECK: if.else.i124:
3233 ; CHECK-NEXT: store i8 undef, i8* [[DESTYPIXELPTR_010_I]], align 1
3334 ; CHECK-NEXT: br label [[IF_END_I126]]
3435 ; CHECK: if.end.i126:
3536 ; CHECK-NEXT: [[INCDEC_PTR_I]] = getelementptr inbounds i8, i8* [[DESTYPIXELPTR_010_I]], i32 1
36 ; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i8* [[INCDEC_PTR_I]], null
37 ; CHECK-NEXT: [[INC_I125]] = add nuw i32 [[X_09_I]], 1
38 ; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i32 [[INC_I125]], undef
3739 ; CHECK-NEXT: br i1 [[EXITCOND]], label [[FOR_BODY21_I]], label [[FOR_END_I129_LOOPEXIT:%.*]]
3840 ; CHECK: for.end.i129.loopexit:
3941 ; CHECK-NEXT: br label [[FOR_END_I129]]
300300 ; PTR64-NEXT: [[CMP1604192:%.*]] = icmp ult i8* undef, [[ADD_PTR1603]]
301301 ; PTR64-NEXT: br i1 [[CMP1604192]], label [[FOR_BODY_PREHEADER:%.*]], label [[FOR_END1609:%.*]]
302302 ; PTR64: for.body.preheader:
303 ; PTR64-NEXT: [[SCEVGEP:%.*]] = getelementptr [512 x i8], [512 x i8]* [[BASE]], i64 1, i64 0
304303 ; PTR64-NEXT: br label [[FOR_BODY:%.*]]
305304 ; PTR64: for.body:
306305 ; PTR64-NEXT: [[R_17193:%.*]] = phi i8* [ [[INCDEC_PTR1608:%.*]], [[FOR_BODY]] ], [ null, [[FOR_BODY_PREHEADER]] ]
307306 ; PTR64-NEXT: [[INCDEC_PTR1608]] = getelementptr i8, i8* [[R_17193]], i64 1
308 ; PTR64-NEXT: [[EXITCOND:%.*]] = icmp ne i8* [[INCDEC_PTR1608]], [[SCEVGEP]]
309 ; PTR64-NEXT: br i1 [[EXITCOND]], label [[FOR_BODY]], label [[FOR_END1609_LOOPEXIT:%.*]]
307 ; PTR64-NEXT: [[CMP1604:%.*]] = icmp ult i8* [[INCDEC_PTR1608]], [[ADD_PTR1603]]
308 ; PTR64-NEXT: br i1 [[CMP1604]], label [[FOR_BODY]], label [[FOR_END1609_LOOPEXIT:%.*]]
310309 ; PTR64: for.end1609.loopexit:
311310 ; PTR64-NEXT: br label [[FOR_END1609]]
312311 ; PTR64: for.end1609:
320319 ; PTR32-NEXT: [[CMP1604192:%.*]] = icmp ult i8* undef, [[ADD_PTR1603]]
321320 ; PTR32-NEXT: br i1 [[CMP1604192]], label [[FOR_BODY_PREHEADER:%.*]], label [[FOR_END1609:%.*]]
322321 ; PTR32: for.body.preheader:
323 ; PTR32-NEXT: [[SCEVGEP:%.*]] = getelementptr [512 x i8], [512 x i8]* [[BASE]], i32 1, i32 0
324322 ; PTR32-NEXT: br label [[FOR_BODY:%.*]]
325323 ; PTR32: for.body:
326324 ; PTR32-NEXT: [[R_17193:%.*]] = phi i8* [ [[INCDEC_PTR1608:%.*]], [[FOR_BODY]] ], [ null, [[FOR_BODY_PREHEADER]] ]
327325 ; PTR32-NEXT: [[INCDEC_PTR1608]] = getelementptr i8, i8* [[R_17193]], i64 1
328 ; PTR32-NEXT: [[EXITCOND:%.*]] = icmp ne i8* [[INCDEC_PTR1608]], [[SCEVGEP]]
329 ; PTR32-NEXT: br i1 [[EXITCOND]], label [[FOR_BODY]], label [[FOR_END1609_LOOPEXIT:%.*]]
326 ; PTR32-NEXT: [[CMP1604:%.*]] = icmp ult i8* [[INCDEC_PTR1608]], [[ADD_PTR1603]]
327 ; PTR32-NEXT: br i1 [[CMP1604]], label [[FOR_BODY]], label [[FOR_END1609_LOOPEXIT:%.*]]
330328 ; PTR32: for.end1609.loopexit:
331329 ; PTR32-NEXT: br label [[FOR_END1609]]
332330 ; PTR32: for.end1609:
2424 ; CHECK-NEXT: entry:
2525 ; CHECK-NEXT: br label [[LOOP:%.*]]
2626 ; CHECK: loop:
27 ; CHECK-NEXT: [[P_0:%.*]] = phi i8* [ getelementptr inbounds ([240 x i8], [240 x i8]* @data, i64 0, i64 0), [[ENTRY:%.*]] ], [ [[TMP3:%.*]], [[CONT:%.*]] ]
28 ; CHECK-NEXT: [[TMP3]] = getelementptr inbounds i8, i8* [[P_0]], i64 1
29 ; CHECK-NEXT: br i1 [[ALWAYS_FALSE:%.*]], label [[NEVER_EXECUTED:%.*]], label [[CONT]]
30 ; CHECK: never_executed:
31 ; CHECK-NEXT: store volatile i8 0, i8* [[TMP3]]
32 ; CHECK-NEXT: br label [[CONT]]
33 ; CHECK: cont:
34 ; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i8* [[TMP3]], getelementptr (i8, i8* getelementptr inbounds ([240 x i8], [240 x i8]* @data, i64 0, i64 0), i64 246)
35 ; CHECK-NEXT: br i1 [[EXITCOND]], label [[LOOP]], label [[EXIT:%.*]]
36 ; CHECK: exit:
37 ; CHECK-NEXT: ret void
38 ;
39 entry:
40 br label %loop
41
42 loop:
43 %i.0 = phi i8 [ 0, %entry ], [ %tmp4, %cont ]
44 %p.0 = phi i8* [ getelementptr inbounds ([240 x i8], [240 x i8]* @data, i64 0, i64 0), %entry ], [ %tmp3, %cont ]
45 %tmp3 = getelementptr inbounds i8, i8* %p.0, i64 1
46 br i1 %always_false, label %never_executed, label %cont
47
48 never_executed:
49 store volatile i8 0, i8* %tmp3
50 br label %cont
51
52 cont:
53 %tmp4 = add i8 %i.0, 1
54 %tmp5 = icmp ult i8 %tmp4, -10
55 br i1 %tmp5, label %loop, label %exit
56
57 exit:
58 ret void
59 }
60
61 ; Similiar to above, but shows how we currently guard non-constant
62 ; memory operands in a manner which hides the latent miscompile.
63 define void @neg_dynamically_dead_inbounds2(i8* %a, i1 %always_false) #0 {
64 ; CHECK-LABEL: @neg_dynamically_dead_inbounds2(
65 ; CHECK-NEXT: entry:
66 ; CHECK-NEXT: br label [[LOOP:%.*]]
67 ; CHECK: loop:
6827 ; CHECK-NEXT: [[I_0:%.*]] = phi i8 [ 0, [[ENTRY:%.*]] ], [ [[TMP4:%.*]], [[CONT:%.*]] ]
69 ; CHECK-NEXT: [[P_0:%.*]] = phi i8* [ [[A:%.*]], [[ENTRY]] ], [ [[TMP3:%.*]], [[CONT]] ]
28 ; CHECK-NEXT: [[P_0:%.*]] = phi i8* [ getelementptr inbounds ([240 x i8], [240 x i8]* @data, i64 0, i64 0), [[ENTRY]] ], [ [[TMP3:%.*]], [[CONT]] ]
7029 ; CHECK-NEXT: [[TMP3]] = getelementptr inbounds i8, i8* [[P_0]], i64 1
7130 ; CHECK-NEXT: br i1 [[ALWAYS_FALSE:%.*]], label [[NEVER_EXECUTED:%.*]], label [[CONT]]
7231 ; CHECK: never_executed:
8443
8544 loop:
8645 %i.0 = phi i8 [ 0, %entry ], [ %tmp4, %cont ]
87 %p.0 = phi i8* [ %a, %entry ], [ %tmp3, %cont ]
46 %p.0 = phi i8* [ getelementptr inbounds ([240 x i8], [240 x i8]* @data, i64 0, i64 0), %entry ], [ %tmp3, %cont ]
8847 %tmp3 = getelementptr inbounds i8, i8* %p.0, i64 1
8948 br i1 %always_false, label %never_executed, label %cont
9049
10160 ret void
10261 }
10362
63 ; Similiar to above, but shows how we currently guard non-constant
64 ; memory operands in a manner which hides the latent miscompile.
65 define void @neg_dynamically_dead_inbounds2(i8* %a, i1 %always_false) #0 {
66 ; CHECK-LABEL: @neg_dynamically_dead_inbounds2(
67 ; CHECK-NEXT: entry:
68 ; CHECK-NEXT: br label [[LOOP:%.*]]
69 ; CHECK: loop:
70 ; CHECK-NEXT: [[I_0:%.*]] = phi i8 [ 0, [[ENTRY:%.*]] ], [ [[TMP4:%.*]], [[CONT:%.*]] ]
71 ; CHECK-NEXT: [[P_0:%.*]] = phi i8* [ [[A:%.*]], [[ENTRY]] ], [ [[TMP3:%.*]], [[CONT]] ]
72 ; CHECK-NEXT: [[TMP3]] = getelementptr inbounds i8, i8* [[P_0]], i64 1
73 ; CHECK-NEXT: br i1 [[ALWAYS_FALSE:%.*]], label [[NEVER_EXECUTED:%.*]], label [[CONT]]
74 ; CHECK: never_executed:
75 ; CHECK-NEXT: store volatile i8 0, i8* [[TMP3]]
76 ; CHECK-NEXT: br label [[CONT]]
77 ; CHECK: cont:
78 ; CHECK-NEXT: [[TMP4]] = add nuw i8 [[I_0]], 1
79 ; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i8 [[TMP4]], -10
80 ; CHECK-NEXT: br i1 [[EXITCOND]], label [[LOOP]], label [[EXIT:%.*]]
81 ; CHECK: exit:
82 ; CHECK-NEXT: ret void
83 ;
84 entry:
85 br label %loop
86
87 loop:
88 %i.0 = phi i8 [ 0, %entry ], [ %tmp4, %cont ]
89 %p.0 = phi i8* [ %a, %entry ], [ %tmp3, %cont ]
90 %tmp3 = getelementptr inbounds i8, i8* %p.0, i64 1
91 br i1 %always_false, label %never_executed, label %cont
92
93 never_executed:
94 store volatile i8 0, i8* %tmp3
95 br label %cont
96
97 cont:
98 %tmp4 = add i8 %i.0, 1
99 %tmp5 = icmp ult i8 %tmp4, -10
100 br i1 %tmp5, label %loop, label %exit
101
102 exit:
103 ret void
104 }
105
104106 define void @dom_store_preinc() #0 {
105107 ; CHECK-LABEL: @dom_store_preinc(
106108 ; CHECK-NEXT: entry:
109111 ; CHECK-NEXT: [[P_0:%.*]] = phi i8* [ getelementptr inbounds ([240 x i8], [240 x i8]* @data, i64 0, i64 0), [[ENTRY:%.*]] ], [ [[TMP3:%.*]], [[LOOP]] ]
110112 ; CHECK-NEXT: store volatile i8 0, i8* [[P_0]]
111113 ; CHECK-NEXT: [[TMP3]] = getelementptr inbounds i8, i8* [[P_0]], i64 1
112 ; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i8* [[TMP3]], getelementptr (i8, i8* getelementptr inbounds ([240 x i8], [240 x i8]* @data, i64 0, i64 0), i64 246)
114 ; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i8* [[P_0]], getelementptr (i8, i8* getelementptr inbounds ([240 x i8], [240 x i8]* @data, i64 0, i64 0), i64 245)
113115 ; CHECK-NEXT: br i1 [[EXITCOND]], label [[LOOP]], label [[EXIT:%.*]]
114116 ; CHECK: exit:
115117 ; CHECK-NEXT: ret void
166166 ; CHECK-NEXT: [[TMP2:%.*]] = load i8, i8* [[DOT0]], align 1
167167 ; CHECK-NEXT: [[TMP3]] = getelementptr inbounds i8, i8* [[P_0]], i64 1
168168 ; CHECK-NEXT: store i8 [[TMP2]], i8* [[P_0]], align 1
169 ; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i8* [[TMP3]], getelementptr (i8, i8* getelementptr inbounds ([240 x i8], [240 x i8]* @data, i64 0, i64 0), i64 240)
169 ; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i8* [[P_0]], getelementptr inbounds ([240 x i8], [240 x i8]* @data, i64 0, i64 239)
170170 ; CHECK-NEXT: br i1 [[EXITCOND]], label [[LOOP]], label [[EXIT:%.*]]
171171 ; CHECK: exit:
172172 ; CHECK-NEXT: ret void
607607 ; CHECK-NEXT: [[IV:%.*]] = phi i8* [ null, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[FOR_BODY29]] ]
608608 ; CHECK-NEXT: [[TMP0:%.*]] = load volatile i8, i8* [[IV]], align 1
609609 ; CHECK-NEXT: [[IV_NEXT]] = getelementptr inbounds i8, i8* [[IV]], i64 1
610 ; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i8* [[IV_NEXT]], inttoptr (i64 11 to i8*)
610 ; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i8* [[IV]], inttoptr (i64 10 to i8*)
611611 ; CHECK-NEXT: br i1 [[EXITCOND]], label [[FOR_BODY29]], label [[EXIT:%.*]]
612612 ; CHECK: exit:
613613 ; CHECK-NEXT: ret void