analyzer: fix NULL deref false positives [PR94851]
authorDavid Malcolm <dmalcolm@redhat.com>
Sat, 22 Aug 2020 10:30:17 +0000 (06:30 -0400)
committerDavid Malcolm <dmalcolm@redhat.com>
Sat, 22 Aug 2020 15:08:46 +0000 (11:08 -0400)
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.

gcc/analyzer/region-model-manager.cc
gcc/testsuite/gcc.dg/analyzer/pr94851-1.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/pr94851-3.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/pr94851-4.c [new file with mode: 0644]

index 75402649a91119d5908d1c098384582895b54390..ce949322db7b53d5c0256126527cb7e7363b6c0f 100644 (file)
@@ -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 (file)
index 0000000..5657131
--- /dev/null
@@ -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 (file)
index 0000000..0f95397
--- /dev/null
@@ -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 (file)
index 0000000..2a15a5d
--- /dev/null
@@ -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;
+}
+