From: David Malcolm <dmalcolm@redhat.com>
Date: Sat, 22 Aug 2020 10:30:17 +0000 (-0400)
Subject: analyzer: fix NULL deref false positives [PR94851]
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=df2b78d407a3fe8685343f7249b9c31c7e3af44d;p=gcc.git

analyzer: fix NULL deref false positives [PR94851]

PR analyzer/94851 reports various false "NULL dereference" diagnostics.
The first case (comment #1) affects GCC 10.2 but no longer affects
trunk; I believe it was fixed by the state rewrite of
r11-2694-g808f4dfeb3a95f50f15e71148e5c1067f90a126d.

The patch adds a regression test for this case.

The other cases (comment #3 and comment #4) still affect trunk.
In both cases, the && in a conditional is optimized to bitwise &
  _1 = p_4 != 0B;
  _2 = p_4 != q_6(D);
  _3 = _1 & _2;
and the analyzer fails to fold this for the case where one (or both) of
the conditionals is false, and thus erroneously considers the path where
"p" is non-NULL despite being passed a NULL value.

Fix this by implementing folding for this case.

gcc/analyzer/ChangeLog:
	PR analyzer/94851
	* region-model-manager.cc
	(region_model_manager::maybe_fold_binop): Fold bitwise "& 0" to 0.

gcc/testsuite/ChangeLog:
	PR analyzer/94851
	* gcc.dg/analyzer/pr94851-1.c: New test.
	* gcc.dg/analyzer/pr94851-3.c: New test.
	* gcc.dg/analyzer/pr94851-4.c: New test.
---

diff --git a/gcc/analyzer/region-model-manager.cc b/gcc/analyzer/region-model-manager.cc
index 75402649a91..ce949322db7 100644
--- a/gcc/analyzer/region-model-manager.cc
+++ b/gcc/analyzer/region-model-manager.cc
@@ -451,6 +451,12 @@ region_model_manager::maybe_fold_binop (tree type, enum tree_code op,
       if (cst1 && integer_onep (cst1))
 	return arg0;
       break;
+    case BIT_AND_EXPR:
+      if (cst1)
+	if (zerop (cst1) && INTEGRAL_TYPE_P (type))
+	  /* "(ARG0 & 0)" -> "0".  */
+	  return get_or_create_constant_svalue (build_int_cst (type, 0));
+      break;
     case TRUTH_ANDIF_EXPR:
     case TRUTH_AND_EXPR:
       if (cst1)
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c b/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c
new file mode 100644
index 00000000000..56571318f91
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr94851-1.c
@@ -0,0 +1,46 @@
+/* { dg-additional-options "-O2" } */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+typedef struct AMARK {
+  struct AMARK *m_next;
+  char m_name;
+} AMARK;
+
+struct buf {
+  AMARK *b_amark;
+};
+
+struct buf *curbp;
+
+int pamark(void) {
+  int c;
+  AMARK *p = curbp->b_amark;
+  AMARK *last = curbp->b_amark;
+
+  c = getchar();
+
+  while (p != (AMARK *)NULL && p->m_name != (char)c) {
+    last = p;
+    p = p->m_next;
+  }
+
+  if (p != (AMARK *)NULL) {
+    printf("over writing mark %c\n", c);
+  } else {
+    if ((p = (AMARK *)malloc(sizeof(AMARK))) == (AMARK *)NULL)
+      return 0;
+
+    p->m_next = (AMARK *)NULL;
+
+    if (curbp->b_amark == (AMARK *)NULL)
+      curbp->b_amark = p;
+    else
+      last->m_next = p;
+  }
+
+  p->m_name = (char)c;
+
+  return 1;
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr94851-3.c b/gcc/testsuite/gcc.dg/analyzer/pr94851-3.c
new file mode 100644
index 00000000000..0f953970b00
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr94851-3.c
@@ -0,0 +1,20 @@
+/* { dg-additional-options "-O1" } */
+
+struct List {
+    struct List *next;
+};
+
+void foo(struct List *p, struct List *q)
+{
+    while (p && p != q){
+        p = p->next;
+    }
+}
+
+int main()
+{
+    struct List x = {0};
+    foo(0, &x);
+    return 0;
+}
+
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr94851-4.c b/gcc/testsuite/gcc.dg/analyzer/pr94851-4.c
new file mode 100644
index 00000000000..2a15a5d7f5b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr94851-4.c
@@ -0,0 +1,24 @@
+/* { dg-additional-options "-O2" } */
+
+#include <stdlib.h>
+
+struct List {
+    struct List *next;
+};
+
+void foo(struct List *p, struct List *q)
+{
+    while (p && p != q){
+        struct List *next = p->next;
+        free(p);
+        p = next;
+    }
+}
+
+int main()
+{
+    struct List x = {0};
+    foo(NULL, &x);
+    return 0;
+}
+