[analyzer] Do not crash when processing binary "?:" in C++

When computing the value of ?: expression, we rely on the last expression in
the previous basic block to be the resulting value of the expression. This is
not the case for binary "?:" operator (GNU extension) in C++. As the last
basic block has the expression for the condition subexpression, which is an
R-value, whereas the true subexpression is the L-value.

Note the operator evaluation just happens to work in C since the true
subexpression is an R-value (like the condition subexpression). CFG is the
same in C and C++ case, but the AST nodes are different, which the LValue to
Rvalue conversion happening after the BinaryConditionalOperator evaluation.

Changed the logic to only use the last expression from the predecessor only
if it matches either true or false subexpression. Note, the logic needed
fortification anyway: L and R were passed but not even used by the function.

Also, change the conjureSymbolVal to correctly compute the type, when the
expression is an LG-value.

llvm-svn: 179574

3 files changed, 49 insertions, 9 deletions

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index 7a53fccf2ed..2d52c01e1d2 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -617,11 +617,15 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex,
                                   const Expr *R,
                                   ExplodedNode *Pred,
                                   ExplodedNodeSet &Dst) {
+  assert(L && R);
+
   StmtNodeBuilder B(Pred, Dst, *currBldrCtx);
   ProgramStateRef state = Pred->getState();
   const LocationContext *LCtx = Pred->getLocationContext();
   const CFGBlock *SrcBlock = 0;
 
+  // Find the predecessor block.
+  ProgramStateRef SrcState = state;
   for (const ExplodedNode *N = Pred ; N ; N = *N->pred_begin()) {
     ProgramPoint PP = N->getLocation();
     if (PP.getAs<PreStmtPurgeDeadSymbols>() || PP.getAs<BlockEntrance>()) {
@@ -629,6 +633,7 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex,
       continue;
     }
     SrcBlock = PP.castAs<BlockEdge>().getSrc();
+    SrcState = N->getState();
     break;
   }
 
@@ -644,14 +649,25 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex,
     CFGElement CE = *I;
     if (Optional<CFGStmt> CS = CE.getAs<CFGStmt>()) {
       const Expr *ValEx = cast<Expr>(CS->getStmt());
-      hasValue = true;
-      V = state->getSVal(ValEx, LCtx);
+      ValEx = ValEx->IgnoreParens();
+
+      // For GNU extension '?:' operator, the left hand side will be an
+      // OpaqueValueExpr, so get the underlying expression.
+      if (const OpaqueValueExpr *OpaqueEx = dyn_cast<OpaqueValueExpr>(L))
+        L = OpaqueEx->getSourceExpr();
+
+      // If the last expression in the predecessor block matches true or false
+      // subexpression, get its the value.
+      if (ValEx == L->IgnoreParens() || ValEx == R->IgnoreParens()) {
+        hasValue = true;
+        V = SrcState->getSVal(ValEx, LCtx);
+      }
       break;
     }
   }
 
-  assert(hasValue);
-  (void) hasValue;
+  if (!hasValue)
+    V = svalBuilder.conjureSymbolVal(0, Ex, LCtx, currBldrCtx->blockCount());
 
   // Generate a new node with the binding from the appropriate path.
   B.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, V, true));
diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
index 19a4353ffc0..309230ebed0 100644
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -106,12 +106,19 @@ SValBuilder::getRegionValueSymbolVal(const TypedValueRegion* region) {
   return nonloc::SymbolVal(sym);
 }
 
-DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const void *symbolTag,
-                                                   const Expr *expr,
+DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const void *SymbolTag,
+                                                   const Expr *Ex,
                                                    const LocationContext *LCtx,
-                                                   unsigned count) {
-  QualType T = expr->getType();
-  return conjureSymbolVal(symbolTag, expr, LCtx, T, count);
+                                                   unsigned Count) {
+  QualType T = Ex->getType();
+
+  // Compute the type of the result. If the expression is not an R-value, the
+  // result should be a location.
+  QualType ExType = Ex->getType();
+  if (Ex->isGLValue())
+    T = LCtx->getAnalysisDeclContext()->getASTContext().getPointerType(ExType);
+
+  return conjureSymbolVal(SymbolTag, Ex, LCtx, T, Count);
 }
 
 DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const void *symbolTag,
diff --git a/clang/test/Analysis/conditional-operator.cpp b/clang/test/Analysis/conditional-operator.cpp
new file mode 100644
index 00000000000..5a3c325b91d
--- /dev/null
+++ b/clang/test/Analysis/conditional-operator.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang -cc1 -analyze -analyzer-checker=core,debug.ExprInspection %s -analyzer-output=text -verify
+
+void clang_analyzer_eval(bool);
+
+// Test that the analyzer does not crash on GNU extension operator "?:".
+void NoCrashTest(int x, int y) {
+	int w = x ?: y;
+}
+
+void OperatorEvaluationTest(int y) {
+  int x = 1;
+	int w = x ?: y;  // expected-note {{'?' condition is true}}
+	
+	// TODO: We are not precise when processing the "?:" operator in C++.
+  clang_analyzer_eval(w == 1); // expected-warning{{UNKNOWN}}
+                               // expected-note@-1{{UNKNOWN}}
+}
\ No newline at end of file