r277989 - [analyzer] Model base to derived casts more precisely.

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Mon Aug  8 04:22:59 2016
New Revision: 277989

URL: http://llvm.org/viewvc/llvm-project?rev=277989&view=rev
Log:
[analyzer] Model base to derived casts more precisely.

Dynamic casts are handled relatively well by the static analyzer.
BaseToDerived casts however are treated conservatively. This can cause some
false positives with the NewDeleteLeaks checker.

This patch alters the behavior of BaseToDerived casts. In case a dynamic cast
would succeed use the same semantics. Otherwise fall back to the conservative
approach.

Differential Revision: https://reviews.llvm.org/D23014

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
    cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
    cfe/trunk/test/Analysis/NewDelete-checker-test.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h?rev=277989&r1=277988&r2=277989&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h Mon Aug  8 04:22:59 2016
@@ -123,15 +123,18 @@ public:
   SVal evalDerivedToBase(SVal Derived, QualType DerivedPtrType,
                          bool IsVirtual);
 
-  /// \brief Evaluates C++ dynamic_cast cast.
+  /// \brief Attempts to do a down cast. Used to model BaseToDerived and C++
+  ///        dynamic_cast.
   /// The callback may result in the following 3 scenarios:
   ///  - Successful cast (ex: derived is subclass of base).
   ///  - Failed cast (ex: derived is definitely not a subclass of base).
+  ///    The distinction of this case from the next one is necessary to model
+  ///    dynamic_cast. 
   ///  - We don't know (base is a symbolic region and we don't have 
   ///    enough info to determine if the cast will succeed at run time).
   /// The function returns an SVal representing the derived class; it's
   /// valid only if Failed flag is set to false.
-  SVal evalDynamicCast(SVal Base, QualType DerivedPtrType, bool &Failed);
+  SVal attemptDownCast(SVal Base, QualType DerivedPtrType, bool &Failed);
 
   const ElementRegion *GetElementZeroRegion(const MemRegion *R, QualType T);
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=277989&r1=277988&r2=277989&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Mon Aug  8 04:22:59 2016
@@ -552,7 +552,7 @@ void CXXInstanceCall::getInitialStackFra
 
       // FIXME: CallEvent maybe shouldn't be directly accessing StoreManager.
       bool Failed;
-      ThisVal = StateMgr.getStoreManager().evalDynamicCast(ThisVal, Ty, Failed);
+      ThisVal = StateMgr.getStoreManager().attemptDownCast(ThisVal, Ty, Failed);
       assert(!Failed && "Calling an incorrectly devirtualized method");
     }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp?rev=277989&r1=277988&r2=277989&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp Mon Aug  8 04:22:59 2016
@@ -386,7 +386,7 @@ void ExprEngine::VisitCast(const CastExp
           Failed = true;
         // Else, evaluate the cast.
         else
-          val = getStoreManager().evalDynamicCast(val, T, Failed);
+          val = getStoreManager().attemptDownCast(val, T, Failed);
 
         if (Failed) {
           if (T->isReferenceType()) {
@@ -412,6 +412,28 @@ void ExprEngine::VisitCast(const CastExp
         Bldr.generateNode(CastE, Pred, state);
         continue;
       }
+      case CK_BaseToDerived: {
+        SVal val = state->getSVal(Ex, LCtx);
+        QualType resultType = CastE->getType();
+        if (CastE->isGLValue())
+          resultType = getContext().getPointerType(resultType);
+
+        bool Failed = false;
+
+        if (!val.isConstant()) {
+          val = getStoreManager().attemptDownCast(val, T, Failed);
+        }
+
+        // Failed to cast or the result is unknown, fall back to conservative.
+        if (Failed || val.isUnknown()) {
+          val =
+            svalBuilder.conjureSymbolVal(nullptr, CastE, LCtx, resultType,
+                                         currBldrCtx->blockCount());
+        }
+        state = state->BindExpr(CastE, LCtx, val);
+        Bldr.generateNode(CastE, Pred, state);
+        continue;
+      }
       case CK_NullToMemberPointer: {
         // FIXME: For now, member pointers are represented by void *.
         SVal V = svalBuilder.makeNull();
@@ -421,7 +443,6 @@ void ExprEngine::VisitCast(const CastExp
       }
       // Various C++ casts that are not handled yet.
       case CK_ToUnion:
-      case CK_BaseToDerived:
       case CK_BaseToDerivedMemberPointer:
       case CK_DerivedToBaseMemberPointer:
       case CK_ReinterpretMemberPointer:

Modified: cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp?rev=277989&r1=277988&r2=277989&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp Mon Aug  8 04:22:59 2016
@@ -292,7 +292,7 @@ static const CXXRecordDecl *getCXXRecord
   return nullptr;
 }
 
-SVal StoreManager::evalDynamicCast(SVal Base, QualType TargetType,
+SVal StoreManager::attemptDownCast(SVal Base, QualType TargetType,
                                    bool &Failed) {
   Failed = false;
 

Modified: cfe/trunk/test/Analysis/NewDelete-checker-test.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/NewDelete-checker-test.cpp?rev=277989&r1=277988&r2=277989&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/NewDelete-checker-test.cpp (original)
+++ cfe/trunk/test/Analysis/NewDelete-checker-test.cpp Mon Aug  8 04:22:59 2016
@@ -377,3 +377,19 @@ void testDoubleDeleteEmptyClass() {
   delete foo;
   delete foo;  // expected-warning {{Attempt to delete released memory}}
 }
+
+struct Base {
+  virtual ~Base() {}
+};
+
+struct Derived : Base {
+};
+
+Base *allocate() {
+  return new Derived;
+}
+
+void shouldNotReportLeak() {
+  Derived *p = (Derived *)allocate();
+  delete p;
+}