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

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:885
+        return;
+      State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+      State = ProcessZeroAllocation(C, CE, 0, State);
----------------
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.


================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:889
+    } else if (FunI == II_g_realloc_n || FunI == II_g_try_realloc_n) {
+      if (CE->getNumArgs() < 2)
+        return;
----------------
Should this be 'getNumArgs() < 3' ?


================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:891
+        return;
+      State = ReallocMem(C, CE, false, State);
+      State = ProcessZeroAllocation(C, CE, 1, State);
----------------
Unfortunately, ReallocMem also assumes a single size argument:

`  // Get the size argument. If there is no size arg then give up.
  const Expr *Arg1 = CE->getArg(1);
  if (!Arg1)
    return nullptr;`


Repository:
  rL LLVM

https://reviews.llvm.org/D28348