From 1c45220028a8aa39ad2f26c8c3c6234d1a816086 Mon Sep 17 00:00:00 2001 From: Juneyoung Lee Date: Thu, 24 Sep 2020 18:52:30 +0900 Subject: [PATCH] [ValueTracking] Check uses of Argument if it is given to isGuaranteedNotToBeUndefOrPoison This is a patch that allows isGuaranteedNotToBeUndefOrPoison to return more precise result when an argument is given, by looking through its uses at the entry block (and following blocks as well, if it is checking poison only). This is useful when there is a function call with noundef arguments at the entry block. Reviewed By: nikic Differential Revision: https://reviews.llvm.org/D88207 --- llvm/lib/Analysis/ValueTracking.cpp | 56 +++++++++++-------- llvm/unittests/Analysis/ValueTrackingTest.cpp | 10 ++++ 2 files changed, 42 insertions(+), 24 deletions(-) diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp index 1a894959c5bd..682baacac3a9 100644 --- a/llvm/lib/Analysis/ValueTracking.cpp +++ b/llvm/lib/Analysis/ValueTracking.cpp @@ -4871,7 +4871,7 @@ bool llvm::canCreatePoison(const Operator *Op) { return ::canCreateUndefOrPoison(Op, /*PoisonOnly=*/true); } -static bool programUndefinedIfUndefOrPoison(const Instruction *Inst, +static bool programUndefinedIfUndefOrPoison(const Value *V, bool PoisonOnly); static bool isGuaranteedNotToBeUndefOrPoison(const Value *V, @@ -4931,10 +4931,8 @@ static bool isGuaranteedNotToBeUndefOrPoison(const Value *V, return true; } - if (auto *I = dyn_cast(V)) { - if (programUndefinedIfUndefOrPoison(I, PoisonOnly)) - return true; - } + if (programUndefinedIfUndefOrPoison(V, PoisonOnly)) + return true; // CxtI may be null or a cloned instruction. if (!CtxI || !CtxI->getParent() || !DT) @@ -5161,7 +5159,7 @@ bool llvm::mustTriggerUB(const Instruction *I, return false; } -static bool programUndefinedIfUndefOrPoison(const Instruction *Inst, +static bool programUndefinedIfUndefOrPoison(const Value *V, bool PoisonOnly) { // We currently only look for uses of values within the same basic // block, as that makes it easier to guarantee that the uses will be @@ -5170,9 +5168,20 @@ static bool programUndefinedIfUndefOrPoison(const Instruction *Inst, // FIXME: Expand this to consider uses beyond the same basic block. To do // this, look out for the distinction between post-dominance and strong // post-dominance. - const BasicBlock *BB = Inst->getParent(); + const BasicBlock *BB = nullptr; + BasicBlock::const_iterator Begin; + if (const auto *Inst = dyn_cast(V)) { + BB = Inst->getParent(); + Begin = Inst->getIterator(); + Begin++; + } else if (const auto *Arg = dyn_cast(V)) { + BB = &Arg->getParent()->getEntryBlock(); + Begin = BB->begin(); + } else { + return false; + } - BasicBlock::const_iterator Begin = Inst->getIterator(), End = BB->end(); + BasicBlock::const_iterator End = BB->end(); if (!PoisonOnly) { // Be conservative & just check whether a value is passed to a noundef @@ -5185,7 +5194,7 @@ static bool programUndefinedIfUndefOrPoison(const Instruction *Inst, if (const auto *CB = dyn_cast(&I)) { for (unsigned i = 0; i < CB->arg_size(); ++i) { if (CB->paramHasAttr(i, Attribute::NoUndef) && - CB->getArgOperand(i) == Inst) + CB->getArgOperand(i) == V) return true; } } @@ -5199,27 +5208,26 @@ static bool programUndefinedIfUndefOrPoison(const Instruction *Inst, // does. SmallSet YieldsPoison; SmallSet Visited; - YieldsPoison.insert(Inst); - Visited.insert(Inst->getParent()); + + YieldsPoison.insert(V); + auto Propagate = [&](const User *User) { + if (propagatesPoison(cast(User))) + YieldsPoison.insert(User); + }; + for_each(V->users(), Propagate); + Visited.insert(BB); unsigned Iter = 0; while (Iter++ < MaxAnalysisRecursionDepth) { for (auto &I : make_range(Begin, End)) { - if (&I != Inst) { - if (mustTriggerUB(&I, YieldsPoison)) - return true; - if (!isGuaranteedToTransferExecutionToSuccessor(&I)) - return false; - } + if (mustTriggerUB(&I, YieldsPoison)) + return true; + if (!isGuaranteedToTransferExecutionToSuccessor(&I)) + return false; // Mark poison that propagates from I through uses of I. - if (YieldsPoison.count(&I)) { - for (const User *User : I.users()) { - const Instruction *UserI = cast(User); - if (propagatesPoison(cast(UserI))) - YieldsPoison.insert(User); - } - } + if (YieldsPoison.count(&I)) + for_each(I.users(), Propagate); } if (auto *NextBB = BB->getSingleSuccessor()) { diff --git a/llvm/unittests/Analysis/ValueTrackingTest.cpp b/llvm/unittests/Analysis/ValueTrackingTest.cpp index c45bca1c53bf..2cfca2ca22ed 100644 --- a/llvm/unittests/Analysis/ValueTrackingTest.cpp +++ b/llvm/unittests/Analysis/ValueTrackingTest.cpp @@ -768,6 +768,16 @@ TEST_F(ValueTrackingTest, isGuaranteedNotToBePoison_exploitBranchCond) { } } +TEST_F(ValueTrackingTest, isGuaranteedNotToBeUndefOrPoison) { + parseAssembly("declare void @f(i32 noundef)" + "define void @test(i32 %x) {\n" + " %A = bitcast i32 %x to i32\n" + " call void @f(i32 noundef %x)\n" + " ret void\n" + "}\n"); + EXPECT_EQ(isGuaranteedNotToBeUndefOrPoison(A), true); +} + TEST(ValueTracking, canCreatePoisonOrUndef) { std::string AsmHead = "declare i32 @g(i32)\n"