aboutsummaryrefslogtreecommitdiff
path: root/gcc
diff options
context:
space:
mode:
authorAlexandre Oliva <oliva@adacore.com>2023-03-03 15:59:30 -0300
committerAlexandre Oliva <oliva@gnu.org>2023-03-03 15:59:30 -0300
commitfdac2bea53bf5e7214352e2afd5542254c3156cb (patch)
treedd10e2566993f448d0cdac814183ba9ed65a2351 /gcc
parentaee43d26e8f9bb6e4d18a4075cbec9f6e5171dff (diff)
downloadgcc-fdac2bea53bf5e7214352e2afd5542254c3156cb.zip
gcc-fdac2bea53bf5e7214352e2afd5542254c3156cb.tar.gz
gcc-fdac2bea53bf5e7214352e2afd5542254c3156cb.tar.bz2
-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.
Diffstat (limited to 'gcc')
-rw-r--r--gcc/gimple-ssa-warn-access.cc3
-rw-r--r--gcc/testsuite/c-c++-common/Wdangling-pointer-5.c4
-rw-r--r--gcc/testsuite/g++.dg/warn/Wdangling-pointer.C29
3 files changed, 32 insertions, 4 deletions
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index a28fce1..8b1c1cc 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -4515,7 +4515,8 @@ pass_waccess::check_dangling_stores (basic_block bb,
use the escaped locals. */
return;
- if (!is_gimple_assign (stmt) || gimple_clobber_p (stmt))
+ if (!is_gimple_assign (stmt) || gimple_clobber_p (stmt)
+ || !gimple_store_p (stmt))
continue;
access_ref lhs_ref;
diff --git a/gcc/testsuite/c-c++-common/Wdangling-pointer-5.c b/gcc/testsuite/c-c++-common/Wdangling-pointer-5.c
index 2a165ce..cb6da9e 100644
--- a/gcc/testsuite/c-c++-common/Wdangling-pointer-5.c
+++ b/gcc/testsuite/c-c++-common/Wdangling-pointer-5.c
@@ -75,9 +75,9 @@ void nowarn_store_arg_store (void **vpp)
void* nowarn_store_arg_store_arg (void **vpp1, void **vpp2)
{
- int x;
+ int x; // { dg-message "'x' declared here" }
void **p = (void**)sink (0);
- *vpp1 = &x; // warn here?
+ *vpp1 = &x; // { dg-warning "storing the address of local variable 'x' in '\\*vpp1'" }
*vpp2 = 0; // might overwrite *vpp1
return p;
}
diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-pointer.C b/gcc/testsuite/g++.dg/warn/Wdangling-pointer.C
index 22c559e..a94477a 100644
--- a/gcc/testsuite/g++.dg/warn/Wdangling-pointer.C
+++ b/gcc/testsuite/g++.dg/warn/Wdangling-pointer.C
@@ -35,7 +35,34 @@ void warn_init_ref_member ()
{ }
} ai;
- sink (&as, &ai);
+ struct Al
+ {
+ const S &sref;
+ Al ():
+ // The temporary S object is destroyed when Al::Al() returns.
+ sref (S ()) // { dg-warning "storing the address" }
+ {
+ // Copying this to an SSA_NAME used to disable the warning:
+ Al *ptr = this;
+ asm ("" : "+r" (ptr));
+ }
+ } al;
+
+ struct An
+ {
+ An *next;
+ const S &sref;
+ An ():
+ next (0),
+ // The temporary S object is destroyed when An::An() returns.
+ sref (S ()) // { dg-warning "storing the address" "" { xfail *-*-* } }
+ {
+ // ??? Writing to another part of *this disables the warning:
+ next = 0;
+ }
+ } an;
+
+ sink (&as, &ai, &al, &an);
}