Message ID | 20220125233333.695137-1-polacek@redhat.com |
---|---|
State | Committed |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9F3F7385ED4A for <patchwork@sourceware.org>; Tue, 25 Jan 2022 23:34:22 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9F3F7385ED4A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1643153662; bh=CybYuZIsd50+wiV79cFOkKa8BR4kFuNV1ci4ChPogCM=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=vXIE5kxjmBJcfklq41BGnkxTEjEZL2xmudPVPgm5ZmB9UDK7hM+PbyKHGqHKTPGA4 GlGMzSA6zpguA3iISuHhHeCzn3gZR/vh5BbVHyS/ZPCm0edmNzBLPTidxvRUr8HZn4 dX+IdwwBAcAh7eErKLM7cGUpPZLH+nZK/rS/VUA8= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id ED269385380A for <gcc-patches@gcc.gnu.org>; Tue, 25 Jan 2022 23:33:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org ED269385380A Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-14-KZ64Jn4dN1ON17jeWlC31w-1; Tue, 25 Jan 2022 18:33:50 -0500 X-MC-Unique: KZ64Jn4dN1ON17jeWlC31w-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 386182F26 for <gcc-patches@gcc.gnu.org>; Tue, 25 Jan 2022 23:33:49 +0000 (UTC) Received: from pdp-11.hsd1.ma.comcast.net (unknown [10.22.10.195]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1EC1C78C2A; Tue, 25 Jan 2022 23:33:41 +0000 (UTC) To: GCC Patches <gcc-patches@gcc.gnu.org> Subject: [PATCH] warn-access: Prevent -Wuse-after-free on ARM [PR104213] Date: Tue, 25 Jan 2022 18:33:33 -0500 Message-Id: <20220125233333.695137-1-polacek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" X-Spam-Status: No, score=-13.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Marek Polacek <polacek@redhat.com> Cc: Martin Sebor <msebor@redhat.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
warn-access: Prevent -Wuse-after-free on ARM [PR104213]
|
|
Commit Message
Marek Polacek
Jan. 25, 2022, 11:33 p.m. UTC
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? 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 base-commit: aeac414923aa1e87986c7fc6f9b921d89a9b86cf
Comments
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
On Tue, Jan 25, 2022 at 05:35:20PM -0700, Martin Sebor wrote: > 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? I did -- that is the "In my first attempt..." paragraph in my patch (we don't use copy_warning to propagate the suppress_warning bits). Maybe it'd be a more correct way to approach this, that is, somehow tweak gimplify_modify_expr to copy_warning for this special case. > 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 (); > } Sure, happy to add it. > 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 > Marek
On 1/25/22 18:33, Marek Polacek 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? OK if Martin doesn't have any suggestions. > 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
On 1/26/22 08:24, Jason Merrill via Gcc-patches wrote: > On 1/25/22 18:33, Marek Polacek 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? > > OK if Martin doesn't have any suggestions. Nothing further from me. Thanks 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 >
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" }