]> granicus.if.org Git - clang/commitdiff
[analyzer] Consolidate constant evaluation logic in SValBuilder.
authorJordan Rose <jordan_rose@apple.com>
Wed, 1 May 2013 23:10:44 +0000 (23:10 +0000)
committerJordan Rose <jordan_rose@apple.com>
Wed, 1 May 2013 23:10:44 +0000 (23:10 +0000)
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>

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

include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
lib/StaticAnalyzer/Core/Environment.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/RegionStore.cpp
lib/StaticAnalyzer/Core/SValBuilder.cpp
test/Analysis/malloc.cpp
test/Analysis/objc-string.mm [new file with mode: 0644]

index 4a4b37a78612ed9175f28a4ac389208ac321f926..713a6d593302918e948e6fcae5708cc9569a3350 100644 (file)
@@ -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));
   }
index fe352aa8b4cf5e7b643a615e5f78fe3847d8dfdd..7b133f6bf645fa3507a062eb150dfe9a09f277f7 100644 (file)
@@ -80,43 +80,17 @@ SVal Environment::getSVal(const EnvironmentEntry &Entry,
     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 EnvironmentEntry &Entry,
     
   // Handle all other Stmt* using a lookup.
   default:
-    break;
+    return lookupExpr(EnvironmentEntry(S, LCtx));
   }
-  
-  return lookupExpr(EnvironmentEntry(S, LCtx));
 }
 
 Environment EnvironmentManager::bindExpr(Environment Env,
index 5b6e97d3fb49d7a46902e9994a1a10aaf1c2f3eb..0d5fb1785b338af520971b5cb83fab6c564f40ef 100644 (file)
@@ -741,20 +741,14 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       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);
index dbfd9d6c9907b2e12f925fe50107754407463cff..3053e3610c89083610caf60f7b2caff03f71042e 100644 (file)
@@ -1755,26 +1755,6 @@ SVal RegionStoreManager::getBindingForObjCIvar(RegionBindingsConstRef B,
   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::getBindingForVar(RegionBindingsConstRef B,
     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.
index 5bab986ed05a79422579454fd051ef0124f8546c..652809777cfdc33a55d8409d6e4a742b0b5f8d0d 100644 (file)
@@ -224,6 +224,63 @@ loc::MemRegionVal SValBuilder::getCXXThis(const CXXRecordDecl *D,
   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,
index 54efa1c2bd794c418c595d8945abf9b16ba0a602..75d06d66c2c3eb870ed0eb7e01d6c80bc1dd51ee 100644 (file)
@@ -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*);
diff --git a/test/Analysis/objc-string.mm b/test/Analysis/objc-string.mm
new file mode 100644 (file)
index 0000000..c67ab5e
--- /dev/null
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(bool);
+@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}}
+  }
+}