[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a subscriber: NoQ.
xazax.hun added inline comments.


================
Comment at: docs/ReleaseNotes.rst:261
+- Static Analyzer can now properly detect and diagnose unary pre-/post-
+  increment/decrement on an uninitialized values.
+
----------------
lebedev.ri wrote:
> JonasToth wrote:
> > The end of the sentence should be either 'on uninitialized values' or 'on an uninitialized values'.
> > The end of the sentence should be either 'on uninitialized values' or 'on an uninitialized values'.
> But it is already `on an uninitialized values.` ?
Maybe Jonas was thinking if removing the plural form if you having `an` or of you want to keep it plural remove the `an`.


================
Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:1048
+
+      // directly perform the store. needed for uninitialized value detection.
+      Bldr.takeNodes(*I);
----------------
Comments should be whole sentences starting with capital letters.


================
Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:1051
+      ExplodedNodeSet Dst3;
+      evalStore(Dst3, U, U, *I, state, loc, V2_untested);
+      Bldr.addNodes(Dst3);
----------------
This will trigger checkBind when unknown value is pre/post incremented. I wonder if this is the desired behavior. Couldn't you achieve the same on the checker side by having a checkPreStmt hook?

@NoQ, @dcoughlin what do you think about triggering checkBind here? In fact, there is a bind during pre/post increment/decrement. But do we want these events to be visible for checkers with undefined values?


================
Comment at: test/Analysis/malloc-plist.c:2
 // RUN: rm -f %t
 // RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,unix.Malloc -analyzer-output=plist -analyzer-config path-diagnostics-alternate=false -o %t %s
 // RUN: FileCheck -input-file %t %s
----------------
In case you willing to replace the plist based test with `-analyzer-output=text` based, it would be awesome, though not required for this patch to be accepted.


Repository:
  rL LLVM

https://reviews.llvm.org/D40463