Minor complaint: why is the size of a strdup allocation Undef and not Unknown? I see we can skip the extent-setting step, but really we might as well just do that anyway if it's Unknown.
Jordy On Feb 22, 2012, at 10:14, Anna Zaks wrote: > Author: zaks > Date: Tue Feb 21 21:14:20 2012 > New Revision: 151124 > > URL: http://llvm.org/viewvc/llvm-project?rev=151124&view=rev > Log: > [analyzer] Malloc checker: mark 'strdup' and 'strndup' as allocators. > > Modified: > cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp > cfe/trunk/test/Analysis/malloc.c > > Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=151124&r1=151123&r2=151124&view=diff > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original) > +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Tue Feb 21 > 21:14:20 2012 > @@ -26,6 +26,8 @@ > #include "llvm/ADT/ImmutableMap.h" > #include "llvm/ADT/SmallString.h" > #include "llvm/ADT/STLExtras.h" > +#include <climits> > + > using namespace clang; > using namespace ento; > > @@ -97,11 +99,13 @@ > mutable OwningPtr<BugType> BT_UseFree; > mutable OwningPtr<BugType> BT_BadFree; > mutable IdentifierInfo *II_malloc, *II_free, *II_realloc, *II_calloc, > - *II_valloc, *II_reallocf; > + *II_valloc, *II_reallocf, *II_strndup, *II_strdup; > + > + static const unsigned InvalidArgIndex = UINT_MAX; > > public: > MallocChecker() : II_malloc(0), II_free(0), II_realloc(0), II_calloc(0), > - II_valloc(0), II_reallocf(0) {} > + II_valloc(0), II_reallocf(0), II_strndup(0), > II_strdup(0) {} > > /// In pessimistic mode, the checker assumes that it does not know which > /// functions might free the memory. > @@ -140,7 +144,8 @@ > /// pointed to by one of its arguments. > bool isMemFunction(const FunctionDecl *FD, ASTContext &C) const; > > - static void MallocMem(CheckerContext &C, const CallExpr *CE); > + static void MallocMem(CheckerContext &C, const CallExpr *CE, > + unsigned SizeIdx); > static void MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE, > const OwnershipAttr* Att); > static ProgramStateRef MallocMemAux(CheckerContext &C, const CallExpr *CE, > @@ -283,6 +288,10 @@ > II_calloc = &Ctx.Idents.get("calloc"); > if (!II_valloc) > II_valloc = &Ctx.Idents.get("valloc"); > + if (!II_strdup) > + II_strdup = &Ctx.Idents.get("strdup"); > + if (!II_strndup) > + II_strndup = &Ctx.Idents.get("strndup"); > } > > bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext &C) > const { > @@ -294,9 +303,9 @@ > > initIdentifierInfo(C); > > - // TODO: Add more here : ex: reallocf! > if (FunI == II_malloc || FunI == II_free || FunI == II_realloc || > - FunI == II_reallocf || FunI == II_calloc || FunI == II_valloc) > + FunI == II_reallocf || FunI == II_calloc || FunI == II_valloc || > + FunI == II_strdup || FunI == II_strndup) > return true; > > if (Filter.CMallocOptimistic && FD->hasAttrs() && > @@ -319,7 +328,7 @@ > return; > > if (FunI == II_malloc || FunI == II_valloc) { > - MallocMem(C, CE); > + MallocMem(C, CE, 0); > return; > } else if (FunI == II_realloc) { > ReallocMem(C, CE, false); > @@ -330,9 +339,15 @@ > } else if (FunI == II_calloc) { > CallocMem(C, CE); > return; > - }else if (FunI == II_free) { > + } else if (FunI == II_free) { > FreeMem(C, CE); > return; > + } else if (FunI == II_strdup) { > + MallocMem(C, CE, InvalidArgIndex); > + return; > + } else if (FunI == II_strndup) { > + MallocMem(C, CE, 1); > + return; > } > > if (Filter.CMallocOptimistic) > @@ -358,10 +373,16 @@ > } > } > > -void MallocChecker::MallocMem(CheckerContext &C, const CallExpr *CE) { > - ProgramStateRef state = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), > - C.getState()); > - C.addTransition(state); > +void MallocChecker::MallocMem(CheckerContext &C, const CallExpr *CE, > + unsigned SizeIdx) { > + SVal Undef = UndefinedVal(); > + ProgramStateRef State; > + if (SizeIdx != InvalidArgIndex) > + State = MallocMemAux(C, CE, CE->getArg(SizeIdx), Undef, C.getState()); > + else > + State = MallocMemAux(C, CE, Undef, Undef, C.getState()); > + > + C.addTransition(State); > } > > void MallocChecker::MallocMemReturnsAttr(CheckerContext &C, const CallExpr > *CE, > @@ -400,16 +421,17 @@ > // Set the region's extent equal to the Size parameter. > const SymbolicRegion *R = > dyn_cast_or_null<SymbolicRegion>(retVal.getAsRegion()); > - if (!R || !isa<DefinedOrUnknownSVal>(Size)) > + if (!R) > return 0; > + if (isa<DefinedOrUnknownSVal>(Size)) { > + DefinedOrUnknownSVal Extent = R->getExtent(svalBuilder); > + DefinedOrUnknownSVal DefinedSize = cast<DefinedOrUnknownSVal>(Size); > + DefinedOrUnknownSVal extentMatchesSize = > + svalBuilder.evalEQ(state, Extent, DefinedSize); > > - DefinedOrUnknownSVal Extent = R->getExtent(svalBuilder); > - DefinedOrUnknownSVal DefinedSize = cast<DefinedOrUnknownSVal>(Size); > - DefinedOrUnknownSVal extentMatchesSize = > - svalBuilder.evalEQ(state, Extent, DefinedSize); > - > - state = state->assume(extentMatchesSize, true); > - assert(state); > + state = state->assume(extentMatchesSize, true); > + assert(state); > + } > > SymbolRef Sym = retVal.getAsLocSymbol(); > assert(Sym); > > Modified: cfe/trunk/test/Analysis/malloc.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=151124&r1=151123&r2=151124&view=diff > ============================================================================== > --- cfe/trunk/test/Analysis/malloc.c (original) > +++ cfe/trunk/test/Analysis/malloc.c Tue Feb 21 21:14:20 2012 > @@ -652,6 +652,25 @@ > return &(px->g); > } > > +// Test various allocation/deallocation functions. > + > +char *strdup(const char *s); > +char *strndup(const char *s, size_t n); > + > +void testStrdup(const char *s, unsigned validIndex) { > + char *s2 = strdup(s); > + s2[validIndex + 1] = 'b';// expected-warning {{Memory is never released; > potential memory leak}} > +} > + > +int testStrndup(const char *s, unsigned validIndex, unsigned size) { > + char *s2 = strndup(s, size); > + s2 [validIndex + 1] = 'b'; > + if (s2[validIndex] != 'a') > + return 0;// expected-warning {{Memory is never released; potential > memory leak}} > + else > + return 1;// expected-warning {{Memory is never released; potential > memory leak}} > +} > + > // Below are the known false positives. > > // TODO: There should be no warning here. This one might be difficult to get > rid of. > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
