[analyzer] Allow tracknullOrUndef look through the ternary operator even when conditi...

Improvement of r178684 and r178685.

Jordan has pointed out that I should not rely on the value of the condition to know which expression branch
has been taken. It will not work in cases the branch condition is an unknown value (ex: we do not track the constraints for floats).
The better way of doing this would be to find out if the current node is the right or left successor of the node
that has the ternary operator as a terminator (which is how this is done in other places, like ConditionBRVisitor).

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178701 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 60a7e65254dce8d10048331b877f11cc40268c0b..c3bbc3baf69b01c7444c61cfe89102cf0af14459 100644 (file)
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -799,21 +799,25 @@ static const Expr *peelOffOuterExpr(const Expr *Ex,
 
   // Peel off the ternary operator.
   if (const ConditionalOperator *CO = dyn_cast<ConditionalOperator>(Ex)) {
-    const Expr *CondEx = CO->getCond();
-
-    // Find a node where the value of the condition is known.
+    // Find a node where the branching occured and find out which branch
+    // we took (true/false) by looking at the ExplodedGraph.
+    const ExplodedNode *NI = N;
     do {
-      ProgramStateRef State = N->getState();
-      SVal CondVal = State->getSVal(CondEx, N->getLocationContext());
-      ConditionTruthVal CondEvaluated = State->isNull(CondVal);
-      if (CondEvaluated.isConstrained()) {
-        if (CondEvaluated.isConstrainedTrue())
-          return peelOffOuterExpr(CO->getFalseExpr(), N);
-        else
-          return peelOffOuterExpr(CO->getTrueExpr(), N);
+      ProgramPoint ProgPoint = NI->getLocation();
+      if (Optional<BlockEdge> BE = ProgPoint.getAs<BlockEdge>()) {
+        const CFGBlock *srcBlk = BE->getSrc();
+        if (const Stmt *term = srcBlk->getTerminator()) {
+          if (term == CO) {
+            bool TookTrueBranch = (*(srcBlk->succ_begin()) == BE->getDst());
+            if (TookTrueBranch)
+              return peelOffOuterExpr(CO->getTrueExpr(), N);
+            else
+              return peelOffOuterExpr(CO->getFalseExpr(), N);
+          }
+        }
       }
-      N = N->getFirstPred();
-    } while (N);
+      NI = NI->getFirstPred();
+    } while (NI);
   }
   return Ex;
 }

diff --git a/test/Analysis/inlining/false-positive-suppression.c b/test/Analysis/inlining/false-positive-suppression.c
index 248d854dbc29b98b4035e11516c31d266f4a9287..a836d9c6243379cdafb7fd54fdedda4515220027 100644 (file)
--- a/test/Analysis/inlining/false-positive-suppression.c
+++ b/test/Analysis/inlining/false-positive-suppression.c
@@ -260,9 +260,10 @@ int testNestedConditionalOperator(int x) {
   return *(x ? (x ? 0 : getPtr()) : getPtr()); // expected-warning {{Dereference of null pointer}}
 }
 
-// False Positve - we are unable to suppress this case because the condition is
-// float.
 int testConditionalOperatorSuppressFloatCond(float x) {
-  return *(x ? getNull() : getPtr()); // expected-warning {{Dereference of null pointer}}
+  return *(x ? getNull() : getPtr());
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Dereference of null pointer}}
+#endif
 }