public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc(refs/users/wschmidt/heads/builtins3)] analyzer: fix NULL deref false positives [PR94851]
@ 2020-08-28 19:56 William Schmidt
  0 siblings, 0 replies; only message in thread
From: William Schmidt @ 2020-08-28 19:56 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:df2b78d407a3fe8685343f7249b9c31c7e3af44d

commit df2b78d407a3fe8685343f7249b9c31c7e3af44d
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Sat Aug 22 06:30:17 2020 -0400

    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:
---
 gcc/analyzer/region-model-manager.cc      |  6 ++++
 gcc/testsuite/gcc.dg/analyzer/pr94851-1.c | 46 +++++++++++++++++++++++++++++++
 gcc/testsuite/gcc.dg/analyzer/pr94851-3.c | 20 ++++++++++++++
 gcc/testsuite/gcc.dg/analyzer/pr94851-4.c | 24 ++++++++++++++++
 4 files changed, 96 insertions(+)

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;
+}
+


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2020-08-28 19:56 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-28 19:56 [gcc(refs/users/wschmidt/heads/builtins3)] analyzer: fix NULL deref false positives [PR94851] William Schmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).