From patchwork Fri Feb 17 07:09:03 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexandre Oliva X-Patchwork-Id: 58372 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp741964wrn; Thu, 16 Feb 2023 23:10:01 -0800 (PST) X-Google-Smtp-Source: AK7set+2UxLOHLtMod9u1cyWJqK6QbkzJdPJpt4YSZL3DflsasWY2rxBrXyuxXPHyM4DiOuSCnkd X-Received: by 2002:a17:906:af66:b0:8aa:c358:fc01 with SMTP id os6-20020a170906af6600b008aac358fc01mr8806683ejb.67.1676617801800; Thu, 16 Feb 2023 23:10:01 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1676617801; cv=none; d=google.com; s=arc-20160816; b=NXQM3vMei9idb2OTsJFIamKsaHI3HK3IVz00qE9ZQ9j5HIzfYP67AZS2d+RqyfN4F3 8I8gIuRqmcgL3dHE5JanFmmgWBFbbLCEpRNoSNjFFpOPpwiVGODxMyoDKK61M+SjRAEP Yaa+3Q0dz8B1Wg1L4b7NMeBfubpPbLy2Cuy3Ay//crR+tnQwpLtpzt6eenzHK+l+Ewtu nKSkrpK//orwoPTfZI5QVpgpsSy7Mq2BMwWj05rBbMre5iVeb8eNPzL748fCFLfhFFWQ b65/kSd5/iGzCCfiKMIUyRAZQflZjeOJpIAhcpMdQVSWmT8gOXfZ9Rx/6kY/zRvrasNC 5imA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:mime-version :user-agent:message-id:date:organization:subject:cc:to:dmarc-filter :delivered-to:dkim-signature:dkim-filter; bh=IwjiRfef8niTO9DWTr7b/QUMS/PT7bLNZsRzUAAgamE=; b=vfEvBMFwtVlE5ZO0OgulUDP2VDGC7RT+LJcsGXz/EIoZqOO/FkbHAO2dLNxUeYyoDP yBcSX2x6pMp4BtGxSywEREznvWYEv7WbT0GHOc/EdxuIpgIVcInLdbYtoUhISn49dQtc IIwenfV8nUH+5FP4mq0A+zvUi8epE9g3FkBLq1JktFx+Hhndf8U9lQt1m9burzgLASZO ZMlCl19h/ekVVI5ytebFz7wzxfgkw47uKqAGf8gSl5ieHA/rYoP+aSilQoukDew8h/iJ 3JIMOGIizhaAXPMU7/TPk8RlXahYg5/dVP6/VaVgyboG52qTwd64COq/6pO3n6nn1RgS ciCA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=hxKbPJTU; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id om19-20020a170907a19300b008b3b05ca5e1si465678ejc.375.2023.02.16.23.10.01 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 Feb 2023 23:10:01 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=hxKbPJTU; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9A2FB3858C39 for ; Fri, 17 Feb 2023 07:10:00 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9A2FB3858C39 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1676617800; bh=IwjiRfef8niTO9DWTr7b/QUMS/PT7bLNZsRzUAAgamE=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=hxKbPJTUxGIW77WUa9sD/ig4KKn3efa3ByyE7vfBTY3DJJ+V4PHO8PDUCrsCj4tyF UAuQ8SIaBdjPlNoKj98W9zUH/wTBzV+3HgOAbgMkR8odi6e/aMDJcWSv5RP9Sq+XoJ vPDC7wztAFY9o2+lm5GGk5SIeuVOUYyqsApH4cqQ= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from rock.gnat.com (rock.gnat.com [205.232.38.15]) by sourceware.org (Postfix) with ESMTPS id 7CB903858D33 for ; Fri, 17 Feb 2023 07:09:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7CB903858D33 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 432A6116B59; Fri, 17 Feb 2023 02:09:17 -0500 (EST) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id L+IN03LxxUor; Fri, 17 Feb 2023 02:09:17 -0500 (EST) Received: from free.home (tron.gnat.com [IPv6:2620:20:4000:0:46a8:42ff:fe0e:e294]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPS id D08F8116965; Fri, 17 Feb 2023 02:09:16 -0500 (EST) Received: from livre (livre.home [172.31.160.2]) by free.home (8.15.2/8.15.2) with ESMTPS id 31H793Th090987 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Fri, 17 Feb 2023 04:09:03 -0300 To: gcc-patches@gcc.gnu.org Cc: msebor@redhat.com Subject: [PATCH] -Wdangling-pointer: don't mark SSA lhs sets as stores Organization: Free thinker, does not speak for AdaCore Date: Fri, 17 Feb 2023 04:09:03 -0300 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Alexandre Oliva via Gcc-patches From: Alexandre Oliva Reply-To: Alexandre Oliva Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1758061188242139727?= X-GMAIL-MSGID: =?utf-8?q?1758061188242139727?= 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. Regstrapped on x86_64-linux-gnu. Tested on arm-vxworks7 (gcc-12) and arm-eabi (trunk). Ok to install? 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. --- gcc/gimple-ssa-warn-access.cc | 3 ++ gcc/testsuite/c-c++-common/Wdangling-pointer-5.c | 4 ++- gcc/testsuite/g++.dg/warn/Wdangling-pointer.C | 29 +++++++++++++++++++++- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc index 2eab1d59abd05..c0efb3fdb4e52 100644 --- a/gcc/gimple-ssa-warn-access.cc +++ b/gcc/gimple-ssa-warn-access.cc @@ -4511,7 +4511,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 2a165cea76768..cb6da9e86394d 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 22c559e4adafe..a94477a647666 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); }