public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Marek Polacek <mpolacek@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc r12-6880] warn-access: Prevent -Wuse-after-free on ARM [PR104213]
Date: Wed, 26 Jan 2022 17:22:26 +0000 (GMT)	[thread overview]
Message-ID: <20220126172226.A7069383F86E@sourceware.org> (raw)

https://gcc.gnu.org/g:7bd1e1296cc36b558a27bbe09352c5c2aca4c5d5

commit r12-6880-g7bd1e1296cc36b558a27bbe09352c5c2aca4c5d5
Author: Marek Polacek <polacek@redhat.com>
Date:   Tue Jan 25 15:12:51 2022 -0500

    warn-access: Prevent -Wuse-after-free on ARM [PR104213]
    
    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.
    
            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.
            * g++.dg/warn/Wuse-after-free3.C: New test.

Diff:
---
 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 ++++++++++
 gcc/testsuite/g++.dg/warn/Wuse-after-free3.C | 16 ++++++++++++++++
 5 files changed, 40 insertions(+), 3 deletions(-)

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 afcf0f71bec..3dcaf4230b8 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" }
diff --git a/gcc/testsuite/g++.dg/warn/Wuse-after-free3.C b/gcc/testsuite/g++.dg/warn/Wuse-after-free3.C
new file mode 100644
index 00000000000..1862ac8b09d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wuse-after-free3.C
@@ -0,0 +1,16 @@
+// PR target/104213
+// { dg-do compile }
+// { dg-options "-Wuse-after-free" }
+// FIXME: We should not output the warning twice.
+
+struct A
+{
+  virtual ~A ();
+  void f ();
+};
+
+A::~A ()
+{
+  operator delete (this);
+  f (); // { dg-warning "used after" }
+} // { dg-warning "used after" }


                 reply	other threads:[~2022-01-26 17:22 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20220126172226.A7069383F86E@sourceware.org \
    --to=mpolacek@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    /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).