[analyzer] Fix a crash on logical operators with vectors.

[NoQ]

Summary

We crash whenever we try to compute x && y or x || y where x and y are of vector type.

For now we do not seem to properly model operations with vectors. In particular, operations && and || on two values of type int __attribute__((ext_vector_type(2))) are not short-circuit, unlike regular logical operators, so even our CFG is incorrect.

Avoid the crash, add respective FIXME tests for later.

[xazax.hun]

LGTM!

lib/StaticAnalyzer/Core/ExprEngineC.cpp

633

I was wondering maybe moving this code down a bit you could reuse the StmtNodeBuilder bellow and the could be slightly shorter. But I guess it is just the matter of taste.

[NoQ]

Yep, right!

[NoQ]

A better name for the test function.

[dcoughlin]

Interesting bug! The workaround looks good to me.

lib/StaticAnalyzer/Core/ExprEngineC.cpp

void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred,
   getCheckerManager().runCheckersForPostStmt(Dst, B.getResults(), DS, *this);
 }
 
 void ExprEngine::VisitLogicalExpr(const BinaryOperator* B, ExplodedNode *Pred,
                                   ExplodedNodeSet &Dst) {
   assert(B->getOpcode() == BO_LAnd ||
          B->getOpcode() == BO_LOr);
 
+  if (B->getType()->isVectorType()) {
+    // FIXME: We do not model vector arithmetic yet. When adding support for
+    // that, note that the CFG-based reasoning below does not apply, because
+    // logical operators on vectors are not short-circuit. Currently they are
+    // modeled as short-circuit in Clang CFG but this is incorrect.
+    StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+    // Do not set the value for the expression. It'd be UnknownVal by default.
+    Bldr.generateNode(B, Pred, Pred->getState());
+    return;
+  }
+
   StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
   ProgramStateRef state = Pred->getState();
 
   ExplodedNode *N = Pred;
   while (!N->getLocation().getAs<BlockEntrance>()) {
     ProgramPoint P = N->getLocation();
     assert(P.getAs<PreStmt>()|| P.getAs<PreStmtPurgeDeadSymbols>());
     (void) P;

test/Analysis/vector.c

+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+typedef int __attribute__((ext_vector_type(2))) V;
+
+void clang_analyzer_numTimesReached();
+void clang_analyzer_eval(int);
+
+int flag;
+
+V pass_through_and_set_flag(V v) {
+  flag = 1;
+  return v;
+}
+
+V no_crash(V x, V y) {
+  flag = 0;
+  V z = x && pass_through_and_set_flag(y);
+  clang_analyzer_eval(flag); // expected-warning{{TRUE}}
+  // FIXME: For now we treat vector operator && as short-circuit,
+  // but in fact it is not. It should always evaluate
+  // pass_through_and_set_flag(). It should not split state.
+  // Now we also get FALSE on the other path.
+  // expected-warning@-5{{FALSE}}
+
+  // FIXME: Should be 1 since we should not split state.
+  clang_analyzer_numTimesReached(); // expected-warning{{2}}
+  return z;
+}