From df2b78d407a3fe8685343f7249b9c31c7e3af44d Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Sat, 22 Aug 2020 06:30:17 -0400 Subject: [PATCH] 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. --- 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(+) create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr94851-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr94851-3.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr94851-4.c 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 +#include + +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 + +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; +} + -- 2.30.2