From: Jason Merrill <jason@redhat.com>
To: Richard Biener <rguenther@suse.de>, gcc-patches@gcc.gnu.org
Cc: Jakub Jelinek <jakub@redhat.com>
Subject: Re: [PATCH] tree-optimization/104475 - bogus -Wstringop-overflow
Date: Tue, 17 Jan 2023 12:37:31 -0500 [thread overview]
Message-ID: <ac07ac48-4b00-d9a8-74d8-e89dbb668d7d@redhat.com> (raw)
In-Reply-To: <20221207112501.989ED136B4@imap1.suse-dmz.suse.de>
On 12/7/22 06:25, Richard Biener wrote:
> The following avoids a bogus -Wstringop-overflow diagnostic by
> properly recognizing that &d->m_mutex cannot be nullptr in C++
> even if m_mutex is at offset zero. The frontend already diagnoses
> a &d->m_mutex != nullptr comparison and the following transfers
> this knowledge to the middle-end which sees &d->m_mutex as
> simple pointer arithmetic. The new ADDR_NONZERO flag on an
> ADDR_EXPR is used to carry this information and it's checked in
> the tree_expr_nonzero_p API which causes this to be folded early.
>
> To avoid the bogus diagnostic this avoids separating the nullptr
> path via jump-threading by eliminating the nullptr check.
>
> I'd appreciate C++ folks picking this up and put the flag on
> the appropriate ADDR_EXPRs - I've tried avoiding to put it on
> all of them and didn't try hard to mimick what -Waddress warns
> on (the code is big, maybe some refactoring would help but also
> not sure what exactly the C++ standard constraints are here).
This is allowed by the standard, at least after CWG2535, but we need to
check -fsanitize=null before asserting that the address is non-null.
With that elaboration, a flag on the ADDR_EXPR may not be a convenient
way to express the property?
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>
> Thanks,
> Richard.
>
> PR tree-optimization/104475
> gcc/
> * tree-core.h: Document use of nothrow_flag on ADDR_EXPR.
> * tree.h (ADDR_NONZERO): New.
> * fold-const.cc (tree_single_nonzero_warnv_p): Check
> ADDR_NONZERO.
>
> gcc/cp/
> * typeck.cc (cp_build_addr_expr_1): Set ADDR_NONZERO
> on the built address if it is of a COMPONENT_REF.
>
> * g++.dg/opt/pr104475.C: New testcase.
> ---
> gcc/cp/typeck.cc | 3 +++
> gcc/fold-const.cc | 4 +++-
> gcc/testsuite/g++.dg/opt/pr104475.C | 12 ++++++++++++
> gcc/tree-core.h | 3 +++
> gcc/tree.h | 4 ++++
> 5 files changed, 25 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/g++.dg/opt/pr104475.C
>
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index 7dfe5acc67e..3563750803e 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -7232,6 +7232,9 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue, tsubst_flags_t complain)
> gcc_assert (same_type_ignoring_top_level_qualifiers_p
> (TREE_TYPE (object), decl_type_context (field)));
> val = build_address (arg);
> + if (TREE_CODE (val) == ADDR_EXPR
> + && TREE_CODE (TREE_OPERAND (val, 0)) == COMPONENT_REF)
> + ADDR_NONZERO (val) = 1;
> }
>
> if (TYPE_PTR_P (argtype)
> diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
> index e80be8049e1..cdfe3f50ae3 100644
> --- a/gcc/fold-const.cc
> +++ b/gcc/fold-const.cc
> @@ -15308,8 +15308,10 @@ tree_single_nonzero_warnv_p (tree t, bool *strict_overflow_p)
>
> case ADDR_EXPR:
> {
> - tree base = TREE_OPERAND (t, 0);
> + if (ADDR_NONZERO (t))
> + return true;
>
> + tree base = TREE_OPERAND (t, 0);
> if (!DECL_P (base))
> base = get_base_address (base);
>
> diff --git a/gcc/testsuite/g++.dg/opt/pr104475.C b/gcc/testsuite/g++.dg/opt/pr104475.C
> new file mode 100644
> index 00000000000..013c70302c6
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/opt/pr104475.C
> @@ -0,0 +1,12 @@
> +// { dg-do compile }
> +// { dg-require-effective-target c++11 }
> +// { dg-options "-O -Waddress -fdump-tree-original" }
> +
> +struct X { int i; };
> +
> +bool foo (struct X *p)
> +{
> + return &p->i != nullptr; /* { dg-warning "never be NULL" } */
> +}
> +
> +/* { dg-final { scan-tree-dump "return <retval> = 1;" "original" } } */
> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
> index e146b133dbd..303e25b5df6 100644
> --- a/gcc/tree-core.h
> +++ b/gcc/tree-core.h
> @@ -1376,6 +1376,9 @@ struct GTY(()) tree_base {
> TREE_THIS_NOTRAP in
> INDIRECT_REF, MEM_REF, TARGET_MEM_REF, ARRAY_REF, ARRAY_RANGE_REF
>
> + ADDR_NONZERO in
> + ADDR_EXPR
> +
> SSA_NAME_IN_FREE_LIST in
> SSA_NAME
>
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 23223ca0c87..1c810c0b21b 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -876,6 +876,10 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
> (TREE_CHECK5 (NODE, INDIRECT_REF, MEM_REF, TARGET_MEM_REF, ARRAY_REF, \
> ARRAY_RANGE_REF)->base.nothrow_flag)
>
> +/* Nozero means this ADDR_EXPR is not equal to NULL. */
> +#define ADDR_NONZERO(NODE) \
> + (TREE_CHECK (NODE, ADDR_EXPR)->base.nothrow_flag)
> +
> /* In a VAR_DECL, PARM_DECL or FIELD_DECL, or any kind of ..._REF node,
> nonzero means it may not be the lhs of an assignment.
> Nonzero in a FUNCTION_DECL means this function should be treated
next prev parent reply other threads:[~2023-01-17 17:37 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-07 11:25 Richard Biener
2023-01-17 17:37 ` Jason Merrill [this message]
2023-01-18 8:06 ` Richard Biener
2023-01-19 20:53 ` Jason Merrill
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ac07ac48-4b00-d9a8-74d8-e89dbb668d7d@redhat.com \
--to=jason@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=rguenther@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).