r180894 - [analyzer] Consolidate constant evaluation logic in SValBuilder.

Jordan Rose jordan_rose at apple.com
Wed May 1 16:10:44 PDT 2013


Author: jrose
Date: Wed May  1 18:10:44 2013
New Revision: 180894

URL: http://llvm.org/viewvc/llvm-project?rev=180894&view=rev
Log:
[analyzer] Consolidate constant evaluation logic in SValBuilder.

Previously, this was scattered across Environment (literal expressions),
ExprEngine (default arguments), and RegionStore (global constants). The
former special-cased several kinds of simple constant expressions, while
the latter two deferred to the AST's constant evaluator.

Now, these are all unified as SValBuilder::getConstantVal(). To keep
Environment fast, the special cases for simple constant expressions have
been left in, but the main benefits are that (a) unusual constants like
ObjCStringLiterals now work as default arguments and global constant
initializers, and (b) we're not duplicating code between ExprEngine and
RegionStore.

This actually caught a bug in our test suite, which is awesome: we stop
tracking allocated memory if it's passed as an argument along with some
kind of callback, but not if the callback is 0. We were testing this in
a case where the callback parameter had a default value, but that value
was 0. After this change, the analyzer now (correctly) flags that as a
leak!

<rdar://problem/13773117>

Added:
    cfe/trunk/test/Analysis/objc-string.mm
Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
    cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
    cfe/trunk/test/Analysis/malloc.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h?rev=180894&r1=180893&r2=180894&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h Wed May  1 18:10:44 2013
@@ -202,6 +202,15 @@ public:
   DefinedSVal getBlockPointer(const BlockDecl *block, CanQualType locTy,
                               const LocationContext *locContext);
 
+  /// Returns the value of \p E, if it can be determined in a non-path-sensitive
+  /// manner.
+  ///
+  /// If \p E is not a constant or cannot be modeled, returns \c None.
+  ///
+  /// Note that this function always treats \p E as a prvalue. Callers should
+  /// check to see if \p E is a glvalue and modify their behavior accordingly.
+  Optional<SVal> getConstantVal(const Expr *E);
+
   NonLoc makeCompoundVal(QualType type, llvm::ImmutableList<SVal> vals) {
     return nonloc::CompoundVal(BasicVals.getCompoundValData(type, vals));
   }

Modified: cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp?rev=180894&r1=180893&r2=180894&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp Wed May  1 18:10:44 2013
@@ -80,43 +80,17 @@ SVal Environment::getSVal(const Environm
     llvm_unreachable("Should have been handled by ignoreTransparentExprs");
 
   case Stmt::AddrLabelExprClass:
-    return svalBuilder.makeLoc(cast<AddrLabelExpr>(S));
-
-  case Stmt::CharacterLiteralClass: {
-    const CharacterLiteral *C = cast<CharacterLiteral>(S);
-    return svalBuilder.makeIntVal(C->getValue(), C->getType());
-  }
-
+  case Stmt::CharacterLiteralClass:
   case Stmt::CXXBoolLiteralExprClass:
-    return svalBuilder.makeBoolVal(cast<CXXBoolLiteralExpr>(S));
-
   case Stmt::CXXScalarValueInitExprClass:
-  case Stmt::ImplicitValueInitExprClass: {
-    QualType Ty = cast<Expr>(S)->getType();
-    return svalBuilder.makeZeroVal(Ty);
-  }
-
+  case Stmt::ImplicitValueInitExprClass:
   case Stmt::IntegerLiteralClass:
-    return svalBuilder.makeIntVal(cast<IntegerLiteral>(S));
-
   case Stmt::ObjCBoolLiteralExprClass:
-    return svalBuilder.makeBoolVal(cast<ObjCBoolLiteralExpr>(S));
-
-  // For special C0xx nullptr case, make a null pointer SVal.
   case Stmt::CXXNullPtrLiteralExprClass:
-    return svalBuilder.makeNull();
-
-  case Stmt::ObjCStringLiteralClass: {
-    MemRegionManager &MRMgr = svalBuilder.getRegionManager();
-    const ObjCStringLiteral *SL = cast<ObjCStringLiteral>(S);
-    return svalBuilder.makeLoc(MRMgr.getObjCStringRegion(SL));
-  }
-
-  case Stmt::StringLiteralClass: {
-    MemRegionManager &MRMgr = svalBuilder.getRegionManager();
-    const StringLiteral *SL = cast<StringLiteral>(S);
-    return svalBuilder.makeLoc(MRMgr.getStringRegion(SL));
-  }
+  case Stmt::ObjCStringLiteralClass:
+  case Stmt::StringLiteralClass:
+    // Known constants; defer to SValBuilder.
+    return svalBuilder.getConstantVal(cast<Expr>(S)).getValue();
 
   case Stmt::ReturnStmtClass: {
     const ReturnStmt *RS = cast<ReturnStmt>(S);
@@ -127,10 +101,8 @@ SVal Environment::getSVal(const Environm
     
   // Handle all other Stmt* using a lookup.
   default:
-    break;
+    return lookupExpr(EnvironmentEntry(S, LCtx));
   }
-  
-  return lookupExpr(EnvironmentEntry(S, LCtx));
 }
 
 Environment EnvironmentManager::bindExpr(Environment Env,

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=180894&r1=180893&r2=180894&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Wed May  1 18:10:44 2013
@@ -741,20 +741,14 @@ void ExprEngine::Visit(const Stmt *S, Ex
       const CXXDefaultArgExpr *DefaultE = cast<CXXDefaultArgExpr>(S);
       const Expr *ArgE = DefaultE->getExpr();
 
-      // Avoid creating and destroying a lot of APSInts.
-      SVal V;
-      llvm::APSInt Result;
+      Optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE);
+      if (!ConstantVal)
+        ConstantVal = UnknownVal();
 
       for (ExplodedNodeSet::iterator I = PreVisit.begin(), E = PreVisit.end();
            I != E; ++I) {
         ProgramStateRef State = (*I)->getState();
-
-        if (ArgE->EvaluateAsInt(Result, getContext()))
-          V = svalBuilder.makeIntVal(Result);
-        else
-          V = State->getSVal(ArgE, LCtx);
-
-        State = State->BindExpr(DefaultE, LCtx, V);
+        State = State->BindExpr(DefaultE, LCtx, *ConstantVal);
         if (DefaultE->isGLValue())
           State = createTemporaryRegionIfNeeded(State, LCtx, DefaultE,
                                                 DefaultE);

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=180894&r1=180893&r2=180894&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Wed May  1 18:10:44 2013
@@ -1755,26 +1755,6 @@ SVal RegionStoreManager::getBindingForOb
   return getBindingForLazySymbol(R);
 }
 
-static Optional<SVal> getConstValue(SValBuilder &SVB, const VarDecl *VD) {
-  ASTContext &Ctx = SVB.getContext();
-  if (!VD->getType().isConstQualified())
-    return None;
-
-  const Expr *Init = VD->getInit();
-  if (!Init)
-    return None;
-
-  llvm::APSInt Result;
-  if (!Init->isGLValue() && Init->EvaluateAsInt(Result, Ctx))
-    return SVB.makeIntVal(Result);
-
-  if (Init->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNotNull))
-    return SVB.makeNull();
-
-  // FIXME: Handle other possible constant expressions.
-  return None;
-}
-
 SVal RegionStoreManager::getBindingForVar(RegionBindingsConstRef B,
                                           const VarRegion *R) {
 
@@ -1791,8 +1771,10 @@ SVal RegionStoreManager::getBindingForVa
     return svalBuilder.getRegionValueSymbolVal(R);
 
   // Is 'VD' declared constant?  If so, retrieve the constant value.
-  if (Optional<SVal> V = getConstValue(svalBuilder, VD))
-    return *V;
+  if (VD->getType().isConstQualified())
+    if (const Expr *Init = VD->getInit())
+      if (Optional<SVal> V = svalBuilder.getConstantVal(Init))
+        return *V;
 
   // This must come after the check for constants because closure-captured
   // constant variables may appear in UnknownSpaceRegion.

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp?rev=180894&r1=180893&r2=180894&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp Wed May  1 18:10:44 2013
@@ -224,6 +224,63 @@ loc::MemRegionVal SValBuilder::getCXXThi
   return loc::MemRegionVal(getRegionManager().getCXXThisRegion(PT, SFC));
 }
 
+Optional<SVal> SValBuilder::getConstantVal(const Expr *E) {
+  E = E->IgnoreParens();
+
+  switch (E->getStmtClass()) {
+  // Handle expressions that we treat differently from the AST's constant
+  // evaluator.
+  case Stmt::AddrLabelExprClass:
+    return makeLoc(cast<AddrLabelExpr>(E));
+
+  case Stmt::CXXScalarValueInitExprClass:
+  case Stmt::ImplicitValueInitExprClass:
+    return makeZeroVal(E->getType());
+
+  case Stmt::ObjCStringLiteralClass: {
+    const ObjCStringLiteral *SL = cast<ObjCStringLiteral>(E);
+    return makeLoc(getRegionManager().getObjCStringRegion(SL));
+  }
+
+  case Stmt::StringLiteralClass: {
+    const StringLiteral *SL = cast<StringLiteral>(E);
+    return makeLoc(getRegionManager().getStringRegion(SL));
+  }
+
+  // Fast-path some expressions to avoid the overhead of going through the AST's
+  // constant evaluator
+  case Stmt::CharacterLiteralClass: {
+    const CharacterLiteral *C = cast<CharacterLiteral>(E);
+    return makeIntVal(C->getValue(), C->getType());
+  }
+
+  case Stmt::CXXBoolLiteralExprClass:
+    return makeBoolVal(cast<CXXBoolLiteralExpr>(E));
+
+  case Stmt::IntegerLiteralClass:
+    return makeIntVal(cast<IntegerLiteral>(E));
+
+  case Stmt::ObjCBoolLiteralExprClass:
+    return makeBoolVal(cast<ObjCBoolLiteralExpr>(E));
+
+  case Stmt::CXXNullPtrLiteralExprClass:
+    return makeNull();
+
+  // If we don't have a special case, fall back to the AST's constant evaluator.
+  default: {
+    ASTContext &Ctx = getContext();
+    llvm::APSInt Result;
+    if (E->EvaluateAsInt(Result, Ctx))
+      return makeIntVal(Result);
+
+    if (E->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNotNull))
+      return makeNull();
+
+    return None;
+  }
+  }
+}
+
 //===----------------------------------------------------------------------===//
 
 SVal SValBuilder::makeSymExprValNN(ProgramStateRef State,

Modified: cfe/trunk/test/Analysis/malloc.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.cpp?rev=180894&r1=180893&r2=180894&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/malloc.cpp (original)
+++ cfe/trunk/test/Analysis/malloc.cpp Wed May  1 18:10:44 2013
@@ -24,12 +24,18 @@ Foo aFunction() {
 // they are defined in system headers and take the const pointer to the
 // allocated memory. (radar://11160612)
 // Test default parameter.
-int const_ptr_and_callback_def_param(int, const char*, int n, void(*)(void*) = 0);
+int const_ptr_and_callback_def_param(int, const char*, int n, void(*)(void*) = free);
 void r11160612_3() {
   char *x = (char*)malloc(12);
   const_ptr_and_callback_def_param(0, x, 12);
 }
 
+int const_ptr_and_callback_def_param_null(int, const char*, int n, void(*)(void*) = 0);
+void r11160612_no_callback() {
+  char *x = (char*)malloc(12);
+  const_ptr_and_callback_def_param_null(0, x, 12);
+} // expected-warning{{leak}}
+
 // Test member function pointer.
 struct CanFreeMemory {
   static void myFree(void*);

Added: cfe/trunk/test/Analysis/objc-string.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/objc-string.mm?rev=180894&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/objc-string.mm (added)
+++ cfe/trunk/test/Analysis/objc-string.mm Wed May  1 18:10:44 2013
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(bool);
+ at class NSString;
+
+void sanity() {
+  clang_analyzer_eval(@""); // expected-warning{{TRUE}}
+  clang_analyzer_eval(@"abc"); // expected-warning{{TRUE}}
+}
+
+namespace rdar13773117 {
+  NSString *const kConstantGlobalString = @"foo";
+  NSString *globalString = @"bar";
+
+  extern void invalidateGlobals();
+
+  void testGlobals() {
+    clang_analyzer_eval(kConstantGlobalString); // expected-warning{{TRUE}}
+    clang_analyzer_eval(globalString); // expected-warning{{UNKNOWN}}
+
+    globalString = @"baz";
+    clang_analyzer_eval(globalString); // expected-warning{{TRUE}}
+
+    invalidateGlobals();
+
+    clang_analyzer_eval(kConstantGlobalString); // expected-warning{{TRUE}}
+    clang_analyzer_eval(globalString); // expected-warning{{UNKNOWN}}
+  }
+
+  NSString *returnString(NSString *input = @"garply") {
+    return input;
+  }
+
+  void testDefaultArg() {
+    clang_analyzer_eval(returnString(@"")); // expected-warning{{TRUE}}
+    clang_analyzer_eval(returnString(0)); // expected-warning{{FALSE}}
+    clang_analyzer_eval(returnString()); // expected-warning{{TRUE}}
+  }
+}





More information about the cfe-commits mailing list