[cfe-commits] r151124 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/MallocChecker.cpp test/Analysis/malloc.c

[Jordy Rose]

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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits