From: Jordy Rose Date: Mon, 7 Jun 2010 19:32:37 +0000 (+0000) Subject: Catch free()s on non-regions and regions known to be not from malloc(), by checking... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=43859f66cdc360ab093cdde67401a7640a4bc05c;p=clang Catch free()s on non-regions and regions known to be not from malloc(), by checking the symbol type and memory space. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@105547 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Checker/MallocChecker.cpp b/lib/Checker/MallocChecker.cpp index 22b8b29f78..30c4f172d0 100644 --- a/lib/Checker/MallocChecker.cpp +++ b/lib/Checker/MallocChecker.cpp @@ -59,11 +59,12 @@ class MallocChecker : public CheckerVisitor { BuiltinBug *BT_DoubleFree; BuiltinBug *BT_Leak; BuiltinBug *BT_UseFree; + BuiltinBug *BT_BadFree; IdentifierInfo *II_malloc, *II_free, *II_realloc, *II_calloc; public: MallocChecker() - : BT_DoubleFree(0), BT_Leak(0), BT_UseFree(0), + : BT_DoubleFree(0), BT_Leak(0), BT_UseFree(0), BT_BadFree(0), II_malloc(0), II_free(0), II_realloc(0), II_calloc(0) {} static void *getTag(); bool EvalCallExpr(CheckerContext &C, const CallExpr *CE); @@ -90,6 +91,10 @@ private: void ReallocMem(CheckerContext &C, const CallExpr *CE); void CallocMem(CheckerContext &C, const CallExpr *CE); + + bool SummarizeValue(llvm::raw_ostream& os, SVal V); + bool SummarizeRegion(llvm::raw_ostream& os, const MemRegion *MR); + void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange range); }; } // end anonymous namespace @@ -191,18 +196,59 @@ void MallocChecker::FreeMem(CheckerContext &C, const CallExpr *CE) { const GRState *MallocChecker::FreeMemAux(CheckerContext &C, const CallExpr *CE, const GRState *state) { - SVal ArgVal = state->getSVal(CE->getArg(0)); + const Expr *ArgExpr = CE->getArg(0); + SVal ArgVal = state->getSVal(ArgExpr); // If ptr is NULL, no operation is preformed. if (ArgVal.isZeroConstant()) return state; + + // Unknown values could easily be okay + // Undefined values are handled elsewhere + if (ArgVal.isUnknownOrUndef()) + return state; - SymbolRef Sym = ArgVal.getAsLocSymbol(); - + const MemRegion *R = ArgVal.getAsRegion(); + + // Nonlocs can't be freed, of course. + // Non-region locations (labels and fixed addresses) also shouldn't be freed. + if (!R) { + ReportBadFree(C, ArgVal, ArgExpr->getSourceRange()); + return NULL; + } + + R = R->StripCasts(); + + // Blocks might show up as heap data, but should not be free()d + if (isa(R)) { + ReportBadFree(C, ArgVal, ArgExpr->getSourceRange()); + return NULL; + } + + const MemSpaceRegion *MS = R->getMemorySpace(); + + // Parameters, locals, statics, and globals 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. + // This means that there isn't actually anything from HeapSpaceRegion + // that should be freed, even though we allow it here. + // Of course, free() can work on memory allocated outside the current + // function, so UnknownSpaceRegion is always a possibility. + // False negatives are better than false positives. + + ReportBadFree(C, ArgVal, ArgExpr->getSourceRange()); + return NULL; + } + + const SymbolicRegion *SR = dyn_cast(R); // Various cases could lead to non-symbol values here. - if (!Sym) + // For now, ignore them. + if (!SR) return state; + SymbolRef Sym = SR->getSymbol(); + const RefState *RS = state->get(Sym); // If the symbol has not been tracked, return. This is possible when free() is @@ -230,6 +276,135 @@ const GRState *MallocChecker::FreeMemAux(CheckerContext &C, const CallExpr *CE, return state->set(Sym, RefState::getReleased(CE)); } +bool MallocChecker::SummarizeValue(llvm::raw_ostream& os, SVal V) { + if (nonloc::ConcreteInt *IntVal = dyn_cast(&V)) + os << "an integer (" << IntVal->getValue() << ")"; + else if (loc::ConcreteInt *ConstAddr = dyn_cast(&V)) + os << "a constant address (" << ConstAddr->getValue() << ")"; + else if (loc::GotoLabel *Label = dyn_cast(&V)) + os << "the address of the label '" + << Label->getLabel()->getID()->getName() + << "'"; + else + return false; + + return true; +} + +bool MallocChecker::SummarizeRegion(llvm::raw_ostream& os, + const MemRegion *MR) { + switch (MR->getKind()) { + case MemRegion::FunctionTextRegionKind: { + const FunctionDecl *FD = cast(MR)->getDecl(); + if (FD) + os << "the address of the function '" << FD << "'"; + else + os << "the address of a function"; + return true; + } + case MemRegion::BlockTextRegionKind: + os << "block text"; + return true; + case MemRegion::BlockDataRegionKind: + // FIXME: where the block came from? + os << "a block"; + return true; + default: { + const MemSpaceRegion *MS = MR->getMemorySpace(); + + switch (MS->getKind()) { + case MemRegion::StackLocalsSpaceRegionKind: { + const VarRegion *VR = dyn_cast(MR); + const VarDecl *VD; + if (VR) + VD = VR->getDecl(); + else + VD = NULL; + + if (VD) + os << "the address of the local variable '" << VD->getName() << "'"; + else + os << "the address of a local stack variable"; + return true; + } + case MemRegion::StackArgumentsSpaceRegionKind: { + const VarRegion *VR = dyn_cast(MR); + const VarDecl *VD; + if (VR) + VD = VR->getDecl(); + else + VD = NULL; + + if (VD) + os << "the address of the parameter '" << VD->getName() << "'"; + else + os << "the address of a parameter"; + return true; + } + case MemRegion::GlobalsSpaceRegionKind: { + const VarRegion *VR = dyn_cast(MR); + const VarDecl *VD; + if (VR) + VD = VR->getDecl(); + else + VD = NULL; + + if (VD) { + if (VD->isStaticLocal()) + os << "the address of the static variable '" << VD->getName() << "'"; + else + os << "the address of the global variable '" << VD->getName() << "'"; + } else + os << "the address of a global variable"; + return true; + } + default: + return false; + } + } + } +} + +void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal, + SourceRange range) { + ExplodedNode *N = C.GenerateSink(); + if (N) { + if (!BT_BadFree) + BT_BadFree = new BuiltinBug("Bad free"); + + llvm::SmallString<100> buf; + llvm::raw_svector_ostream os(buf); + + const MemRegion *MR = ArgVal.getAsRegion(); + if (MR) { + while (const ElementRegion *ER = dyn_cast(MR)) + MR = ER->getSuperRegion(); + + // Special case for alloca() + if (isa(MR)) + os << "Argument to free() was allocated by alloca(), not malloc()"; + else { + os << "Argument to free() is "; + if (SummarizeRegion(os, MR)) + os << ", which is not memory allocated by malloc()"; + else + os << "not memory allocated by malloc()"; + } + } else { + os << "Argument to free() is "; + if (SummarizeValue(os, ArgVal)) + os << ", which is not memory allocated by malloc()"; + else + os << "not memory allocated by malloc()"; + } + + os.flush(); + EnhancedBugReport *R = new EnhancedBugReport(*BT_BadFree, buf.str(), N); + R->addRange(range); + C.EmitReport(R); + } +} + void MallocChecker::ReallocMem(CheckerContext &C, const CallExpr *CE) { const GRState *state = C.getState(); const Expr *Arg0 = CE->getArg(0); diff --git a/test/Analysis/free.c b/test/Analysis/free.c new file mode 100644 index 0000000000..60bb3f2eb5 --- /dev/null +++ b/test/Analysis/free.c @@ -0,0 +1,71 @@ +// RUN: %clang_cc1 -analyze -analyzer-experimental-internal-checks -analyzer-check-objc-mem -analyzer-store=region -analyzer-experimental-checks -fblocks -verify %s +void free(void *); + +void t1 () { + int a[] = { 1 }; + free(a); // expected-warning {{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}} +} + +void t2 () { + int a = 1; + free(&a); // expected-warning {{Argument to free() is the address of the local variable 'a', which is not memory allocated by malloc()}} +} + +void t3 () { + static int a[] = { 1 }; + free(a); // expected-warning {{Argument to free() is the address of the static variable 'a', which is not memory allocated by malloc()}} +} + +void t4 (char *x) { + free(x); // no-warning +} + +void t5 () { + extern char *ptr(); + free(ptr()); // no-warning +} + +void t6 () { + free((void*)1000); // expected-warning {{Argument to free() is a constant address (1000), which is not memory allocated by malloc()}} +} + +void t7 (char **x) { + free(*x); // no-warning +} + +void t8 (char **x) { + // ugh + free((*x)+8); // no-warning +} + +void t9 () { +label: + free(&&label); // expected-warning {{Argument to free() is the address of the label 'label', which is not memory allocated by malloc()}} +} + +void t10 () { + free((void*)&t10); // expected-warning {{Argument to free() is the address of the function 't10', which is not memory allocated by malloc()}} +} + +void t11 () { + char *p = (char*)__builtin_alloca(2); + free(p); // expected-warning {{Argument to free() was allocated by alloca(), not malloc()}} +} + +void t12 () { + free(^{return;}); // expected-warning {{Argument to free() is a block, which is not memory allocated by malloc()}} +} + +void t13 (char a) { + free(&a); // expected-warning {{Argument to free() is the address of the parameter 'a', which is not memory allocated by malloc()}} +} + +static int someGlobal[2]; +void t14 () { + free(someGlobal); // expected-warning {{Argument to free() is the address of the global variable 'someGlobal', which is not memory allocated by malloc()}} +} + +void t15 (char **x, int offset) { + // Unknown value + free(x[offset]); // no-warning +}