[PATCH] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki requested changes to this revision.
danielmarjamaki added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:788
 
+SVal MallocChecker::SValBinMulOp(CheckerContext &C, const Expr *Blocks,
+                                 const Expr *BlockBytes, ProgramStateRef State) {
----------------
I have the feeling this should be renamed. Since its purpose is to calculate the total size maybe MallocChecker::calculateBufferSize()


================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:790
+                                 const Expr *BlockBytes, ProgramStateRef State) {
+  SValBuilder &svalBuilder = C.getSValBuilder();
+  SVal nBlocks = C.getSVal(Blocks);
----------------
Please begin variables with Capitals. svalBuilder,nBlocks,nBlockBytes


================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:911
+      if (FunI == II_g_malloc0_n || FunI == II_g_try_malloc0_n) {
+        SValBuilder &svalBuilder = C.getSValBuilder();
+        Init = svalBuilder.makeZeroVal(svalBuilder.getContext().CharTy);
----------------
Capital. svalBuilder


================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2040
 
   const Expr *arg0Expr = CE->getArg(0);
   const LocationContext *LCtx = C.getLocationContext();
----------------
Capital variable: arg0Expr, arg0Val


================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2054
   const Expr *Arg1 = CE->getArg(1);
   if (!Arg1)
     return nullptr;
----------------
is this "if (!Arg1)" needed? It seems to me that if CE->getNumArgs() returns >= 2 then CE->getArg(1) should be non-NULL. Does it crash/assert if you remove this condition?


================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2061
+    const Expr *Arg2 = CE->getArg(2);
+    if (!Arg2)
+      return nullptr;
----------------
hmm.. seems weird if CE->getArg(2) returns nullptr even though CE->getNumArgs() >= 3 .. is it possible to remove this check or does that cause some crash etc?

> Generally, i'd prefer the code for computing the buffer size to be structured as

I would also prefer such code.



Repository:
  rL LLVM

https://reviews.llvm.org/D30771