[PATCH] D22862: [analyzer] Fix for PR15623: eliminate unwanted ProgramState checker data propagation.

[Anton Yartsev via cfe-commits]

ayartsev created this revision.
ayartsev added reviewers: zaks.anna, krememek.
ayartsev added a subscriber: cfe-commits.

The attached patch eliminates unneeded checker data propagation from one of the operands of a logical operation to the operation result. The result of a logical operation is calculated from the logical values of its operands and is independent from operands nature.

One of the test changed its result (misc-ps-region-store.m, rdar_7275774). I did not manage to understand the test, something is definitely wrong with it - at least the comment inside the test do not correspond to reality and an old test result seem to be wrong.

The patch fixes https://llvm.org/bugs/show_bug.cgi?id=15623.

Please review!

https://reviews.llvm.org/D22862

Files:
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  test/Analysis/misc-ps-region-store.m
  test/Analysis/unwanted-programstate-data-propagation.c

Index: test/Analysis/unwanted-programstate-data-propagation.c
===================================================================
--- test/Analysis/unwanted-programstate-data-propagation.c
+++ test/Analysis/unwanted-programstate-data-propagation.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc -verify %s
+// expected-no-diagnostics
+
+// test for PR15623
+#include "Inputs/system-header-simulator.h"
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+void free(void *);
+
+_Bool test1(void) {
+   char *param = malloc(10);
+   char *value = malloc(10);
+   _Bool ok = (param && value);
+   free(param);
+   free(value);
+   // Previously we ended up with 'Use of memory after it is freed' on return.
+   // This happened due to incorrect processing of logical AND at line
+   // '_Bool ok = (param && value);'. The ProgramState data attached to the
+   // pointers memory region by the unix.Malloc checker was propogated to the
+   // 'ok' variable by mistake.
+   return ok; // no warning
+}
Index: test/Analysis/misc-ps-region-store.m
===================================================================
--- test/Analysis/misc-ps-region-store.m
+++ test/Analysis/misc-ps-region-store.m
@@ -323,14 +323,15 @@
 void rdar_7275774(void *data, unsigned n) {
   if (!(data || n == 0))
     return;
-  
+
+  // 'data' == null, n > 0
   unsigned short *p = (unsigned short*) data;
   unsigned short *q = p + (n / 2);
 
   if (p < q) {
     // If we reach here, 'p' cannot be null.  If 'p' is null, then 'n' must
     // be '0', meaning that this branch is not feasible.
-    *p = *q; // no-warning
+    *p = *q; // expected-warning{{Dereference of null pointer (loaded from variable 'p')}}
   }
 }
 
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -602,8 +602,7 @@
       if (StTrue) {
         if (StFalse) {
           // We can't constrain the value to 0 or 1.
-          // The best we can do is a cast.
-          X = getSValBuilder().evalCast(RHSVal, B->getType(), RHS->getType());
+          X = UnknownVal();
         } else {
           // The value is known to be true.
           X = getSValBuilder().makeIntVal(1, B->getType());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D22862.65716.patch
Type: text/x-patch
Size: 2342 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160727/4657e5ad/attachment.bin>