]> git.ipfire.org Git - thirdparty/gcc.git/commit
-Wdangling-pointer: don't mark SSA lhs sets as stores
authorAlexandre Oliva <oliva@adacore.com>
Fri, 3 Mar 2023 18:59:30 +0000 (15:59 -0300)
committerAlexandre Oliva <oliva@gnu.org>
Fri, 3 Mar 2023 18:59:30 +0000 (15:59 -0300)
commitfdac2bea53bf5e7214352e2afd5542254c3156cb
treedd10e2566993f448d0cdac814183ba9ed65a2351
parentaee43d26e8f9bb6e4d18a4075cbec9f6e5171dff
-Wdangling-pointer: don't mark SSA lhs sets as stores

check_dangling_stores has some weirdnesses that causes its behavior to
change when the target ABI requires C++ ctors to return this: while
scanning stmts backwards in e.g. the AS ctor on a target that returns
this in ctors, the scan first encounters a copy of this to the SSA
name used to hold the return value.  m_ptr_query.get_ref resolves lhs
(the return SSA name) to the rhs (the default SSA name for this), does
not skip it because auto_var_p is false for SSA_NAMEs, and proceeds to
add it to stores, which seems to prevent later attempts to add stores
into *this from succeeding, which disables warnings that should have
triggered.

This is also the case when the backwards search finds unrelated stores
to other fields of *this before it reaches stores that IMHO should be
warned about.  The store found first disables checking of other
stores, as if the store appearing later in the code would necessarily
overwrite the store that should be warned about.  I've added an
xfailed variant of the existing test (struct An) that triggers this
problem, but I'm not sure how to go about fixing it.

Meanwhile, this patch prevents assignments with SSA_NAMEs in the lhs
from being regarded as stores, which is enough to remove the
undesirable side effect on -Wdangling-pointer of ABI-mandated ctors'
returning this.  Another variant of the existing test (struct Al) that
demonstrates the problem regardless of this aspect of the ABI, and
that gets the desired warning with the proposed patch, but not
without.

Curiously, this fix exposes yet another problem in
Wdangling-pointer-5.c: it is the return stmt of the unrelated pointer
p, not the store into possibly-overlapping *vpp2, that caused the
warning to not be issued for the store in *vpp1.  I'm not sure whether
we should or should not warn in that case, but this patch adjusts the
test to reflect the behavior change.

for  gcc/ChangeLog

* gimple-ssa-warn-access.cc
(pass_waccess::check_dangling_stores): Skip non-stores.

for  gcc/testsuite/ChangeLog

* g++.dg/warn/Wdangling-pointer.C (warn_init_ref_member): Add
two new variants, one fixed, one xfailed.
* c-c++-common/Wdangling-pointer-5.c
(nowarn_store_arg_store_arg): Add now-expected warnings.
gcc/gimple-ssa-warn-access.cc
gcc/testsuite/c-c++-common/Wdangling-pointer-5.c
gcc/testsuite/g++.dg/warn/Wdangling-pointer.C