[PATCH] D23014: [analyzer] Model base to derived casts more precisely.

[Gábor Horváth via cfe-commits]

xazax.hun created this revision.
xazax.hun added reviewers: zaks.anna, dcoughlin, dergachev.a, a.sidorin.
xazax.hun added a subscriber: cfe-commits.

Dynamic casts are handled relatively well by the static analyzer. BaseToDerived casts however are handled 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.

A slightly related question: in case a cast on a pointer can not be modelled precisely, maybe that should be handled as a pointer escape to avoid false positives in some cases?

https://reviews.llvm.org/D23014

Files:
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  test/Analysis/NewDelete-checker-test.cpp

Index: test/Analysis/NewDelete-checker-test.cpp
===================================================================
--- test/Analysis/NewDelete-checker-test.cpp
+++ test/Analysis/NewDelete-checker-test.cpp
@@ -377,3 +377,19 @@
   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;
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -412,6 +412,28 @@
         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.isZeroConstant()) {
+          val = getStoreManager().evalDynamicCast(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 @@
       }
       // Various C++ casts that are not handled yet.
       case CK_ToUnion:
-      case CK_BaseToDerived:
       case CK_BaseToDerivedMemberPointer:
       case CK_DerivedToBaseMemberPointer:
       case CK_ReinterpretMemberPointer:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D23014.66315.patch
Type: text/x-patch
Size: 2045 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160801/2b30273d/attachment-0001.bin>