[analyzer] Support C++ default arguments if they are literal values.

A CXXDefaultArgExpr wraps an Expr owned by a ParmVarDecl belonging to the
called function. In general, ExprEngine and Environment ought to treat this
like a ParenExpr or other transparent wrapper expression, with the inside
expression evaluated first.

However, if we call the same function twice, we'd produce a CFG that contains
the same wrapped expression twice, and we're not set up to handle that. I've
added a FIXME to the CFG builder to come back to that, but meanwhile we can
at least handle expressions that don't need to be explicitly evaluated:
literals. This probably handles many common uses of default parameters:
true/false, null, etc.

Part of PR13385 / <rdar://problem/12156507>

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

diff --git a/lib/Analysis/CFG.cpp b/lib/Analysis/CFG.cpp
index 41d06f454610b8a9ea69c0585478481f78465f66..70ea990f559fc55cc5b83cba21e8e2a59015d9d5 100644 (file)
--- a/lib/Analysis/CFG.cpp
+++ b/lib/Analysis/CFG.cpp
@@ -1022,6 +1022,14 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc) {
     case Stmt::ExprWithCleanupsClass:
       return VisitExprWithCleanups(cast<ExprWithCleanups>(S), asc);
 
+    case Stmt::CXXDefaultArgExprClass:
+      // FIXME: The expression inside a CXXDefaultArgExpr is owned by the
+      // called function's declaration, not by the caller. If we simply add
+      // this expression to the CFG, we could end up with the same Expr
+      // appearing multiple times.
+      // PR13385 / <rdar://problem/12156507>
+      return VisitStmt(S, asc);
+
     case Stmt::CXXBindTemporaryExprClass:
       return VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), asc);
 

diff --git a/lib/StaticAnalyzer/Core/Environment.cpp b/lib/StaticAnalyzer/Core/Environment.cpp
index 52644f7702fcdd1085c6e439e1fc4cba136c2ce5..534f37858c7895add03edcf24ad52ceed8ad2bc1 100644 (file)
--- a/lib/StaticAnalyzer/Core/Environment.cpp
+++ b/lib/StaticAnalyzer/Core/Environment.cpp
@@ -99,6 +99,9 @@ SVal Environment::getSVal(const EnvironmentEntry &Entry,
       case Stmt::SubstNonTypeTemplateParmExprClass:
         E = cast<SubstNonTypeTemplateParmExpr>(E)->getReplacement();
         continue;
+      case Stmt::CXXDefaultArgExprClass:
+        E = cast<CXXDefaultArgExpr>(E)->getExpr();
+        continue;
       case Stmt::ObjCStringLiteralClass: {
         MemRegionManager &MRMgr = svalBuilder.getRegionManager();
         const ObjCStringLiteral *SL = cast<ObjCStringLiteral>(E);

diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index a77a338b2797eee2127f6dbae5e71fd822123c04..d9b7ff57bff4c5f7e4a0f41f3b30e940664491fa 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -529,11 +529,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       break;
     }
     
-    // We don't handle default arguments either yet, but we can fake it
-    // for now by just skipping them.
-    case Stmt::CXXDefaultArgExprClass:
-      break;
-
     case Stmt::ParenExprClass:
       llvm_unreachable("ParenExprs already handled.");
     case Stmt::GenericSelectionExprClass:
@@ -619,6 +614,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
     case Stmt::StringLiteralClass:
     case Stmt::ObjCStringLiteralClass:
     case Stmt::CXXBindTemporaryExprClass:
+    case Stmt::CXXDefaultArgExprClass:
     case Stmt::SubstNonTypeTemplateParmExprClass:
     case Stmt::CXXNullPtrLiteralExprClass: {
       Bldr.takeNodes(Pred);

diff --git a/test/Analysis/inline.cpp b/test/Analysis/inline.cpp
index 6b9a885f50f391adeb50e1de6435bcd8798ba3f0..65907762662e9dc0c4b43982316ea76ae1dd2b49 100644 (file)
--- a/test/Analysis/inline.cpp
+++ b/test/Analysis/inline.cpp
@@ -193,3 +193,37 @@ namespace Invalidation {
     }
   };
 }
+
+namespace DefaultArgs {
+  int takesDefaultArgs(int i = 42) {
+    return -i;
+  }
+
+  void testFunction() {
+    clang_analyzer_eval(takesDefaultArgs(1) == -1); // expected-warning{{TRUE}}
+    clang_analyzer_eval(takesDefaultArgs() == -42); // expected-warning{{TRUE}}
+  }
+
+  class Secret {
+  public:
+    static const int value = 42;
+    int get(int i = value) {
+      return i;
+    }
+  };
+
+  void testMethod() {
+    Secret obj;
+    clang_analyzer_eval(obj.get(1) == 1); // expected-warning{{TRUE}}
+
+    // FIXME: Should be 'TRUE'. See PR13673 or <rdar://problem/11720796>.
+    clang_analyzer_eval(obj.get() == 42); // expected-warning{{UNKNOWN}}
+
+    // FIXME: Even if we constrain the variable, we still have a problem.
+    // See PR13385 or <rdar://problem/12156507>.
+    if (Secret::value != 42)
+      return;
+    clang_analyzer_eval(Secret::value == 42); // expected-warning{{TRUE}}
+    clang_analyzer_eval(obj.get() == 42); // expected-warning{{UNKNOWN}}
+  }
+}