<div dir="ltr">I'd quite happily add them... but can I do it in another patch? I think I could be more thorough that way.<div><br></div><div>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 <i>because</i> of their kooky APIs, they're harder to implement checkers for.</div></div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div dir="ltr">Sincerely,<br>Alexander Riccio<br>--<br>"Change the world or go home."<div><a href="http://about.me/ariccio" target="_blank">about.me/ariccio</a></div><div><a href="http://about.me/ariccio" target="_blank"><br></a></div><div>If left to my own devices, I will build more.</div><div>⁂<br></div></div></div></div></div></div></div>
<br><div class="gmail_quote">On Tue, Mar 1, 2016 at 8:38 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron.ballman@gmail.com" target="_blank">aaron.ballman@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Mar 1, 2016 at 2:16 AM, Alexander Riccio <<a href="mailto:alexander@riccio.com">alexander@riccio.com</a>> wrote:<br>
> ariccio updated this revision to Diff 49456.<br>
> ariccio added a comment.<br>
><br>
> Nit addressed.<br>
><br>
><br>
> <a href="http://reviews.llvm.org/D17688" rel="noreferrer" target="_blank">http://reviews.llvm.org/D17688</a><br>
><br>
> Files:<br>
> llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp<br>
><br>
> Index: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp<br>
> ===================================================================<br>
> --- llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp<br>
> +++ llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp<br>
> @@ -169,11 +169,12 @@<br>
> {<br>
> public:<br>
> MallocChecker()<br>
> - : II_alloca(nullptr), II_malloc(nullptr), II_free(nullptr),<br>
> - II_realloc(nullptr), II_calloc(nullptr), II_valloc(nullptr),<br>
> - II_reallocf(nullptr), II_strndup(nullptr), II_strdup(nullptr),<br>
> - II_kmalloc(nullptr), II_if_nameindex(nullptr),<br>
> - II_if_freenameindex(nullptr) {}<br>
> + : II_alloca(nullptr), II_win_alloca(nullptr), II_malloc(nullptr),<br>
> + II_free(nullptr), II_realloc(nullptr), II_calloc(nullptr),<br>
> + II_valloc(nullptr), II_reallocf(nullptr), II_strndup(nullptr),<br>
> + II_strdup(nullptr), II_win_strdup(nullptr), II_kmalloc(nullptr),<br>
> + II_if_nameindex(nullptr), II_if_freenameindex(nullptr),<br>
> + II_wcsdup(nullptr), II_win_wcsdup(nullptr) {}<br>
><br>
> /// In pessimistic mode, the checker assumes that it does not know which<br>
> /// functions might free the memory.<br>
> @@ -231,10 +232,11 @@<br>
> mutable std::unique_ptr<BugType> BT_MismatchedDealloc;<br>
> mutable std::unique_ptr<BugType> BT_OffsetFree[CK_NumCheckKinds];<br>
> mutable std::unique_ptr<BugType> BT_UseZerroAllocated[CK_NumCheckKinds];<br>
> - mutable IdentifierInfo *II_alloca, *II_malloc, *II_free, *II_realloc,<br>
> - *II_calloc, *II_valloc, *II_reallocf, *II_strndup,<br>
> - *II_strdup, *II_kmalloc, *II_if_nameindex,<br>
> - *II_if_freenameindex;<br>
> + mutable IdentifierInfo *II_alloca, *II_win_alloca, *II_malloc, *II_free,<br>
> + *II_realloc, *II_calloc, *II_valloc, *II_reallocf,<br>
> + *II_strndup, *II_strdup, *II_win_strdup, *II_kmalloc,<br>
> + *II_if_nameindex, *II_if_freenameindex, *II_wcsdup,<br>
> + *II_win_wcsdup;<br>
> mutable Optional<uint64_t> KernelZeroFlagVal;<br>
><br>
> void initIdentifierInfo(ASTContext &C) const;<br>
> @@ -540,9 +542,15 @@<br>
> II_valloc = &Ctx.Idents.get("valloc");<br>
> II_strdup = &Ctx.Idents.get("strdup");<br>
> II_strndup = &Ctx.Idents.get("strndup");<br>
> + II_wcsdup = &Ctx.Idents.get("wcsdup");<br>
> II_kmalloc = &Ctx.Idents.get("kmalloc");<br>
> II_if_nameindex = &Ctx.Idents.get("if_nameindex");<br>
> II_if_freenameindex = &Ctx.Idents.get("if_freenameindex");<br>
> +<br>
> + //MSVC uses `_`-prefixed instead, so we check for them too.<br>
> + II_win_strdup = &Ctx.Idents.get("_strdup");<br>
> + II_win_wcsdup = &Ctx.Idents.get("_wcsdup");<br>
> + II_win_alloca = &Ctx.Idents.get("_alloca");<br>
<br>
</div></div>What about: _mbsdup, _strdup_dbg, _wcsdup_dbg, _aligned_realloc, and<br>
the rest? If we're going to add these (which I really support), it<br>
would be good to make a comprehensive sweep for the Win32 additions<br>
and add them all.<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> }<br>
><br>
> bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext &C) const {<br>
> @@ -585,7 +593,8 @@<br>
> if (Family == AF_Malloc && CheckAlloc) {<br>
> if (FunI == II_malloc || FunI == II_realloc || FunI == II_reallocf ||<br>
> FunI == II_calloc || FunI == II_valloc || FunI == II_strdup ||<br>
> - FunI == II_strndup || FunI == II_kmalloc)<br>
> + FunI == II_win_strdup || FunI == II_strndup || FunI == II_wcsdup ||<br>
> + FunI == II_win_wcsdup || FunI == II_kmalloc)<br>
> return true;<br>
> }<br>
><br>
> @@ -600,7 +609,7 @@<br>
> }<br>
><br>
> if (Family == AF_Alloca && CheckAlloc) {<br>
> - if (FunI == II_alloca)<br>
> + if (FunI == II_alloca || FunI == II_win_alloca)<br>
> return true;<br>
> }<br>
> }<br>
> @@ -789,11 +798,12 @@<br>
> State = ProcessZeroAllocation(C, CE, 1, State);<br>
> } else if (FunI == II_free) {<br>
> State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);<br>
> - } else if (FunI == II_strdup) {<br>
> + } else if (FunI == II_strdup || FunI == II_win_strdup ||<br>
> + FunI == II_wcsdup || FunI == II_win_wcsdup) {<br>
> State = MallocUpdateRefState(C, CE, State);<br>
> } else if (FunI == II_strndup) {<br>
> State = MallocUpdateRefState(C, CE, State);<br>
> - } else if (FunI == II_alloca) {<br>
> + } else if (FunI == II_alloca || FunI == II_win_alloca) {<br>
> State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,<br>
> AF_Alloca);<br>
> State = ProcessZeroAllocation(C, CE, 0, State);<br>
><br>
><br>
</div></div></blockquote></div><br></div>