From 59ed15b0525c314648d5adbaef47fbf93b61f0f4 Mon Sep 17 00:00:00 2001 From: Anton Yartsev Date: Wed, 13 Mar 2013 14:39:10 +0000 Subject: [PATCH] Refactoring: + Individual Report* method for each bug type + Comment improved: missing non-trivial alloca() case annotated + 'range' parameter of ReportBadFree() capitalized + 'SymbolRef Sym = State->getSVal(A, C.getLocationContext()).getAsSymbol();' shorten to 'SymbolRef Sym = C.getSVal(A).getAsSymbol();' llvm-svn: 176949 --- .../lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 89 +++++++++++++--------- 1 file changed, 55 insertions(+), 34 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 28a2999..5a02965 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -229,8 +229,14 @@ private: static bool SummarizeValue(raw_ostream &os, SVal V); static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR); - void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange range) const; + void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange Range) const; + void ReportBadDealloc(CheckerContext &C, SourceRange Range, + const Expr *DeallocExpr, const RefState *RS) const; void ReportOffsetFree(CheckerContext &C, SVal ArgVal, SourceRange Range)const; + void ReportUseAfterFree(CheckerContext &C, SourceRange Range, + SymbolRef Sym) const; + void ReportDoubleFree(CheckerContext &C, SourceRange Range, bool Released, + SymbolRef Sym, bool Interesting) const; /// Find the location of the allocation for Sym on the path leading to the /// exploded node N. @@ -714,7 +720,8 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, const MemSpaceRegion *MS = R->getMemorySpace(); - // Parameters, locals, statics, and globals shouldn't be freed. + // Parameters, locals, statics, globals, and memory returned by alloca() + // shouldn't be freed. if (!(isa(MS) || isa(MS))) { // FIXME: at the time this code was written, malloc() regions were // represented by conjured symbols, which are all in UnknownSpaceRegion. @@ -742,22 +749,8 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, if (RsBase && (RsBase->isReleased() || RsBase->isRelinquished()) && !didPreviousFreeFail(State, SymBase, PreviousRetStatusSymbol)) { - - if (ExplodedNode *N = C.generateSink()) { - if (!BT_DoubleFree) - BT_DoubleFree.reset( - new BugType("Double free", "Memory Error")); - BugReport *R = new BugReport(*BT_DoubleFree, - (RsBase->isReleased() ? "Attempt to free released memory" - : "Attempt to free non-owned memory"), - N); - R->addRange(ArgExpr->getSourceRange()); - R->markInteresting(SymBase); - if (PreviousRetStatusSymbol) - R->markInteresting(PreviousRetStatusSymbol); - R->addVisitor(new MallocBugVisitor(SymBase)); - C.emitReport(R); - } + ReportDoubleFree(C, ParentExpr->getSourceRange(), RsBase->isReleased(), + SymBase, PreviousRetStatusSymbol); return 0; } @@ -884,7 +877,7 @@ bool MallocChecker::SummarizeRegion(raw_ostream &os, } void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal, - SourceRange range) const { + SourceRange Range) const { if (ExplodedNode *N = C.generateSink()) { if (!BT_BadFree) BT_BadFree.reset(new BugType("Bad free", "Memory Error")); @@ -917,7 +910,7 @@ void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal, BugReport *R = new BugReport(*BT_BadFree, os.str(), N); R->markInteresting(MR); - R->addRange(range); + R->addRange(Range); C.emitReport(R); } } @@ -957,6 +950,43 @@ void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal, C.emitReport(R); } +void MallocChecker::ReportUseAfterFree(CheckerContext &C, SourceRange Range, + SymbolRef Sym) const { + + if (ExplodedNode *N = C.generateSink()) { + if (!BT_UseFree) + BT_UseFree.reset(new BugType("Use-after-free", "Memory Error")); + + BugReport *R = new BugReport(*BT_UseFree, + "Use of memory after it is freed", N); + + R->markInteresting(Sym); + R->addRange(Range); + R->addVisitor(new MallocBugVisitor(Sym)); + C.emitReport(R); + } +} + +void MallocChecker::ReportDoubleFree(CheckerContext &C, SourceRange Range, + bool Released, SymbolRef Sym, + bool Interesting) const { + + if (ExplodedNode *N = C.generateSink()) { + if (!BT_DoubleFree) + BT_DoubleFree.reset(new BugType("Double free", "Memory Error")); + + BugReport *R = new BugReport(*BT_DoubleFree, + (Released ? "Attempt to free released memory" + : "Attempt to free non-owned memory"), + N); + R->addRange(Range); + if (Interesting) + R->markInteresting(Sym); + R->addVisitor(new MallocBugVisitor(Sym)); + C.emitReport(R); + } +} + ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C, const CallExpr *CE, bool FreesOnFail) const { @@ -1222,7 +1252,7 @@ void MallocChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const { E = CE->arg_end(); I != E; ++I) { const Expr *A = *I; if (A->getType().getTypePtr()->isAnyPointerType()) { - SymbolRef Sym = State->getSVal(A, C.getLocationContext()).getAsSymbol(); + SymbolRef Sym = C.getSVal(A).getAsSymbol(); if (!Sym) continue; if (checkUseAfterFree(Sym, C, A)) @@ -1303,21 +1333,12 @@ bool MallocChecker::isReleased(SymbolRef Sym, CheckerContext &C) const { bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C, const Stmt *S) const { + if (isReleased(Sym, C)) { - if (ExplodedNode *N = C.generateSink()) { - if (!BT_UseFree) - BT_UseFree.reset(new BugType("Use-after-free", "Memory Error")); - - BugReport *R = new BugReport(*BT_UseFree, - "Use of memory after it is freed",N); - if (S) - R->addRange(S->getSourceRange()); - R->markInteresting(Sym); - R->addVisitor(new MallocBugVisitor(Sym)); - C.emitReport(R); - return true; - } + ReportUseAfterFree(C, S->getSourceRange(), Sym); + return true; } + return false; } -- 2.7.4