On 1/25/22 16:33, Marek Polacek via Gcc-patches wrote:
Here, -Wuse-after-free warns about using 'this' which, on ARM, cdtors
return, as mandated by the EABI. To be entirely correct, it only
requires it for C1 and C2 ctors and D2 and D1 dtors, but I don't feel
like changing that now and possibly running into issues later on.
This patch uses suppress_warning on 'this' for certain cdtor_returns_this
cases in the C++ FE, and then warn_invalid_pointer makes use of this
information and doesn't warn.
In my first attempt I tried suppress_warning the MODIFY_EXPR or RETURN_EXPR
we build in build_delete_destructor_body, but the complication is that
the suppress_warning bits don't always survive gimplification; see e.g.
gimplify_modify_expr where we do
6130 if (COMPARISON_CLASS_P (*from_p))
6131 copy_warning (assign, *from_p);
but here we're not dealing with a comparison. Removing that check
regresses uninit-pr74762.C. Adding copy_warning (assign, *expr_p)
regresses c-c++-common/uninit-17.c.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
Thanks for picking this up!
My only concern with suppressing the warning for the pointer rather
than just for the return statement is that it not lead to false
negatives. I played with the patch a little, trying to trigger one
but couldn't so it might be unfounded. Still, I'd expect targeting
the statement to be a simpler change (the warning already checks
for the suppression bit on the statement). Did you try it?
For reference, one of the test cases I tried is below. It would
be good to add it to the test suite to make sure the bug is caught
(there's a duplicate warning there that should at some point get
squashed).
struct A
{
virtual ~A ();
void f ();
};
A::~A ()
{
operator delete (this);
f ();
}
Martin
PR target/104213
gcc/cp/ChangeLog:
* decl.cc (finish_constructor_body): Suppress -Wuse-after-free.
(finish_destructor_body): Likewise.
* optimize.cc (build_delete_destructor_body): Likewise.
gcc/ChangeLog:
* gimple-ssa-warn-access.cc (pass_waccess::warn_invalid_pointer): Don't
warn when the SSA_NAME_VAR of REF has supressed -Wuse-after-free.
gcc/testsuite/ChangeLog:
* g++.dg/warn/Wuse-after-free2.C: New test.
---
gcc/cp/decl.cc | 2 ++
gcc/cp/optimize.cc | 1 +
gcc/gimple-ssa-warn-access.cc | 14 +++++++++++---
gcc/testsuite/g++.dg/warn/Wuse-after-free2.C | 10 ++++++++++
4 files changed, 24 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/warn/Wuse-after-free2.C
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 22d3dd1e2ad..6534a7fd320 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -17315,6 +17315,7 @@ finish_constructor_body (void)
add_stmt (build_stmt (input_location, LABEL_EXPR, cdtor_label));
val = DECL_ARGUMENTS (current_function_decl);
+ suppress_warning (val, OPT_Wuse_after_free);
val = build2 (MODIFY_EXPR, TREE_TYPE (val),
DECL_RESULT (current_function_decl), val);
/* Return the address of the object. */
@@ -17408,6 +17409,7 @@ finish_destructor_body (void)
tree val;
val = DECL_ARGUMENTS (current_function_decl);
+ suppress_warning (val, OPT_Wuse_after_free);
val = build2 (MODIFY_EXPR, TREE_TYPE (val),
DECL_RESULT (current_function_decl), val);
/* Return the address of the object. */
diff --git a/gcc/cp/optimize.cc b/gcc/cp/optimize.cc
index 4ad3f1dc9aa..13ab8b7361e 100644
--- a/gcc/cp/optimize.cc
+++ b/gcc/cp/optimize.cc
@@ -166,6 +166,7 @@ build_delete_destructor_body (tree delete_dtor, tree
complete_dtor)
if (targetm.cxx.cdtor_returns_this ())
{
tree val = DECL_ARGUMENTS (delete_dtor);
+ suppress_warning (val, OPT_Wuse_after_free);
val = build2 (MODIFY_EXPR, TREE_TYPE (val),
DECL_RESULT (delete_dtor), val);
add_stmt (build_stmt (0, RETURN_EXPR, val));
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index 8bc33eeb6fa..484bcd34c25 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -3880,9 +3880,17 @@ pass_waccess::warn_invalid_pointer (tree ref, gimple
*use_stmt,
bool maybe, bool equality /* = false */)
{
/* Avoid printing the unhelpful "<unknown>" in the diagnostics. */
- if (ref && TREE_CODE (ref) == SSA_NAME
- && (!SSA_NAME_VAR (ref) || DECL_ARTIFICIAL (SSA_NAME_VAR (ref))))
- ref = NULL_TREE;
+ if (ref && TREE_CODE (ref) == SSA_NAME)
+ {
+ tree var = SSA_NAME_VAR (ref);
+ if (!var)
+ ref = NULL_TREE;
+ /* Don't warn for cases like when a cdtor returns 'this' on ARM. */
+ else if (warning_suppressed_p (var, OPT_Wuse_after_free))
+ return;
+ else if (DECL_ARTIFICIAL (var))
+ ref = NULL_TREE;
+ }
location_t use_loc = gimple_location (use_stmt);
if (use_loc == UNKNOWN_LOCATION)
diff --git a/gcc/testsuite/g++.dg/warn/Wuse-after-free2.C
b/gcc/testsuite/g++.dg/warn/Wuse-after-free2.C
new file mode 100644
index 00000000000..6d5f2bf01b5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wuse-after-free2.C
@@ -0,0 +1,10 @@
+// PR target/104213
+// { dg-do compile }
+// { dg-options "-Wuse-after-free" }
+
+class C
+{
+ virtual ~C();
+};
+
+C::~C() {} // { dg-bogus "used after" }
base-commit: aeac414923aa1e87986c7fc6f9b921d89a9b86cf
