Charusso updated this revision to Diff 209654.
Charusso marked 2 inline comments as done.
Charusso added a comment.
- Fix.
- Move the logic to `free()` for better matching.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64680/new/
https://reviews.llvm.org/D64680
Files:
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/test/Analysis/retain-count-alloc.cpp
Index: clang/test/Analysis/retain-count-alloc.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/retain-count-alloc.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=core,unix.Malloc \
+// RUN: -verify %s
+
+// expected-no-diagnostics: We do not model Integer Set Library's retain-count
+// based allocation.
+
+#define __isl_take
+#define __isl_keep
+
+struct Object { int Ref; };
+void free(void *);
+
+Object *copyObj(__isl_keep Object *O) {
+ O->Ref++;
+ return O;
+}
+
+void freeObj(__isl_take Object *O) {
+ if (--O->Ref > 0)
+ return;
+
+ free(O);
+}
+
+void useAfterFree(__isl_take Object *A) {
+ if (!A)
+ return;
+
+ Object *B = copyObj(A);
+ freeObj(B);
+
+ A->Ref = 13;
+ // no-warning: 'Use of memory after it is freed' was here.
+}
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -17,6 +17,7 @@
#include "clang/AST/ParentMap.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Lexer.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
@@ -41,6 +42,7 @@
// Used to check correspondence between allocators and deallocators.
enum AllocationFamily {
AF_None,
+ AF_NotModeled,
AF_Malloc,
AF_CXXNew,
AF_CXXNewArray,
@@ -50,19 +52,22 @@
};
class RefState {
- enum Kind { // Reference to allocated memory.
- Allocated,
- // Reference to zero-allocated memory.
- AllocatedOfSizeZero,
- // Reference to released/freed memory.
- Released,
- // The responsibility for freeing resources has transferred from
- // this reference. A relinquished symbol should not be freed.
- Relinquished,
- // We are no longer guaranteed to have observed all manipulations
- // of this pointer/memory. For example, it could have been
- // passed as a parameter to an opaque function.
- Escaped
+ enum Kind {
+ // If this checker does not model the allocation.
+ DoNothing,
+ // Reference to allocated memory.
+ Allocated,
+ // Reference to zero-allocated memory.
+ AllocatedOfSizeZero,
+ // Reference to released/freed memory.
+ Released,
+ // The responsibility for freeing resources has transferred from
+ // this reference. A relinquished symbol should not be freed.
+ Relinquished,
+ // We are no longer guaranteed to have observed all manipulations
+ // of this pointer/memory. For example, it could have been
+ // passed as a parameter to an opaque function.
+ Escaped
};
const Stmt *S;
@@ -75,6 +80,7 @@
assert(family != AF_None);
}
public:
+ bool isDoNothing() const { return K == DoNothing; }
bool isAllocated() const { return K == Allocated; }
bool isAllocatedOfSizeZero() const { return K == AllocatedOfSizeZero; }
bool isReleased() const { return K == Released; }
@@ -105,6 +111,9 @@
static RefState getEscaped(const RefState *RS) {
return RefState(Escaped, RS->getStmt(), RS->getAllocationFamily());
}
+ static RefState getDoNothing(const Stmt *S) {
+ return RefState(DoNothing, S, AF_NotModeled);
+ }
void Profile(llvm::FoldingSetNodeID &ID) const {
ID.AddInteger(K);
@@ -115,6 +124,7 @@
void dump(raw_ostream &OS) const {
switch (static_cast<Kind>(K)) {
#define CASE(ID) case ID: OS << #ID; break;
+ CASE(DoNothing)
CASE(Allocated)
CASE(AllocatedOfSizeZero)
CASE(Released)
@@ -359,6 +369,9 @@
/// Check if the memory associated with this symbol was released.
bool isReleased(SymbolRef Sym, CheckerContext &C) const;
+ /// Check whether we do not model the memory allocation.
+ bool isNotModeled(const CallExpr *CE, CheckerContext &C) const;
+
bool checkUseAfterFree(SymbolRef Sym, CheckerContext &C, const Stmt *S) const;
void checkUseZeroAllocated(SymbolRef Sym, CheckerContext &C,
@@ -877,6 +890,9 @@
State = ProcessZeroAllocation(C, CE, 0, State);
State = ProcessZeroAllocation(C, CE, 1, State);
} else if (FunI == II_free || FunI == II_g_free || FunI == II_kfree) {
+ if (isNotModeled(CE, C))
+ return;
+
State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
} else if (FunI == II_strdup || FunI == II_win_strdup ||
FunI == II_wcsdup || FunI == II_win_wcsdup) {
@@ -1460,6 +1476,7 @@
case AF_CXXNewArray: os << "'new[]'"; return;
case AF_IfNameIndex: os << "'if_nameindex()'"; return;
case AF_InnerBuffer: os << "container-specific allocator"; return;
+ case AF_NotModeled: os << "not modeled allocator"; return;
case AF_Alloca:
case AF_None: llvm_unreachable("not a deallocation expression");
}
@@ -1473,6 +1490,7 @@
case AF_CXXNewArray: os << "'delete[]'"; return;
case AF_IfNameIndex: os << "'if_freenameindex()'"; return;
case AF_InnerBuffer: os << "container-specific deallocator"; return;
+ case AF_NotModeled: os << "not modeled deallocator"; return;
case AF_Alloca:
case AF_None: llvm_unreachable("suspicious argument");
}
@@ -1662,6 +1680,8 @@
return CK_InnerPointerChecker;
return None;
}
+ case AF_NotModeled:
+ return None;
case AF_None: {
llvm_unreachable("no family");
}
@@ -2532,6 +2552,42 @@
return (RS && RS->isReleased());
}
+bool MallocChecker::isNotModeled(const CallExpr *CE, CheckerContext &C) const {
+ StringRef FunctionStr = "";
+ if (const Decl *D = C.getStackFrame()->getDecl())
+ if (const FunctionDecl *FD = D->getAsFunction())
+ FunctionStr = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(
+ {FD->getBeginLoc(), FD->getBody()->getBeginLoc()}),
+ C.getSourceManager(), C.getLangOpts());
+
+ if (FunctionStr.equals(""))
+ return false;
+
+ // We do not model Integer Set Library's retain-count based allocation.
+ if (!FunctionStr.contains("__isl_"))
+ return false;
+
+ ProgramStateRef State = C.getState();
+ const LocationContext *LCtx = C.getLocationContext();
+
+ const Expr *Arg = CE->getArg(0)->IgnoreImpCasts();
+ if (const auto *DRE = dyn_cast<DeclRefExpr>(Arg->IgnoreImpCasts())) {
+ if (const auto *VD = dyn_cast<ParmVarDecl>(DRE->getDecl())) {
+ SVal V = State->getSVal(State->getLValue(VD, LCtx));
+ if (SymbolRef Sym = V.getAsSymbol()) {
+ const RefState *RS = State->get<RegionState>(Sym);
+ if (!RS) {
+ State = State->set<RegionState>(Sym, RefState::getDoNothing(Arg));
+ C.addTransition(State);
+ }
+ }
+ }
+ }
+
+ return true;
+}
+
bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C,
const Stmt *S) const {
@@ -2876,9 +2932,13 @@
ProgramStateRef statePrev = N->getFirstPred()->getState();
const RefState *RS = state->get<RegionState>(Sym);
+ const Stmt *S = PathDiagnosticLocation::getStmt(N);
+
+ if (RS && RS->isDoNothing())
+ return nullptr;
+
const RefState *RSPrev = statePrev->get<RegionState>(Sym);
- const Stmt *S = PathDiagnosticLocation::getStmt(N);
// When dealing with containers, we sometimes want to give a note
// even if the statement is missing.
if (!S && (!RS || RS->getAllocationFamily() != AF_InnerBuffer))
@@ -2963,6 +3023,8 @@
Msg = OS.str();
break;
}
+ case AF_NotModeled:
+ llvm_unreachable("The report should be invalidated at this point.");
case AF_None:
llvm_unreachable("Unhandled allocation family!");
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
