Jordy, You are right, it should be Unknown. However, as I was looking at this, I've realized that the Malloc Checker should not try to evaluate strdup - it's the job of the CString checker; addressed in r151188.
Thanks, Anna. On Feb 22, 2012, at 7:59 AM, Jordy Rose wrote: > 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
