From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1725) id B40C6386EC3C; Fri, 28 Aug 2020 19:56:06 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B40C6386EC3C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1598644566; bh=bmF2xc1NZfTuY6Fb9QPBKWnxAewvdzNvweL5qTXC3rs=; h=From:To:Subject:Date:From; b=DJ6vfSd3IbSiT+fj7byWCJifMM5q2mCX3ckTd1KEUoN8vePrW1wppeCgRC1kWlLUz lZhfY/EqUxRT8WwoAk4Wa7MzD21UwT4SHgtBrxj26drBqY4PeR/irjr+AQ+0WNL4vb bnPrvQ1PTyaDl5xiXWud1G1GLC3b4ZLJuHOM5zvc= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: William Schmidt To: gcc-cvs@gcc.gnu.org Subject: [gcc(refs/users/wschmidt/heads/builtins3)] analyzer: fix NULL deref false positives [PR94851] X-Act-Checkin: gcc X-Git-Author: David Malcolm X-Git-Refname: refs/users/wschmidt/heads/builtins3 X-Git-Oldrev: c199723d7ed0032db095abc75b82a9710eaa5e56 X-Git-Newrev: df2b78d407a3fe8685343f7249b9c31c7e3af44d Message-Id: <20200828195606.B40C6386EC3C@sourceware.org> Date: Fri, 28 Aug 2020 19:56:06 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Aug 2020 19:56:06 -0000 https://gcc.gnu.org/g:df2b78d407a3fe8685343f7249b9c31c7e3af44d commit df2b78d407a3fe8685343f7249b9c31c7e3af44d Author: David Malcolm 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 +#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; +} +