[analyzer] pr37209: Fix casts of glvalues to references.

[NoQ]

Summary

In the provided test case the expression (int *&)*(int *)p is evaluated as follows:

1. Take an lvalue p of type char *. Its symbolic value will be &p.
2. Load an rvalue p of type char *. The resulting value is &SymRegion{reg_$0<p>}.
3. Cast it to rvalue (int *)p of type int *. The resulting value is &element{SymRegion{reg_$0<p>}, 0, int}.
4. Treat it as lvalue *(int *)p of type int. The resulting value is still &element{SymRegion{reg_$0<p>}, 0, int} because the analyzer doesn't discriminate between lvalues and respective pointer rvalues - both are simply "Loc".
5. Cast it to lvalue (int *&)*(int *)p of type (int *). The resulting value is computed incorrectly as &element{SymRegion{reg_$0<p>}, 0, int}.

Note: on the last step we take an lvalue of type int and cast it to lvalue of type int *. There are no references in the expression types, but there's a reference in the type-as-written of the outer cast-expression.

Then we try to evaluate a cast from int to int *&, because the analyzer is clever enough to realize that it needs to take type-as-written for the explicit cast. The cast, of course, misbehaves - there's no intended behavior in converting a Loc from an integer to a reference-to-a-pointer.

I tried to detect that situation and work around it by saying that we should instead evaluate a cast from int & to int *&. I.e., add an artificial & to the original expression's type. It seems to work correctly (i.e., the resulting value becomes &element{SymRegion{reg_$0<p>}, 0, int *} which seems reasonable) and it should work because it's a reasonable requirement for evalCast() to support such casts. That said, i didn't investigate all various ASTs carefully yet, in order to figure out if my pattern-matching is correct, so i might have missed some corner cases, so i'll most likely try to investigate it more carefully, or at least see if it causes regressions on large codebases, before committing.

[NoQ]

i'll most likely try to investigate it more carefully, or at least see if it causes regressions on large codebases, before committing

Seemed fine, no visible change. I hope this means that i pattern-matched carefully enough.

lib/StaticAnalyzer/Core/ExprEngineC.cpp

void ExprEngine::VisitBlockExpr(const BlockExpr *BE, ExplodedNode *Pred,
   // FIXME: Move all post/pre visits to ::Visit().
   getCheckerManager().runCheckersForPostStmt(Dst, Tmp, BE, *this);
 }
 
 ProgramStateRef ExprEngine::handleLValueBitCast(
     ProgramStateRef state, const Expr* Ex, const LocationContext* LCtx,
     QualType T, QualType ExTy, const CastExpr* CastE, StmtNodeBuilder& Bldr,
     ExplodedNode* Pred) {
+  if (T->isLValueReferenceType()) {
+    assert(!CastE->getType()->isLValueReferenceType());
+    ExTy = getContext().getLValueReferenceType(ExTy);
+  } else if (T->isRValueReferenceType()) {
+    assert(!CastE->getType()->isRValueReferenceType());
+    ExTy = getContext().getRValueReferenceType(ExTy);
+  }
   // Delegate to SValBuilder to process.
   SVal OrigV = state->getSVal(Ex, LCtx);
   SVal V = svalBuilder.evalCast(OrigV, T, ExTy);
   // Negate the result if we're treating the boolean as a signed i1
   if (CastE->getCastKind() == CK_BooleanToSignedIntegral)
     V = evalMinus(V);
   state = state->BindExpr(CastE, LCtx, V);
   if (V.isUnknown() && !OrigV.isUnknown()) {

test/Analysis/casts.cpp

case 0:
     break;
   }
 
   switch ((int)(bool)c) { // no-warning
     case 0:
       break;
   }
 }
+
+int *&castToIntPtrLValueRef(char *p) {
+  return (int *&)*(int *)p;
+}
+bool testCastToIntPtrLValueRef(char *p, int *s) {
+  return castToIntPtrLValueRef(p) != s; // no-crash
+}
+
+int *&&castToIntPtrRValueRef(char *p) {
+  return (int *&&)*(int *)p;
+}
+bool testCastToIntPtrRValueRef(char *p, int *s) {
+  return castToIntPtrRValueRef(p) != s; // no-crash
+}