[PATCH] D17688: Fix missed leak from MSVC specific allocation functions

[<Alexander G. Riccio> via cfe-commits]

I'd quite happily add them... but can I do it in another patch? I think I
could be more thorough that way.

For the same reason, can we list all the microsoft memory allocating
routines here? There are a thousand routines we might want to add, and then
a few others (like _dupenv_s, _malloca, and _expand) which are especially
important to be able to analyze (because they have really tricky APIs), but
*because* of their kooky APIs, they're harder to implement checkers for.

Sincerely,
Alexander Riccio
--
"Change the world or go home."
about.me/ariccio

<http://about.me/ariccio>
If left to my own devices, I will build more.
⁂

On Tue, Mar 1, 2016 at 8:38 AM, Aaron Ballman <aaron.ballman at gmail.com>
wrote:

> On Tue, Mar 1, 2016 at 2:16 AM, Alexander Riccio <alexander at riccio.com>
> wrote:
> > ariccio updated this revision to Diff 49456.
> > ariccio added a comment.
> >
> > Nit addressed.
> >
> >
> > http://reviews.llvm.org/D17688
> >
> > Files:
> >   llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
> >
> > Index: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
> > ===================================================================
> > --- llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
> > +++ llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
> > @@ -169,11 +169,12 @@
> >  {
> >  public:
> >    MallocChecker()
> > -      : II_alloca(nullptr), II_malloc(nullptr), II_free(nullptr),
> > -        II_realloc(nullptr), II_calloc(nullptr), II_valloc(nullptr),
> > -        II_reallocf(nullptr), II_strndup(nullptr), II_strdup(nullptr),
> > -        II_kmalloc(nullptr), II_if_nameindex(nullptr),
> > -        II_if_freenameindex(nullptr) {}
> > +      : II_alloca(nullptr), II_win_alloca(nullptr), II_malloc(nullptr),
> > +        II_free(nullptr), II_realloc(nullptr), II_calloc(nullptr),
> > +        II_valloc(nullptr), II_reallocf(nullptr), II_strndup(nullptr),
> > +        II_strdup(nullptr), II_win_strdup(nullptr), II_kmalloc(nullptr),
> > +        II_if_nameindex(nullptr), II_if_freenameindex(nullptr),
> > +        II_wcsdup(nullptr), II_win_wcsdup(nullptr) {}
> >
> >    /// In pessimistic mode, the checker assumes that it does not know
> which
> >    /// functions might free the memory.
> > @@ -231,10 +232,11 @@
> >    mutable std::unique_ptr<BugType> BT_MismatchedDealloc;
> >    mutable std::unique_ptr<BugType> BT_OffsetFree[CK_NumCheckKinds];
> >    mutable std::unique_ptr<BugType>
> BT_UseZerroAllocated[CK_NumCheckKinds];
> > -  mutable IdentifierInfo *II_alloca, *II_malloc, *II_free, *II_realloc,
> > -                         *II_calloc, *II_valloc, *II_reallocf,
> *II_strndup,
> > -                         *II_strdup, *II_kmalloc, *II_if_nameindex,
> > -                         *II_if_freenameindex;
> > +  mutable IdentifierInfo *II_alloca, *II_win_alloca, *II_malloc,
> *II_free,
> > +                         *II_realloc, *II_calloc, *II_valloc,
> *II_reallocf,
> > +                         *II_strndup, *II_strdup, *II_win_strdup,
> *II_kmalloc,
> > +                         *II_if_nameindex, *II_if_freenameindex,
> *II_wcsdup,
> > +                         *II_win_wcsdup;
> >    mutable Optional<uint64_t> KernelZeroFlagVal;
> >
> >    void initIdentifierInfo(ASTContext &C) const;
> > @@ -540,9 +542,15 @@
> >    II_valloc = &Ctx.Idents.get("valloc");
> >    II_strdup = &Ctx.Idents.get("strdup");
> >    II_strndup = &Ctx.Idents.get("strndup");
> > +  II_wcsdup = &Ctx.Idents.get("wcsdup");
> >    II_kmalloc = &Ctx.Idents.get("kmalloc");
> >    II_if_nameindex = &Ctx.Idents.get("if_nameindex");
> >    II_if_freenameindex = &Ctx.Idents.get("if_freenameindex");
> > +
> > +  //MSVC uses `_`-prefixed instead, so we check for them too.
> > +  II_win_strdup = &Ctx.Idents.get("_strdup");
> > +  II_win_wcsdup = &Ctx.Idents.get("_wcsdup");
> > +  II_win_alloca = &Ctx.Idents.get("_alloca");
>
> What about: _mbsdup, _strdup_dbg, _wcsdup_dbg, _aligned_realloc, and
> the rest? If we're going to add these (which I really support), it
> would be good to make a comprehensive sweep for the Win32 additions
> and add them all.
>
> ~Aaron
>
> >  }
> >
> >  bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext
> &C) const {
> > @@ -585,7 +593,8 @@
> >      if (Family == AF_Malloc && CheckAlloc) {
> >        if (FunI == II_malloc || FunI == II_realloc || FunI ==
> II_reallocf ||
> >            FunI == II_calloc || FunI == II_valloc || FunI == II_strdup ||
> > -          FunI == II_strndup || FunI == II_kmalloc)
> > +          FunI == II_win_strdup || FunI == II_strndup || FunI ==
> II_wcsdup ||
> > +          FunI == II_win_wcsdup || FunI == II_kmalloc)
> >          return true;
> >      }
> >
> > @@ -600,7 +609,7 @@
> >      }
> >
> >      if (Family == AF_Alloca && CheckAlloc) {
> > -      if (FunI == II_alloca)
> > +      if (FunI == II_alloca || FunI == II_win_alloca)
> >          return true;
> >      }
> >    }
> > @@ -789,11 +798,12 @@
> >        State = ProcessZeroAllocation(C, CE, 1, State);
> >      } else if (FunI == II_free) {
> >        State = FreeMemAux(C, CE, State, 0, false,
> ReleasedAllocatedMemory);
> > -    } else if (FunI == II_strdup) {
> > +    } else if (FunI == II_strdup || FunI == II_win_strdup ||
> > +               FunI == II_wcsdup || FunI == II_win_wcsdup) {
> >        State = MallocUpdateRefState(C, CE, State);
> >      } else if (FunI == II_strndup) {
> >        State = MallocUpdateRefState(C, CE, State);
> > -    } else if (FunI == II_alloca) {
> > +    } else if (FunI == II_alloca || FunI == II_win_alloca) {
> >        State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
> >                             AF_Alloca);
> >        State = ProcessZeroAllocation(C, CE, 0, State);
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160301/403b8610/attachment-0001.html>