Hi Anna,
On ons, 2014-09-24 at 10:02 -0700, Anna Zaks wrote:
> How about the similar functions from the malloc family:
> isAllocationFunction and isFreeFunction?
>
> You could either introduce a helper function which checks if the
> FunctionDecl declares a function from the given list of identifiers or
> introduce a function that takes FunctionDecl, ASTContext, familyKind,
> and MemoryOperationKind (enum class MemoryOperationKind { MOK_Allocate,
> MOK_Free };) and checks if the FunctionDecl belongs to that family and
> memory operation. (The second approach is probably better.)
Yes, the second approach does sound better. Combining the four functions
into one required a bit if thinking in order to maintain readability.
Attached is a patch that refactors isAllocationFunction and
isFreeFunction, as well as adding support for if_*nameindex functions.
Any comments?
Cheers,
Daniel
Index: test/Analysis/ifnameindex.c
===================================================================
--- test/Analysis/ifnameindex.c (revision 0)
+++ test/Analysis/ifnameindex.c (working copy)
@@ -0,0 +1,133 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,unix.MismatchedDeallocator -analyzer-store=region -verify %s
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+void *valloc(size_t);
+void free(void *);
+void *realloc(void *ptr, size_t size);
+void *reallocf(void *ptr, size_t size);
+void *calloc(size_t nmemb, size_t size);
+char *strdup(const char *s);
+char *strndup(const char *s, size_t n);
+
+struct if_nameindex;
+struct if_nameindex *if_nameindex(void);
+void if_freenameindex(struct if_nameindex *ptr);
+
+void f1() {
+ struct if_nameindex *p;
+ p = if_nameindex();
+ if (p)
+ if_freenameindex(p); // no-warning
+}
+
+void f2() {
+ struct if_nameindex *p;
+ p = if_nameindex();
+ (void) p;
+ return; // expected-warning{{Potential leak of memory pointed to by 'p'}}
+}
+
+void f3() {
+ struct if_nameindex *p;
+ p = if_nameindex();
+ (void) p;
+ p = if_nameindex();
+ if (p)
+ if_freenameindex(p);
+ return; // expected-warning{{Potential leak of memory pointed to by 'p'}}
+}
+
+void f4() {
+ struct if_nameindex *p;
+ p = if_nameindex();
+ if (p) {
+ if_freenameindex(p);
+ if_freenameindex(p); // expected-warning{{Attempt to free released memory}}
+ }
+}
+
+void bar(const struct if_nameindex *p);
+
+void f5() {
+ struct if_nameindex *p;
+ p = if_nameindex();
+ if (p) {
+ if_freenameindex(p);
+ bar(p); // expected-warning{{Use of memory after it is freed}}
+ }
+}
+
+void f6() {
+ struct if_nameindex *p;
+ p = if_nameindex();
+ if (p)
+ free(p); // expected-warning{{Memory allocated by if_nameindex() should be deallocated by 'if_freenameindex()', not free()}}
+}
+
+void f7() {
+ struct if_nameindex *p;
+ void *p2;
+ p = if_nameindex();
+ if (p)
+ p2 = realloc(p, 1); // expected-warning{{Memory allocated by if_nameindex() should be deallocated by 'if_freenameindex()', not realloc()}}
+ (void) p2;
+}
+
+void f8() {
+ struct if_nameindex *p;
+ void *p2;
+ p = if_nameindex();
+ if (p)
+ p2 = reallocf(p, 1); // expected-warning{{Memory allocated by if_nameindex() should be deallocated by 'if_freenameindex()', not reallocf()}}
+ (void) p2;
+}
+
+void f9() {
+ struct if_nameindex *p;
+ p = malloc(100);
+ if (p)
+ if_freenameindex(p); // expected-warning{{Memory allocated by malloc() should be deallocated by free(), not if_freenameindex()}}
+}
+
+void f10() {
+ struct if_nameindex *p;
+ p = valloc(100);
+ if (p)
+ if_freenameindex(p); // expected-warning{{Memory allocated by valloc() should be deallocated by free(), not if_freenameindex()}}
+}
+
+void f11() {
+ struct if_nameindex *p;
+ p = calloc(1, 100);
+ if (p)
+ if_freenameindex(p); // expected-warning{{Memory allocated by calloc() should be deallocated by free(), not if_freenameindex()}}
+}
+
+void f12(void *p) {
+ void *p2;
+ p2 = realloc(p, 1);
+ if (p2)
+ if_freenameindex(p2); // expected-warning{{Memory allocated by realloc() should be deallocated by free(), not if_freenameindex()}}
+}
+
+void f13(void *p) {
+ void *p2;
+ p2 = reallocf(p, 1);
+ if (p2)
+ if_freenameindex(p2); // expected-warning{{Memory allocated by reallocf() should be deallocated by free(), not if_freenameindex()}}
+}
+
+void f14(const char *str) {
+ struct if_nameindex *p;
+ p = (struct if_nameindex *) strdup(str);
+ if (p)
+ if_freenameindex(p); // expected-warning{{Memory allocated by strdup() should be deallocated by free(), not if_freenameindex()}}
+}
+
+void f15(const char *str) {
+ struct if_nameindex *p;
+ p = (struct if_nameindex *) strndup(str, 1);
+ if (p)
+ if_freenameindex(p); //expected-warning{{Memory allocated by strndup() should be deallocated by free(), not if_freenameindex()}}
+}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp (revision 218827)
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp (working copy)
@@ -42,7 +42,8 @@ enum AllocationFamily {
AF_None,
AF_Malloc,
AF_CXXNew,
- AF_CXXNewArray
+ AF_CXXNewArray,
+ AF_IfNameIndex
};
class RefState {
@@ -161,7 +162,8 @@ public:
MallocChecker()
: II_malloc(nullptr), II_free(nullptr), II_realloc(nullptr),
II_calloc(nullptr), II_valloc(nullptr), II_reallocf(nullptr),
- II_strndup(nullptr), II_strdup(nullptr), II_kmalloc(nullptr) {}
+ II_strndup(nullptr), II_strdup(nullptr), II_kmalloc(nullptr),
+ II_if_nameindex(nullptr), II_if_freenameindex(nullptr) {}
/// In pessimistic mode, the checker assumes that it does not know which
/// functions might free the memory.
@@ -174,6 +176,13 @@ public:
CK_NumCheckKinds
};
+ enum class MemoryOperationKind {
+ MOK_None,
+ MOK_Allocate,
+ MOK_Free,
+ MOK_Any
+ };
+
DefaultBool ChecksEnabled[CK_NumCheckKinds];
CheckName CheckNames[CK_NumCheckKinds];
@@ -212,7 +221,7 @@ private:
mutable std::unique_ptr<BugType> BT_OffsetFree[CK_NumCheckKinds];
mutable IdentifierInfo *II_malloc, *II_free, *II_realloc, *II_calloc,
*II_valloc, *II_reallocf, *II_strndup, *II_strdup,
- *II_kmalloc;
+ *II_kmalloc, *II_if_nameindex, *II_if_freenameindex;
mutable Optional<uint64_t> KernelZeroFlagVal;
void initIdentifierInfo(ASTContext &C) const;
@@ -238,8 +247,10 @@ private:
/// Check if this is one of the functions which can allocate/reallocate memory
/// pointed to by one of its arguments.
bool isMemFunction(const FunctionDecl *FD, ASTContext &C) const;
- bool isFreeFunction(const FunctionDecl *FD, ASTContext &C) const;
- bool isAllocationFunction(const FunctionDecl *FD, ASTContext &C) const;
+ bool isCMemFunction(const FunctionDecl *FD,
+ ASTContext &C,
+ AllocationFamily Family,
+ enum MemoryOperationKind) const;
bool isStandardNewDelete(const FunctionDecl *FD, ASTContext &C) const;
///@}
ProgramStateRef MallocMemReturnsAttr(CheckerContext &C,
@@ -496,13 +507,15 @@ void MallocChecker::initIdentifierInfo(A
II_strdup = &Ctx.Idents.get("strdup");
II_strndup = &Ctx.Idents.get("strndup");
II_kmalloc = &Ctx.Idents.get("kmalloc");
+ II_if_nameindex = &Ctx.Idents.get("if_nameindex");
+ II_if_freenameindex = &Ctx.Idents.get("if_freenameindex");
}
bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext &C) const {
- if (isFreeFunction(FD, C))
+ if (isCMemFunction(FD, C, AF_Malloc, MemoryOperationKind::MOK_Any))
return true;
- if (isAllocationFunction(FD, C))
+ if (isCMemFunction(FD, C, AF_IfNameIndex, MemoryOperationKind::MOK_Any))
return true;
if (isStandardNewDelete(FD, C))
@@ -511,45 +524,61 @@ bool MallocChecker::isMemFunction(const
return false;
}
-bool MallocChecker::isAllocationFunction(const FunctionDecl *FD,
- ASTContext &C) const {
+bool MallocChecker::isCMemFunction(const FunctionDecl *FD,
+ ASTContext &C,
+ AllocationFamily Family,
+ enum MemoryOperationKind MemKind) const {
if (!FD)
return false;
+ bool CheckFree = (MemKind == MemoryOperationKind::MOK_Any ||
+ MemKind == MemoryOperationKind::MOK_Free);
+ bool CheckAlloc = (MemKind == MemoryOperationKind::MOK_Any ||
+ MemKind == MemoryOperationKind::MOK_Allocate);
+
if (FD->getKind() == Decl::Function) {
- IdentifierInfo *FunI = FD->getIdentifier();
+ const IdentifierInfo *FunI = FD->getIdentifier();
initIdentifierInfo(C);
- if (FunI == II_malloc || FunI == II_realloc ||
- FunI == II_reallocf || FunI == II_calloc || FunI == II_valloc ||
- FunI == II_strdup || FunI == II_strndup || FunI == II_kmalloc)
- return true;
- }
+ if (Family == AF_Malloc && CheckFree) {
+ if (FunI == II_free || FunI == II_realloc || FunI == II_reallocf)
+ return true;
+ }
- if (ChecksEnabled[CK_MallocOptimistic] && FD->hasAttrs())
- for (const auto *I : FD->specific_attrs<OwnershipAttr>())
- if (I->getOwnKind() == OwnershipAttr::Returns)
+ if (Family == AF_Malloc && CheckAlloc) {
+ if (FunI == II_malloc || FunI == II_realloc || FunI == II_reallocf ||
+ FunI == II_calloc || FunI == II_valloc || FunI == II_strdup ||
+ FunI == II_strndup || FunI == II_kmalloc)
return true;
- return false;
-}
+ }
-bool MallocChecker::isFreeFunction(const FunctionDecl *FD, ASTContext &C) const {
- if (!FD)
- return false;
+ if (Family == AF_IfNameIndex && CheckFree) {
+ if (FunI == II_if_freenameindex)
+ return true;
+ }
- if (FD->getKind() == Decl::Function) {
- IdentifierInfo *FunI = FD->getIdentifier();
- initIdentifierInfo(C);
+ if (Family == AF_IfNameIndex && CheckAlloc) {
+ if (FunI == II_if_nameindex)
+ return true;
+ }
+ }
- if (FunI == II_free || FunI == II_realloc || FunI == II_reallocf)
- return true;
+ if (Family != AF_Malloc)
+ return false;
+
+ if (ChecksEnabled[CK_MallocOptimistic] && FD->hasAttrs()) {
+ for (const auto *I : FD->specific_attrs<OwnershipAttr>()) {
+ OwnershipAttr::OwnershipKind OwnKind = I->getOwnKind();
+ if(OwnKind == OwnershipAttr::Takes || OwnKind == OwnershipAttr::Holds) {
+ if (CheckFree)
+ return true;
+ } else if (OwnKind == OwnershipAttr::Returns) {
+ if (CheckAlloc)
+ return true;
+ }
+ }
}
- if (ChecksEnabled[CK_MallocOptimistic] && FD->hasAttrs())
- for (const auto *I : FD->specific_attrs<OwnershipAttr>())
- if (I->getOwnKind() == OwnershipAttr::Takes ||
- I->getOwnKind() == OwnershipAttr::Holds)
- return true;
return false;
}
@@ -732,6 +761,13 @@ void MallocChecker::checkPostStmt(const
State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
else
llvm_unreachable("not a new/delete operator");
+ } else if (FunI == II_if_nameindex) {
+ // Should we model this differently? We can allocate a fixed number of
+ // elements with zeros in the last one.
+ State = MallocMemAux(C, CE, UnknownVal(), UnknownVal(), State,
+ AF_IfNameIndex);
+ } else if (FunI == II_if_freenameindex) {
+ State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
}
}
@@ -1016,7 +1052,7 @@ AllocationFamily MallocChecker::getAlloc
ASTContext &Ctx = C.getASTContext();
- if (isAllocationFunction(FD, Ctx) || isFreeFunction(FD, Ctx))
+ if (isCMemFunction(FD, Ctx, AF_Malloc, MemoryOperationKind::MOK_Any))
return AF_Malloc;
if (isStandardNewDelete(FD, Ctx)) {
@@ -1027,6 +1063,9 @@ AllocationFamily MallocChecker::getAlloc
return AF_CXXNewArray;
}
+ if (isCMemFunction(FD, Ctx, AF_IfNameIndex, MemoryOperationKind::MOK_Any))
+ return AF_IfNameIndex;
+
return AF_None;
}
@@ -1090,6 +1129,7 @@ void MallocChecker::printExpectedAllocNa
case AF_Malloc: os << "malloc()"; return;
case AF_CXXNew: os << "'new'"; return;
case AF_CXXNewArray: os << "'new[]'"; return;
+ case AF_IfNameIndex: os << "'if_nameindex()'"; return;
case AF_None: llvm_unreachable("not a deallocation expression");
}
}
@@ -1100,6 +1140,7 @@ void MallocChecker::printExpectedDealloc
case AF_Malloc: os << "free()"; return;
case AF_CXXNew: os << "'delete'"; return;
case AF_CXXNewArray: os << "'delete[]'"; return;
+ case AF_IfNameIndex: os << "'if_freenameindex()'"; return;
case AF_None: llvm_unreachable("suspicious AF_None argument");
}
}
@@ -1243,7 +1284,8 @@ ProgramStateRef MallocChecker::FreeMemAu
Optional<MallocChecker::CheckKind>
MallocChecker::getCheckIfTracked(AllocationFamily Family) const {
switch (Family) {
- case AF_Malloc: {
+ case AF_Malloc:
+ case AF_IfNameIndex: {
if (ChecksEnabled[CK_MallocOptimistic]) {
return CK_MallocOptimistic;
} else if (ChecksEnabled[CK_MallocPessimistic]) {
@@ -1907,13 +1949,16 @@ void MallocChecker::checkPreCall(const C
if (!FD)
return;
+ ASTContext &Ctx = C.getASTContext();
if ((ChecksEnabled[CK_MallocOptimistic] ||
ChecksEnabled[CK_MallocPessimistic]) &&
- isFreeFunction(FD, C.getASTContext()))
+ (isCMemFunction(FD, Ctx, AF_Malloc, MemoryOperationKind::MOK_Free) ||
+ isCMemFunction(FD, Ctx, AF_IfNameIndex,
+ MemoryOperationKind::MOK_Free)))
return;
if (ChecksEnabled[CK_NewDeleteChecker] &&
- isStandardNewDelete(FD, C.getASTContext()))
+ isStandardNewDelete(FD, Ctx))
return;
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
