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

Reply via email to