From 40eaa8df99982568bc71017e8a2e79242dbe0c5a Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Wed, 25 Feb 2015 18:00:15 +0000 Subject: Fix really obscure bug in CannotBeNegativeZero() (PR22688) With a diabolically crafted test case, we could recurse through this code and return true instead of false. The larger engineering crime is the use of magic numbers. Added FIXME comments for those. llvm-svn: 230515 --- llvm/lib/Analysis/ValueTracking.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'llvm/lib/Analysis/ValueTracking.cpp') diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp index 5ae8574ebe1..0458d284eb4 100644 --- a/llvm/lib/Analysis/ValueTracking.cpp +++ b/llvm/lib/Analysis/ValueTracking.cpp @@ -2000,8 +2000,11 @@ bool llvm::CannotBeNegativeZero(const Value *V, unsigned Depth) { if (const ConstantFP *CFP = dyn_cast(V)) return !CFP->getValueAPF().isNegZero(); + // FIXME: Magic number! At the least, this should be given a name because it's + // used similarly in CannotBeOrderedLessThanZero(). A better fix may be to + // expose it as a parameter, so it can be used for testing / experimenting. if (Depth == 6) - return 1; // Limit search depth. + return false; // Limit search depth. const Operator *I = dyn_cast(V); if (!I) return false; @@ -2048,6 +2051,9 @@ bool llvm::CannotBeOrderedLessThanZero(const Value *V, unsigned Depth) { if (const ConstantFP *CFP = dyn_cast(V)) return !CFP->getValueAPF().isNegative() || CFP->getValueAPF().isZero(); + // FIXME: Magic number! At the least, this should be given a name because it's + // used similarly in CannotBeNegativeZero(). A better fix may be to + // expose it as a parameter, so it can be used for testing / experimenting. if (Depth == 6) return false; // Limit search depth. -- cgit v1.2.3