From 84a1ca52807a4fd79dd1b43aa3700514638b0c65 Mon Sep 17 00:00:00 2001
From: Ted Kremenek <kremenek@apple.com>
Date: Sat, 6 Aug 2011 00:30:00 +0000
Subject: [PATCH] [analyzer] Simplify logic for
 ExprEngine::VisitUnaryExprOrTypeTraitExpr to avoid recursion to
 subexpression.

This exposed bugs in the live variables analysis, and a latent analyzer bug in the SymbolReaper.

llvm-svn: 137006
---
 clang/lib/Analysis/CFG.cpp                   |  9 ++++
 clang/lib/Analysis/LiveVariables.cpp         | 43 +++++++++++++++-----
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 27 +++++-------
 3 files changed, 53 insertions(+), 26 deletions(-)

diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index be6ffee668d2..1edb328d013e 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -2204,6 +2204,15 @@ CFGBlock *CFGBuilder::VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E,
          VA != 0; VA = FindVA(VA->getElementType().getTypePtr()))
       lastBlock = addStmt(VA->getSizeExpr());
   }
+  else {
+    // For sizeof(x), where 'x' is a VLA, we should include the computation
+    // of the lvalue of 'x'.
+    Expr *subEx = E->getArgumentExpr();
+    if (subEx->getType()->isVariableArrayType()) {
+      assert(subEx->isLValue());
+      lastBlock = addStmt(subEx);
+    }
+  }
 
   return lastBlock;
 }
diff --git a/clang/lib/Analysis/LiveVariables.cpp b/clang/lib/Analysis/LiveVariables.cpp
index c6c091e12959..71c1917c82f7 100644
--- a/clang/lib/Analysis/LiveVariables.cpp
+++ b/clang/lib/Analysis/LiveVariables.cpp
@@ -146,6 +146,19 @@ public:
 };
 }
 
+static const VariableArrayType *FindVA(QualType Ty) {
+  const Type *ty = Ty.getTypePtr();
+  while (const ArrayType *VT = dyn_cast<ArrayType>(ty)) {
+    if (const VariableArrayType *VAT = dyn_cast<VariableArrayType>(VT))
+      if (VAT->getSizeExpr())
+        return VAT;
+    
+    ty = VT->getElementType().getTypePtr();
+  }
+  
+  return 0;
+}
+
 void TransferFunctions::Visit(Stmt *S) {
   if (observer)
     observer->observeStmt(S, currentBlock, val);
@@ -174,6 +187,17 @@ void TransferFunctions::Visit(Stmt *S) {
                         CE->getImplicitObjectArgument()->IgnoreParens());
       break;
     }
+    case Stmt::DeclStmtClass: {
+      const DeclStmt *DS = cast<DeclStmt>(S);
+      if (const VarDecl *VD = dyn_cast<VarDecl>(DS->getSingleDecl())) {
+        for (const VariableArrayType* VA = FindVA(VD->getType());
+             VA != 0; VA = FindVA(VA->getElementType())) {
+          val.liveStmts = LV.SSetFact.add(val.liveStmts,
+                                          VA->getSizeExpr()->IgnoreParens());
+        }
+      }
+      break;
+    }
     // FIXME: These cases eventually shouldn't be needed.
     case Stmt::ExprWithCleanupsClass: {
       S = cast<ExprWithCleanups>(S)->getSubExpr();
@@ -187,6 +211,10 @@ void TransferFunctions::Visit(Stmt *S) {
       S = cast<MaterializeTemporaryExpr>(S)->GetTemporaryExpr();
       break;
     }
+    case Stmt::UnaryExprOrTypeTraitExprClass: {
+      // No need to unconditionally visit subexpressions.
+      return;
+    }
   }
   
   for (Stmt::child_iterator it = S->child_begin(), ei = S->child_end();
@@ -281,16 +309,11 @@ VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *UE)
   if (UE->getKind() != UETT_SizeOf || UE->isArgumentType())
     return;
 
-  const DeclRefExpr *DR =
-    dyn_cast<DeclRefExpr>(UE->getArgumentExpr()->IgnoreParens());
-  
-  if (!DR)
-    return;
-  
-  const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl());
-
-  if (VD && VD->getType()->isVariableArrayType())
-    val.liveDecls = LV.DSetFact.add(val.liveDecls, VD);
+  const Expr *subEx = UE->getArgumentExpr();
+  if (subEx->getType()->isVariableArrayType()) {
+    assert(subEx->isLValue());
+    val.liveStmts = LV.SSetFact.add(val.liveStmts, subEx->IgnoreParens());
+  }
 }
 
 void TransferFunctions::VisitUnaryOperator(UnaryOperator *UO) {
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 71f6f471fb89..765db9955578 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2269,25 +2269,20 @@ void ExprEngine::VisitUnaryExprOrTypeTraitExpr(
       // Get the size by getting the extent of the sub-expression.
       // First, visit the sub-expression to find its region.
       const Expr *Arg = Ex->getArgumentExpr();
-      ExplodedNodeSet Tmp;
-      Visit(Arg, Pred, Tmp);
+      const GRState *state = GetState(Pred);
+      const MemRegion *MR = state->getSVal(Arg).getAsRegion();
 
-      for (ExplodedNodeSet::iterator I=Tmp.begin(), E=Tmp.end(); I!=E; ++I) {
-        const GRState* state = GetState(*I);
-        const MemRegion *MR = state->getSVal(Arg).getAsRegion();
-
-        // If the subexpression can't be resolved to a region, we don't know
-        // anything about its size. Just leave the state as is and continue.
-        if (!MR) {
-          Dst.Add(*I);
-          continue;
-        }
-
-        // The result is the extent of the VLA.
-        SVal Extent = cast<SubRegion>(MR)->getExtent(svalBuilder);
-        MakeNode(Dst, Ex, *I, state->BindExpr(Ex, Extent));
+      // If the subexpression can't be resolved to a region, we don't know
+      // anything about its size. Just leave the state as is and continue.
+      if (!MR) {
+        Dst.Add(Pred);
+        return;
       }
 
+      // The result is the extent of the VLA.
+      SVal Extent = cast<SubRegion>(MR)->getExtent(svalBuilder);
+      MakeNode(Dst, Ex, Pred, state->BindExpr(Ex, Extent));
+
       return;
     }
     else if (T->getAs<ObjCObjectType>()) {