[cfe-commits] r162453 - in /cfe/trunk: lib/Analysis/CFG.cpp lib/StaticAnalyzer/Core/Environment.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/Analysis/inline.cpp

[Jordan Rose]

Author: jrose
Date: Thu Aug 23 13:10:53 2012
New Revision: 162453

URL: http://llvm.org/viewvc/llvm-project?rev=162453&view=rev
Log:
[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>

Modified:
    cfe/trunk/lib/Analysis/CFG.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
    cfe/trunk/test/Analysis/inline.cpp

Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=162453&r1=162452&r2=162453&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Thu Aug 23 13:10:53 2012
@@ -1022,6 +1022,14 @@
     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);
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp?rev=162453&r1=162452&r2=162453&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp Thu Aug 23 13:10:53 2012
@@ -99,6 +99,9 @@
       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);

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=162453&r1=162452&r2=162453&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Thu Aug 23 13:10:53 2012
@@ -529,11 +529,6 @@
       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 @@
     case Stmt::StringLiteralClass:
     case Stmt::ObjCStringLiteralClass:
     case Stmt::CXXBindTemporaryExprClass:
+    case Stmt::CXXDefaultArgExprClass:
     case Stmt::SubstNonTypeTemplateParmExprClass:
     case Stmt::CXXNullPtrLiteralExprClass: {
       Bldr.takeNodes(Pred);

Modified: cfe/trunk/test/Analysis/inline.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inline.cpp?rev=162453&r1=162452&r2=162453&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/inline.cpp (original)
+++ cfe/trunk/test/Analysis/inline.cpp Thu Aug 23 13:10:53 2012
@@ -193,3 +193,37 @@
     }
   };
 }
+
+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}}
+  }
+}