From patchwork Wed Jan 26 17:21:57 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 1584584 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=h4+rPj5U; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JkVt24Dzbz9t6h for ; Thu, 27 Jan 2022 04:23:05 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id AB5673851404 for ; Wed, 26 Jan 2022 17:23:02 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AB5673851404 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1643217782; bh=R0/To6euG0XZB9KEyPB8y/7ZXY+vBLDkFb0R+7QduSs=; h=Date:To:Subject:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=h4+rPj5UgbVO0yvxDW4AX3FaGSm2DFYMBgxJ4KOMfAHSNCXYr+e9tMf1ZzMJR2XDN PfXKiuuLyiBGS0sz48IQm9//ef7FJwdYulFLjH1HmgUBIZ//wkAR9CVJqk3DIdTHWx ZxkHi+I6ylh/lfLtxpS7+l5JX3A2/6/1weruWBKs= 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 52B98383F86A for ; Wed, 26 Jan 2022 17:22:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 52B98383F86A Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-32-cRXalweQMfmbc4PKv20LKw-1; Wed, 26 Jan 2022 12:22:01 -0500 X-MC-Unique: cRXalweQMfmbc4PKv20LKw-1 Received: by mail-qv1-f69.google.com with SMTP id d5-20020a0cffa5000000b004257627bf5fso262389qvv.23 for ; Wed, 26 Jan 2022 09:22:01 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=R0/To6euG0XZB9KEyPB8y/7ZXY+vBLDkFb0R+7QduSs=; b=L11ht8bZ0uHYiZSQI/qZB88fU8wbj6SI/uWmsDkpOKXMMXOToYMQCt9S3o7UvtiLW6 wQxfXt+6YVirS8BX7vdXOQMgRHUVFn8RvYNgmWZ+bYAyF5ZDdZC6hk9HN+b/utZkUflr zdMQpd57skgZ2/FQI8JwCOvpP9LirqPhvVldkLeCON2y+V/KfoAQmni0r7ZW+oNXuquD DyOkhOx4IL/rThUNMMY8/b1etwfH9ysIAfm+WAE2Uj5pkMp2xKAIlQkoLA78QQOKaT+A N1zUgZE6jrmaVn6wh9MnP7duA/nZ0WhNKGbe8U5fTvr7X2+sZgxaNaXjkQRlgOnFp3Fc ytsA== X-Gm-Message-State: AOAM531jtnRwUumkrz5fI9S3icB4Yefltf1OBz+7FzY7CFkUP5bneyx+ VkofKjCkT5ydnGaSiRBuDvOyd8dum3OO/vGypXxbkpgJoh2EEdHIoXJ5ZcQNyre2EGj6vVo7sLN Md1bVoe1TuQmh/UfvNg== X-Received: by 2002:a05:620a:c10:: with SMTP id l16mr13321711qki.87.1643217720418; Wed, 26 Jan 2022 09:22:00 -0800 (PST) X-Google-Smtp-Source: ABdhPJzRhIdDL+mU5EQXA08obhTgcei76RSh1Unq6d8LYb73xCARHxYLajnEtjCrjwjVXVrMZ2p9fg== X-Received: by 2002:a05:620a:c10:: with SMTP id l16mr13321678qki.87.1643217720077; Wed, 26 Jan 2022 09:22:00 -0800 (PST) Received: from redhat.com ([2601:184:4780:4310::7156]) by smtp.gmail.com with ESMTPSA id d22sm7136075qkn.45.2022.01.26.09.21.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Jan 2022 09:21:59 -0800 (PST) Date: Wed, 26 Jan 2022 12:21:57 -0500 To: Martin Sebor Subject: [PATCH v2] warn-access: Prevent -Wuse-after-free on ARM [PR104213] Message-ID: References: <20220125233333.695137-1-polacek@redhat.com> <72ba75eb-32a0-cbf3-fc89-0deaf37d2af5@gmail.com> MIME-Version: 1.0 In-Reply-To: <72ba75eb-32a0-cbf3-fc89-0deaf37d2af5@gmail.com> User-Agent: Mutt/2.1.5 (2021-12-30) X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, BODY_8BITS, 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Marek Polacek via Gcc-patches From: Marek Polacek Reply-To: Marek Polacek Cc: Martin Sebor , GCC Patches Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" On Wed, Jan 26, 2022 at 09:39:46AM -0700, Martin Sebor wrote: > 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. Here's what I've pushed then (it adds the testcase Martin mentioned earlier). Thanks, -- >8 -- 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. --- 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(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wuse-after-free2.C create mode 100644 gcc/testsuite/g++.dg/warn/Wuse-after-free3.C base-commit: 00d8321124123daf41f7c51526355a5a610cdeb8 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 "" 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" }