[PATCH] D28348: [analyzer] Taught the analyzer about Glib API to check Memory-leak

[Leslie Zhai via Phabricator via cfe-commits]

xiangzhai marked 3 inline comments as done.
xiangzhai added a comment.

Hi Anna,

Thank you so much for the review!

I will try to implement extend MallocMemAux and ReallocMem in the follow up patch.

Regards,
Leslie Zhai



================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:885
+        return;
+      State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+      State = ProcessZeroAllocation(C, CE, 0, State);
----------------
zaks.anna wrote:
> I am not sure this is correct as the third argument is "size of allocation", which in this case would be the value of CE->getArg(0) times the value of CE->getArg(2).
> 
> The current implementation of MallocMemAux would need to be extended to incorporate this:
> `  // Set the region's extent equal to the Size parameter.
>   const SymbolicRegion *R =
>       dyn_cast_or_null<SymbolicRegion>(RetVal.getAsRegion());
>   if (!R)
>     return nullptr;
>   if (Optional<DefinedOrUnknownSVal> DefinedSize =
>           Size.getAs<DefinedOrUnknownSVal>()) {
>     SValBuilder &svalBuilder = C.getSValBuilder();
>     DefinedOrUnknownSVal Extent = R->getExtent(svalBuilder);
>     DefinedOrUnknownSVal extentMatchesSize =
>         svalBuilder.evalEQ(State, Extent, *DefinedSize);
> 
>     State = State->assume(extentMatchesSize, true);
>     assert(State);
>   }`
> 
> My suggestion is to submit the patch without the 'n' variants and extend MallocMemAux to deal with them as a follow up patch.
Hi Anna,

Thanks for your review!

> I am not sure this is correct as the third argument is "size of allocation", which in this case would be the value of CE->getArg(0) times the value of CE->getArg(2).

```g_malloc_n``` just use two arguments https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html#g-malloc-n

so in this case would be the value of CE->getArg(0) x CE->getArg(1)?

> My suggestion is to submit the patch without the 'n' variants and extend MallocMemAux to deal with them as a follow up patch.

I will drop XXX_n from this patch, then implement them until extend MallocMemAux and ReallocMem implemented in the follow up patch.

Regards,
Leslie Zhai


Repository:
  rL LLVM

https://reviews.llvm.org/D28348