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:
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:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
