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