diff mbox series

[committed] analyzer: assume that POINTER_PLUS_EXPR of non-NULL is non-NULL [PR101962]

Message ID 20210823181953.193899-1-dmalcolm@redhat.com
State New
Headers show
Series [committed] analyzer: assume that POINTER_PLUS_EXPR of non-NULL is non-NULL [PR101962] | expand

Commit Message

David Malcolm Aug. 23, 2021, 6:19 p.m. UTC
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r12-3094-ge82e0f149b0aba660896ea9aa12c442c07a16d12.

gcc/analyzer/ChangeLog:
	PR analyzer/101962
	* region-model.cc (region_model::eval_condition_without_cm):
	Refactor comparison against zero, adding a check for
	POINTER_PLUS_EXPR of non-NULL.

gcc/testsuite/ChangeLog:
	PR analyzer/101962
	* gcc.dg/analyzer/data-model-23.c: New test.
	* gcc.dg/analyzer/pr101962.c: New test.
---
 gcc/analyzer/region-model.cc                  | 73 ++++++++++++-------
 gcc/testsuite/gcc.dg/analyzer/data-model-23.c | 26 +++++++
 gcc/testsuite/gcc.dg/analyzer/pr101962.c      | 51 +++++++++++++
 3 files changed, 122 insertions(+), 28 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/data-model-23.c
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr101962.c
diff mbox series

Patch

diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 9870007e57e..f54be14e639 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -2488,34 +2488,51 @@  region_model::eval_condition_without_cm (const svalue *lhs,
     if (const constant_svalue *cst_rhs = rhs->dyn_cast_constant_svalue ())
       return constant_svalue::eval_condition (cst_lhs, op, cst_rhs);
 
-  /* Handle comparison of a region_svalue against zero.  */
-
-  if (const region_svalue *ptr = lhs->dyn_cast_region_svalue ())
-    if (const constant_svalue *cst_rhs = rhs->dyn_cast_constant_svalue ())
-      if (zerop (cst_rhs->get_constant ()))
-	{
-	  /* A region_svalue is a non-NULL pointer, except in certain
-	     special cases (see the comment for region::non_null_p.  */
-	  const region *pointee = ptr->get_pointee ();
-	  if (pointee->non_null_p ())
-	    {
-	      switch (op)
-		{
-		default:
-		  gcc_unreachable ();
-
-		case EQ_EXPR:
-		case GE_EXPR:
-		case LE_EXPR:
-		  return tristate::TS_FALSE;
-
-		case NE_EXPR:
-		case GT_EXPR:
-		case LT_EXPR:
-		  return tristate::TS_TRUE;
-		}
-	    }
-	}
+  /* Handle comparison against zero.  */
+  if (const constant_svalue *cst_rhs = rhs->dyn_cast_constant_svalue ())
+    if (zerop (cst_rhs->get_constant ()))
+      {
+	if (const region_svalue *ptr = lhs->dyn_cast_region_svalue ())
+	  {
+	    /* A region_svalue is a non-NULL pointer, except in certain
+	       special cases (see the comment for region::non_null_p).  */
+	    const region *pointee = ptr->get_pointee ();
+	    if (pointee->non_null_p ())
+	      {
+		switch (op)
+		  {
+		  default:
+		    gcc_unreachable ();
+
+		  case EQ_EXPR:
+		  case GE_EXPR:
+		  case LE_EXPR:
+		    return tristate::TS_FALSE;
+
+		  case NE_EXPR:
+		  case GT_EXPR:
+		  case LT_EXPR:
+		    return tristate::TS_TRUE;
+		  }
+	      }
+	  }
+	else if (const binop_svalue *binop = lhs->dyn_cast_binop_svalue ())
+	  {
+	    /* Treat offsets from a non-NULL pointer as being non-NULL.  This
+	       isn't strictly true, in that eventually ptr++ will wrap
+	       around and be NULL, but it won't occur in practise and thus
+	       can be used to suppress effectively false positives that we
+	       shouldn't warn for.  */
+	    if (binop->get_op () == POINTER_PLUS_EXPR)
+	      {
+		tristate lhs_ts
+		  = eval_condition_without_cm (binop->get_arg0 (),
+					       op, rhs);
+		if (lhs_ts.is_known ())
+		  return lhs_ts;
+	      }
+	  }
+      }
 
   /* Handle rejection of equality for comparisons of the initial values of
      "external" values (such as params) with the address of locals.  */
diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-23.c b/gcc/testsuite/gcc.dg/analyzer/data-model-23.c
new file mode 100644
index 00000000000..c76dd4ed31e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/data-model-23.c
@@ -0,0 +1,26 @@ 
+#include "analyzer-decls.h"
+
+#define NULL ((void *)0)
+
+void * __attribute__((noinline))
+hide (void *ptr)
+{
+  return ptr;
+}
+
+void test_1 (void)
+{
+  int a;
+  __analyzer_eval (hide (&a) == NULL); /* { dg-warning "FALSE" } */
+  __analyzer_eval (hide (&a) + 1 != NULL); /* { dg-warning "TRUE" } */
+  __analyzer_eval (hide (&a) + 1 == NULL); /* { dg-warning "FALSE" } */
+  __analyzer_eval (hide (&a) - 1 != NULL); /* { dg-warning "TRUE" } */
+  __analyzer_eval (hide (&a) - 1 == NULL); /* { dg-warning "FALSE" } */
+}
+
+void test_2 (void)
+{
+  __analyzer_eval (hide (NULL) == NULL); /* { dg-warning "TRUE" } */
+  __analyzer_eval (hide (NULL) - 1 == NULL); /* { dg-warning "FALSE" } */
+  __analyzer_eval (hide (NULL) + 1 == NULL); /* { dg-warning "FALSE" } */
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr101962.c b/gcc/testsuite/gcc.dg/analyzer/pr101962.c
new file mode 100644
index 00000000000..7b83d0345b7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr101962.c
@@ -0,0 +1,51 @@ 
+#include "analyzer-decls.h"
+
+#define NULL ((void *)0)
+
+/* Verify that the analyzer makes the simplifying assumption that we don't
+   hit NULL when incrementing pointers to non-NULL memory regions.  */
+
+static int * __attribute__((noinline))
+maybe_inc_int_ptr (int *ptr)
+{
+  if (!ptr)
+    return NULL;
+  return ++ptr;
+}
+
+int
+test_1 (void)
+{
+  int stack;
+  int *a = &stack;
+  a = maybe_inc_int_ptr (a);
+  a = maybe_inc_int_ptr (a);
+  __analyzer_eval (a == NULL); /* { dg-warning "FALSE" } */
+  __analyzer_eval (a != NULL); /* { dg-warning "TRUE" } */
+  return *a; /* { dg-warning "use of uninitialized value '\\*a'" } */
+  /* TODO: a complaint about out-of-bounds would be a better warning.  */
+}
+
+static const char * __attribute__((noinline))
+maybe_inc_char_ptr (const char *ptr)
+{
+  if (!ptr)
+    return NULL;
+  return ++ptr;
+}
+
+char
+test_s (void)
+{
+  const char *msg = "hello world";
+  const char *a = msg;
+  __analyzer_eval (*a == 'h'); /* { dg-warning "TRUE" } */
+  a = maybe_inc_char_ptr (a);
+  __analyzer_eval (*a == 'e'); /* { dg-warning "TRUE" } */
+  a = maybe_inc_char_ptr (a);
+  __analyzer_eval (*a == 'l'); /* { dg-warning "TRUE" } */
+  a = maybe_inc_char_ptr (a);
+  __analyzer_eval (*a == 'l'); /* { dg-warning "TRUE" } */
+  a = maybe_inc_char_ptr (a);
+  __analyzer_eval (*a == 'o'); /* { dg-warning "TRUE" } */
+}