[PATCH] [Review request][analyzer] Duplicate '0 size allocation' check from unix.API in unix.Malloc.

[Антон Ярцев]

Updated the patch.


================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:823
@@ +822,3 @@
+// Returns true if we try to do a zero byte allocation, false otherwise.
+// Fills in TrueState and TalseState.
+static bool IsZeroByteAllocation(ProgramStateRef State,
----------------
zaks.anna wrote:
> TalseState -> FalseState
> 
> Are the True and False states backwards? I would expect TrueState to mean zero allocation; but by looking at the return it seems that it's the opposite..
Inverted.

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:845
@@ +844,3 @@
+  // Sanity check for the correct number of arguments
+  if (CE->getNumArgs() != NumArgs)
+    return nullptr;
----------------
zaks.anna wrote:
> Does it make sense to pass an extra argument just for sanity check?
Removed.

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:866
@@ +865,3 @@
+
+ProgramStateRef MallocChecker::CheckCallocZero(CheckerContext &C,
+                                               const CallExpr *CE,
----------------
zaks.anna wrote:
> Why is CheckCallocZero different from BasicAllocationCheck? (I feel like I am missing something..)
Hm, really. Removed CheckCallocZero, added two calls to BasicAllocationCheck instead.

================
Comment at: test/Analysis/malloc.c:957
@@ -951,1 +956,3 @@
 // ----------------------------------------------------------------------------
+// Test zero-sized allocations.
+void testMallocZero() {
----------------
zaks.anna wrote:
> Could you add a couple of more interesting zero allocation tests (where we are not dealing with zero constant).
> 
> Theoretically, the inlined defensive checks could be a problem here, right?
> (This is where the value is assumed to be zero by an inlined function, but known not be be zero in a caller.) 
> 
Added a path-sensitive test (testMallocPathZero) and an additional tests for 'calloc' (testCalloc1Unknown2Zero - covers the case when the first parameter to 'calloc' is an UnknownVal).

Yes, but only with 'suppress-inlined-defensive-checks=false' given to analyzer.

http://reviews.llvm.org/D6178

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/