From patchwork Wed Jul 21 21:29:59 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 1508470 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: 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=RDUjU4NU; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (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 ozlabs.org (Postfix) with ESMTPS id 4GVTKk2X6wz9sWw for ; Thu, 22 Jul 2021 07:31:22 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6A3B3383D02F for ; Wed, 21 Jul 2021 21:31:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6A3B3383D02F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1626903079; bh=yDEjTOUabY6FaDktujo04shHx72Zi+G+z0KXQwALBIA=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=RDUjU4NUPtSIuqTtjaDPr8LKs/03gaW9MxmD4HoPSxqwYZTFnmcDJgYYtWoPr/b0q xYWNR1zd8NfVZ6egBmp2tEQTlCls/mY8PvZaWGIAa3J/dOd1mmWw3Vnp4oDM3jwW9Q uJLXFkwUmEtCntE2nrfmV5Bg+pHOhEtOfd4npO7k= 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 [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 8CD143898537 for ; Wed, 21 Jul 2021 21:30:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8CD143898537 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-389-h3d15Cq6MoC-WTXmO35KXQ-1; Wed, 21 Jul 2021 17:30:01 -0400 X-MC-Unique: h3d15Cq6MoC-WTXmO35KXQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id DD139824F87 for ; Wed, 21 Jul 2021 21:30:00 +0000 (UTC) Received: from t14s.localdomain.com (ovpn-112-158.phx2.redhat.com [10.3.112.158]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7195819C79; Wed, 21 Jul 2021 21:30:00 +0000 (UTC) To: gcc-patches@gcc.gnu.org Subject: [committed] analyzer: fix issues with phi handling Date: Wed, 21 Jul 2021 17:29:59 -0400 Message-Id: <20210721212959.454732-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-14.0 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_H4, 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: David Malcolm via Gcc-patches From: David Malcolm Reply-To: David Malcolm Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" The analyzer's state purging code was overzealously purging state for ssa names that might be used within phi nodes, leading to false positives from -Wanalyzer-use-of-uninitialized-value. This patch updates phi handling in the analyzer to fix these issues. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as e0a7a6752dad7848eb4b29b826a551c0992256ec. gcc/analyzer/ChangeLog: * region-model.cc (region_model::handle_phi): Add "old_state" param and use it. (region_model::update_for_phis): Update so that all of the phi stmts are effectively handled simultaneously, rather than in order. * region-model.h (region_model::handle_phi): Add "old_state" param. * state-purge.cc (self_referential_phi_p): Replace with... (name_used_by_phis_p): ...this new function. (state_purge_per_ssa_name::process_point): Update to use the above, so that all phi stmts at a basic block are effectively considered simultaneously, and only consider the phi arguments for the pertinent in-edge. * supergraph.cc (cfg_superedge::get_phi_arg_idx): New. (cfg_superedge::get_phi_arg): Use the above. * supergraph.h (cfg_superedge::get_phi_arg_idx): New decl. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/explode-2.c: Remove xfail. * gcc.dg/analyzer/explode-2a.c: Remove expected leak warning on while stmt. * gcc.dg/analyzer/phi-2.c: New test. Signed-off-by: David Malcolm --- gcc/analyzer/region-model.cc | 18 +++++++--- gcc/analyzer/region-model.h | 1 + gcc/analyzer/state-purge.cc | 42 ++++++++++++---------- gcc/analyzer/supergraph.cc | 11 +++++- gcc/analyzer/supergraph.h | 1 + gcc/testsuite/gcc.dg/analyzer/explode-2.c | 2 +- gcc/testsuite/gcc.dg/analyzer/explode-2a.c | 2 +- gcc/testsuite/gcc.dg/analyzer/phi-2.c | 27 ++++++++++++++ 8 files changed, 78 insertions(+), 26 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/phi-2.c diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 6d02c60449c..c029759cb9b 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -1553,11 +1553,14 @@ region_model::on_longjmp (const gcall *longjmp_call, const gcall *setjmp_call, /* Update this region_model for a phi stmt of the form LHS = PHI <...RHS...>. - where RHS is for the appropriate edge. */ + where RHS is for the appropriate edge. + Get state from OLD_STATE so that all of the phi stmts for a basic block + are effectively handled simultaneously. */ void region_model::handle_phi (const gphi *phi, tree lhs, tree rhs, + const region_model &old_state, region_model_context *ctxt) { /* For now, don't bother tracking the .MEM SSA names. */ @@ -1566,9 +1569,10 @@ region_model::handle_phi (const gphi *phi, if (VAR_DECL_IS_VIRTUAL_OPERAND (var)) return; - const svalue *rhs_sval = get_rvalue (rhs, ctxt); + const svalue *src_sval = old_state.get_rvalue (rhs, ctxt); + const region *dst_reg = old_state.get_lvalue (lhs, ctxt); - set_value (get_lvalue (lhs, ctxt), rhs_sval, ctxt); + set_value (dst_reg, src_sval, ctxt); if (ctxt) ctxt->on_phi (phi, rhs); @@ -3036,6 +3040,10 @@ region_model::update_for_phis (const supernode *snode, { gcc_assert (last_cfg_superedge); + /* Copy this state and pass it to handle_phi so that all of the phi stmts + are effectively handled simultaneously. */ + const region_model old_state (*this); + for (gphi_iterator gpi = const_cast(snode)->start_phis (); !gsi_end_p (gpi); gsi_next (&gpi)) { @@ -3044,8 +3052,8 @@ region_model::update_for_phis (const supernode *snode, tree src = last_cfg_superedge->get_phi_arg (phi); tree lhs = gimple_phi_result (phi); - /* Update next_state based on phi. */ - handle_phi (phi, lhs, src, ctxt); + /* Update next_state based on phi and old_state. */ + handle_phi (phi, lhs, src, old_state, ctxt); } } diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index 734ec601237..cc39929db26 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -582,6 +582,7 @@ class region_model region_model_context *ctxt); void handle_phi (const gphi *phi, tree lhs, tree rhs, + const region_model &old_state, region_model_context *ctxt); bool maybe_update_for_edge (const superedge &edge, diff --git a/gcc/analyzer/state-purge.cc b/gcc/analyzer/state-purge.cc index 3c3b77500a6..bfa48a9ef3f 100644 --- a/gcc/analyzer/state-purge.cc +++ b/gcc/analyzer/state-purge.cc @@ -288,17 +288,23 @@ state_purge_per_ssa_name::add_to_worklist (const function_point &point, } } -/* Does this phi depend on itself? - e.g. in: - added_2 = PHI - the middle defn (from edge 3) requires added_2 itself. */ +/* Return true iff NAME is used by any of the phi nodes in SNODE + when processing the in-edge with PHI_ARG_IDX. */ static bool -self_referential_phi_p (const gphi *phi) +name_used_by_phis_p (tree name, const supernode *snode, + size_t phi_arg_idx) { - for (unsigned i = 0; i < gimple_phi_num_args (phi); i++) - if (gimple_phi_arg_def (phi, i) == gimple_phi_result (phi)) - return true; + gcc_assert (TREE_CODE (name) == SSA_NAME); + + for (gphi_iterator gpi + = const_cast (snode)->start_phis (); + !gsi_end_p (gpi); gsi_next (&gpi)) + { + gphi *phi = gpi.phi (); + if (gimple_phi_arg_def (phi, phi_arg_idx) == name) + return true; + } return false; } @@ -339,27 +345,27 @@ state_purge_per_ssa_name::process_point (const function_point &point, = const_cast (snode)->start_phis (); !gsi_end_p (gpi); gsi_next (&gpi)) { + gcc_assert (point.get_from_edge ()); + const cfg_superedge *cfg_sedge + = point.get_from_edge ()->dyn_cast_cfg_superedge (); + gcc_assert (cfg_sedge); + gphi *phi = gpi.phi (); /* Are we at the def-stmt for m_name? */ if (phi == def_stmt) { - /* Does this phi depend on itself? - e.g. in: - added_2 = PHI - the middle defn (from edge 3) requires added_2 itself - so we can't purge it here. */ - if (self_referential_phi_p (phi)) + if (name_used_by_phis_p (m_name, snode, + cfg_sedge->get_phi_arg_idx ())) { if (logger) - logger->log ("self-referential def stmt within phis;" + logger->log ("name in def stmt used within phis;" " continuing"); } else { - /* Otherwise, we can stop here, so that m_name - can be purged. */ if (logger) - logger->log ("def stmt within phis; terminating"); + logger->log ("name in def stmt not used within phis;" + " terminating"); return; } } diff --git a/gcc/analyzer/supergraph.cc b/gcc/analyzer/supergraph.cc index 8611d0f8689..1eb25436f94 100644 --- a/gcc/analyzer/supergraph.cc +++ b/gcc/analyzer/supergraph.cc @@ -1032,12 +1032,21 @@ cfg_superedge::dump_label_to_pp (pretty_printer *pp, /* Otherwise, no label. */ } +/* Get the index number for this edge for use in phi stmts + in its destination. */ + +size_t +cfg_superedge::get_phi_arg_idx () const +{ + return m_cfg_edge->dest_idx; +} + /* Get the phi argument for PHI for this CFG edge. */ tree cfg_superedge::get_phi_arg (const gphi *phi) const { - size_t index = m_cfg_edge->dest_idx; + size_t index = get_phi_arg_idx (); return gimple_phi_arg_def (phi, index); } diff --git a/gcc/analyzer/supergraph.h b/gcc/analyzer/supergraph.h index f4090fd5e0e..877958f75fa 100644 --- a/gcc/analyzer/supergraph.h +++ b/gcc/analyzer/supergraph.h @@ -514,6 +514,7 @@ class cfg_superedge : public superedge int false_value_p () const { return get_flags () & EDGE_FALSE_VALUE; } int back_edge_p () const { return get_flags () & EDGE_DFS_BACK; } + size_t get_phi_arg_idx () const; tree get_phi_arg (const gphi *phi) const; private: diff --git a/gcc/testsuite/gcc.dg/analyzer/explode-2.c b/gcc/testsuite/gcc.dg/analyzer/explode-2.c index 3b987e10a4a..c16982f3bc4 100644 --- a/gcc/testsuite/gcc.dg/analyzer/explode-2.c +++ b/gcc/testsuite/gcc.dg/analyzer/explode-2.c @@ -24,7 +24,7 @@ void test (void) p0 = malloc (16); /* { dg-warning "leak" "" { xfail *-*-* } } */ break; case 1: - free (p0); /* { dg-warning "double-'free' of 'p0'" "" { xfail *-*-* } } */ + free (p0); /* { dg-warning "double-'free' of 'p0'" } */ break; case 2: diff --git a/gcc/testsuite/gcc.dg/analyzer/explode-2a.c b/gcc/testsuite/gcc.dg/analyzer/explode-2a.c index f60354cae1b..32c71ca44aa 100644 --- a/gcc/testsuite/gcc.dg/analyzer/explode-2a.c +++ b/gcc/testsuite/gcc.dg/analyzer/explode-2a.c @@ -14,7 +14,7 @@ void test (void) explode-2.c as this code. */ int a = get (); int b = get (); - while (a) /* { dg-warning "leak" } */ + while (a) { switch (b) { diff --git a/gcc/testsuite/gcc.dg/analyzer/phi-2.c b/gcc/testsuite/gcc.dg/analyzer/phi-2.c new file mode 100644 index 00000000000..2ab8344cfe2 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/phi-2.c @@ -0,0 +1,27 @@ +/* { dg-additional-options "-O1" } */ + +struct list_head { + struct list_head *next, *prev; +}; + +struct mbochs_dmabuf { + /* [...snip...] */ + struct dma_buf *buf; + /* [...snip...] */ + struct list_head next; + /* [...snip...] */ +}; + +void mbochs_close(struct list_head *dmabufs, + struct mbochs_dmabuf *dmabuf, + struct mbochs_dmabuf *tmp) +{ + /* [...snip...] */ + while (&dmabuf->next != dmabufs) + { + dmabuf = tmp; + tmp = ((struct mbochs_dmabuf *)((void *)(tmp->next.next) - __builtin_offsetof(struct mbochs_dmabuf, next))); + } + + /* [...snip...] */ +}