On 06.04.2013 0:30, Jordan Rose wrote:
On Apr 5, 2013, at 13:22 , Anton Yartsev <[email protected]> wrote:
On 05.04.2013 21:55, Jordan Rose wrote:
if (!Filter.CMallocOptimistic && !Filter.CMallocPessimistic &&
- !Filter.CNewDeleteChecker)
+ !Filter.CNewDeleteLeaksChecker)
return;
- if (!isTrackedFamily(C, Sym))
+ const RefState *RS = C.getState()->get<RegionState>(Sym);
+ assert(RS && "cannot leak an untracked symbol");
+ AllocationFamily Family = RS->getAllocationFamily();
+ if (!isTrackedFamily(Family))
return;
+ // Special case for new and new[]; these are controlled by a separate
checker
+ // flag so that they can be selectively disabled.
+ if (Family == AF_CXXNew || Family == AF_CXXNewArray)
+ if (!Filter.CNewDeleteLeaksChecker)
+ return;
We already checked for Filter.CNewDeleteLeaksChecker at the beginning.
Now this works properly just because isTrackedFamily() does not know about
CNewDeleteLeaksChecker and returns false.
What about adding bug types at this point?
The first check says "any of
{MallocOptimistic,MallocPessimistic,NewDeleteLeaks}". This one checks exclusively
for NewDeleteLeaks and AF_CXXNew[Array].
Oops, sorry.
This is basically a quick-and-dirty "bug types" implementation that's used because today
we only have one bug type that's different. I'm not sure the added generalization will actually
make things clearer—here we implicitly know the "type" is a leak because we're in
reportLeak. None of the rest of the code has to know that leaks are any different from anything
else. But maybe I'm missing some benefit of the general bug types.
No benefits other then just looks clearer to me :) Passing a bug kind
shows, on the one hand, that sometimes we depend on the bug type, and
allows to freely add other bug-dependent code on the other.
I quickly returned the bug types while waited for your replay, just to
see, if it simplify the appearance, attached the patch so you could also
see how it looks like.
I am under no circumstances insist on adding bug types, sending just for
information, perhaps you like it. :)
In any case, these may be slightly simplified as isTrackedFamily() now
treats AF_None as unacceptable (after r178899 )
I'll admit it's a bit kludgy that the family is effectively checked twice, but I actually
like the implication that alpha.cplusplus.NewDeleteLeaks is a "sub-checker" of
cplusplus.NewDelete.
Jordan
--
Anton
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp (revision 178899)
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp (working copy)
@@ -145,6 +145,15 @@
check::Location,
eval::Assume>
{
+ enum BugKind {
+ BK_DoubleFree,
+ BK_Leak,
+ BK_UseFree,
+ BK_BadFree,
+ BK_MismatchedDealloc,
+ BK_OffsetFree
+ };
+
mutable OwningPtr<BugType> BT_DoubleFree;
mutable OwningPtr<BugType> BT_Leak;
mutable OwningPtr<BugType> BT_UseFree;
@@ -285,9 +294,10 @@
// Used to suppress warnings if they are not related to the tracked family
// (derived from Sym or AllocDeallocStmt).
- bool isTrackedFamily(AllocationFamily Family) const;
- bool isTrackedFamily(CheckerContext &C, const Stmt *AllocDeallocStmt) const;
- bool isTrackedFamily(CheckerContext &C, SymbolRef Sym) const;
+ bool isTrackedFamily(BugKind Kind, AllocationFamily Family) const;
+ bool isTrackedFamily(BugKind Kind, CheckerContext &C,
+ const Stmt *AllocDeallocStmt) const;
+ bool isTrackedFamily(BugKind Kind, CheckerContext &C, SymbolRef Sym) const;
static bool SummarizeValue(raw_ostream &os, SVal V);
static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR);
@@ -1070,7 +1080,8 @@
RefState::getReleased(Family, ParentExpr));
}
-bool MallocChecker::isTrackedFamily(AllocationFamily Family) const {
+bool
+MallocChecker::isTrackedFamily(BugKind Kind, AllocationFamily Family) const {
switch (Family) {
case AF_Malloc: {
if (!Filter.CMallocOptimistic && !Filter.CMallocPessimistic)
@@ -1079,8 +1090,12 @@
}
case AF_CXXNew:
case AF_CXXNewArray: {
- if (!Filter.CNewDeleteChecker)
+ if (!Filter.CNewDeleteChecker && !Filter.CNewDeleteLeaksChecker)
return false;
+
+ if (Kind == BK_Leak && !Filter.CNewDeleteLeaksChecker)
+ return false;
+
return true;
}
case AF_None: {
@@ -1090,16 +1105,16 @@
llvm_unreachable("unhandled family");
}
-bool MallocChecker::isTrackedFamily(CheckerContext &C,
+bool MallocChecker::isTrackedFamily(BugKind Kind, CheckerContext &C,
const Stmt *AllocDeallocStmt) const {
- return isTrackedFamily(getAllocationFamily(C, AllocDeallocStmt));
+ return isTrackedFamily(Kind, getAllocationFamily(C, AllocDeallocStmt));
}
-bool MallocChecker::isTrackedFamily(CheckerContext &C, SymbolRef Sym) const {
-
+bool MallocChecker::isTrackedFamily(BugKind Kind, CheckerContext &C,
+ SymbolRef Sym) const {
const RefState *RS = C.getState()->get<RegionState>(Sym);
assert(RS);
- return isTrackedFamily(RS->getAllocationFamily());
+ return isTrackedFamily(Kind, RS->getAllocationFamily());
}
bool MallocChecker::SummarizeValue(raw_ostream &os, SVal V) {
@@ -1197,7 +1212,7 @@
!Filter.CNewDeleteChecker)
return;
- if (!isTrackedFamily(C, DeallocExpr))
+ if (!isTrackedFamily(BK_BadFree, C, DeallocExpr))
return;
if (ExplodedNode *N = C.generateSink()) {
@@ -1285,7 +1300,7 @@
!Filter.CNewDeleteChecker)
return;
- if (!isTrackedFamily(C, AllocExpr))
+ if (!isTrackedFamily(BK_OffsetFree, C, AllocExpr))
return;
ExplodedNode *N = C.generateSink();
@@ -1337,7 +1352,7 @@
!Filter.CNewDeleteChecker)
return;
- if (!isTrackedFamily(C, Sym))
+ if (!isTrackedFamily(BK_UseFree, C, Sym))
return;
if (ExplodedNode *N = C.generateSink()) {
@@ -1362,7 +1377,7 @@
!Filter.CNewDeleteChecker)
return;
- if (!isTrackedFamily(C, Sym))
+ if (!isTrackedFamily(BK_DoubleFree, C, Sym))
return;
if (ExplodedNode *N = C.generateSink()) {
@@ -1541,18 +1556,9 @@
!Filter.CNewDeleteLeaksChecker)
return;
- const RefState *RS = C.getState()->get<RegionState>(Sym);
- assert(RS && "cannot leak an untracked symbol");
- AllocationFamily Family = RS->getAllocationFamily();
- if (!isTrackedFamily(Family))
+ if (!isTrackedFamily(BK_Leak, C, Sym))
return;
- // Special case for new and new[]; these are controlled by a separate checker
- // flag so that they can be selectively disabled.
- if (Family == AF_CXXNew || Family == AF_CXXNewArray)
- if (!Filter.CNewDeleteLeaksChecker)
- return;
-
assert(N);
if (!BT_Leak) {
BT_Leak.reset(new BugType("Memory leak", "Memory Error"));
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
