From patchwork Wed Mar 15 10:48:32 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 66404 Return-Path: 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 40D373858C66 for ; Wed, 15 Mar 2023 10:49:04 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 40D373858C66 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1678877344; bh=63GYkNksx6Q+ffc/vYDM8jCv985AOZsoew8Y8yMeIws=; h=Date:To:cc:Subject:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=Hq0JcctwMTvhgntXag6AArmkXy2JHC4lD/CHbtuP4umfIeKhT0neVaDbZbqpeGZnc 9kl9SK7UcLhcwWJP19PhHs2M+znFa/e8t1bIE4qacOmG/m97l4Z1MWRpE1PJbvg9gr sCcRb3kBMayr73hshYpB1Vt60WbIZymhjS0OwjwM= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 14BD23858D33 for ; Wed, 15 Mar 2023 10:48:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 14BD23858D33 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 068C32199A; Wed, 15 Mar 2023 10:48:33 +0000 (UTC) Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id ECD3C2C141; Wed, 15 Mar 2023 10:48:32 +0000 (UTC) Date: Wed, 15 Mar 2023 10:48:32 +0000 (UTC) To: gcc-patches@gcc.gnu.org cc: Jakub Jelinek Subject: [PATCH 1/2] Avoid random stmt order result in pass_waccess::use_after_inval_p User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, MISSING_MID, 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: Richard Biener via Gcc-patches From: Richard Biener Reply-To: Richard Biener Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Message-Id: <20230315104904.40D373858C66@sourceware.org> use_after_inval_p uses stmt UIDs to speed up repeated dominance checks within a basic-block but it fails to assign UIDs to PHIs which means compares with PHIs in the same block get a random result. The following factors renumber_gimple_stmt_uids to expose a new renumber_gimple_stmt_uids_in_block we can share. This XFAILs the conditional loop case in gcc.dg/Wuse-after-free-2.c Bootstrap and regtest running on x86_64-unknown-linux-gnu. This makes 2/2 a net positive on the testsuite (the early waccess would not run into this bug) OK if testing succeeds? (we could also special-case PHIs somehow and assert we never get to compare two PHIs here) * tree-dfa.h (renumber_gimple_stmt_uids_in_block): New. * tree-dfa.cc (renumber_gimple_stmt_uids_in_block): Split out from ... (renumber_gimple_stmt_uids): ... here and (renumber_gimple_stmt_uids_in_blocks): ... here. * gimple-ssa-warn-access.cc (pass_waccess::use_after_inval_p): Use renumber_gimple_stmt_uids_in_block to also assign UIDs to PHIs. * gcc.dg/Wuse-after-free-2.c: XFAIL conditional loop case. --- gcc/gimple-ssa-warn-access.cc | 8 +--- gcc/testsuite/gcc.dg/Wuse-after-free-2.c | 4 +- gcc/tree-dfa.cc | 48 +++++++++++------------- gcc/tree-dfa.h | 1 + 4 files changed, 25 insertions(+), 36 deletions(-) diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc index 8b1c1cc019e..a8ad7a6df65 100644 --- a/gcc/gimple-ssa-warn-access.cc +++ b/gcc/gimple-ssa-warn-access.cc @@ -3862,13 +3862,7 @@ pass_waccess::use_after_inval_p (gimple *inval_stmt, gimple *use_stmt, to consecutive statements in it. Use the ids to determine which precedes which. This avoids the linear traversal on subsequent visits to the same block. */ - for (auto si = gsi_start_bb (inval_bb); !gsi_end_p (si); - gsi_next_nondebug (&si)) - { - gimple *stmt = gsi_stmt (si); - unsigned uid = inc_gimple_stmt_max_uid (m_func); - gimple_set_uid (stmt, uid); - } + renumber_gimple_stmt_uids_in_block (m_func, inval_bb); return gimple_uid (inval_stmt) < gimple_uid (use_stmt); } diff --git a/gcc/testsuite/gcc.dg/Wuse-after-free-2.c b/gcc/testsuite/gcc.dg/Wuse-after-free-2.c index ebc051690db..cd5c43c0fa3 100644 --- a/gcc/testsuite/gcc.dg/Wuse-after-free-2.c +++ b/gcc/testsuite/gcc.dg/Wuse-after-free-2.c @@ -113,6 +113,6 @@ int warn_cond_loop (char *p) while (*q) ++q; - free (p); // { dg-message "call to 'free'" } - return *q; // { dg-warning "pointer 'q' used after 'free'" } + free (p); // dg-message "call to 'free'" + return *q; // { dg-warning "pointer 'q' used after 'free'" "" { xfail *-*-* } } } diff --git a/gcc/tree-dfa.cc b/gcc/tree-dfa.cc index ec8df0d6401..82803a8ccb1 100644 --- a/gcc/tree-dfa.cc +++ b/gcc/tree-dfa.cc @@ -59,6 +59,25 @@ static void collect_dfa_stats (struct dfa_stats_d *); Dataflow analysis (DFA) routines ---------------------------------------------------------------------------*/ +/* Renumber the gimple stmt uids in one block. The caller is responsible + of calling set_gimple_stmt_max_uid (fun, 0) at some point. */ + +void +renumber_gimple_stmt_uids_in_block (struct function *fun, basic_block bb) +{ + gimple_stmt_iterator bsi; + for (bsi = gsi_start_phis (bb); !gsi_end_p (bsi); gsi_next (&bsi)) + { + gimple *stmt = gsi_stmt (bsi); + gimple_set_uid (stmt, inc_gimple_stmt_max_uid (fun)); + } + for (bsi = gsi_start_bb (bb); !gsi_end_p (bsi); gsi_next (&bsi)) + { + gimple *stmt = gsi_stmt (bsi); + gimple_set_uid (stmt, inc_gimple_stmt_max_uid (fun)); + } +} + /* Renumber all of the gimple stmt uids. */ void @@ -68,19 +87,7 @@ renumber_gimple_stmt_uids (struct function *fun) set_gimple_stmt_max_uid (fun, 0); FOR_ALL_BB_FN (bb, fun) - { - gimple_stmt_iterator bsi; - for (bsi = gsi_start_phis (bb); !gsi_end_p (bsi); gsi_next (&bsi)) - { - gimple *stmt = gsi_stmt (bsi); - gimple_set_uid (stmt, inc_gimple_stmt_max_uid (fun)); - } - for (bsi = gsi_start_bb (bb); !gsi_end_p (bsi); gsi_next (&bsi)) - { - gimple *stmt = gsi_stmt (bsi); - gimple_set_uid (stmt, inc_gimple_stmt_max_uid (fun)); - } - } + renumber_gimple_stmt_uids_in_block (fun, bb); } /* Like renumber_gimple_stmt_uids, but only do work on the basic blocks @@ -93,20 +100,7 @@ renumber_gimple_stmt_uids_in_blocks (basic_block *blocks, int n_blocks) set_gimple_stmt_max_uid (cfun, 0); for (i = 0; i < n_blocks; i++) - { - basic_block bb = blocks[i]; - gimple_stmt_iterator bsi; - for (bsi = gsi_start_phis (bb); !gsi_end_p (bsi); gsi_next (&bsi)) - { - gimple *stmt = gsi_stmt (bsi); - gimple_set_uid (stmt, inc_gimple_stmt_max_uid (cfun)); - } - for (bsi = gsi_start_bb (bb); !gsi_end_p (bsi); gsi_next (&bsi)) - { - gimple *stmt = gsi_stmt (bsi); - gimple_set_uid (stmt, inc_gimple_stmt_max_uid (cfun)); - } - } + renumber_gimple_stmt_uids_in_block (cfun, blocks[i]); } diff --git a/gcc/tree-dfa.h b/gcc/tree-dfa.h index 1632487c3ed..074a4da3a6c 100644 --- a/gcc/tree-dfa.h +++ b/gcc/tree-dfa.h @@ -20,6 +20,7 @@ along with GCC; see the file COPYING3. If not see #ifndef GCC_TREE_DFA_H #define GCC_TREE_DFA_H +extern void renumber_gimple_stmt_uids_in_block (struct function *, basic_block); extern void renumber_gimple_stmt_uids (struct function *); extern void renumber_gimple_stmt_uids_in_blocks (basic_block *, int); extern void dump_variable (FILE *, tree); From patchwork Wed Mar 15 10:49:19 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 66405 Return-Path: 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 D2080385842C for ; Wed, 15 Mar 2023 10:49:48 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D2080385842C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1678877388; bh=4p15iaPvESIwqtBDNuDKWu15SWKSVbcTvlKpakf2Zx8=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=s7kweQJjKe1gcIUJZAFlPWp/1QuR14uB2HKTGX/ZkF0iSAjEtIfZTMIxZ8AZOlEF/ UVPvadK9QYt9OMbx3pgg8c4vhmOtOVTfIXPzdavJ7hX6UUsuuBPyQ2w3heoc6I27uV ywVU+pPq0bovmhWa8mJv9+sviv58PmglEStz+ln4= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id CD4613858296 for ; Wed, 15 Mar 2023 10:49:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CD4613858296 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id DE97A2190B for ; Wed, 15 Mar 2023 10:49:19 +0000 (UTC) Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id D47CB2C141 for ; Wed, 15 Mar 2023 10:49:19 +0000 (UTC) Date: Wed, 15 Mar 2023 10:49:19 +0000 (UTC) To: gcc-patches@gcc.gnu.org Subject: [PATCH 2/2] tree-optimization/109123 - run -Wuse-afer-free only early User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, MISSING_MID, 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: Richard Biener via Gcc-patches From: Richard Biener Reply-To: Richard Biener Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Message-Id: <20230315104948.D2080385842C@sourceware.org> The following switches the -Wuse-after-free diagnostics from emitted during the late access warning passes to the early access warning passes to make sure we run before passes performing code motion run which are the source of a lot of false positives on use-after-free not involving memory operations. The patch also fixes issues in c-c++-common/Wuse-after-free-6.c and g++.dg/warn/Wuse-after-free3.C. Bootstrapped and tested on x86_64-unknown-linux-gnu (without 1/2 sofar, but its testcase XFAILed). OK? Thanks, Richard. PR tree-optimization/109123 * gimple-ssa-warn-access.cc (pass_waccess::warn_invalid_pointer): Do not emit -Wuse-after-free late. (pass_waccess::check_call): Always check call pointer uses. * gcc.dg/Wuse-after-free-pr109123.c: New testcase. * c-c++-common/Wuse-after-free-6.c: Un-XFAIL case. * g++.dg/warn/Wuse-after-free3.C: Remove expected duplicate diagnostic. --- gcc/gimple-ssa-warn-access.cc | 28 ++++++------- .../c-c++-common/Wuse-after-free-6.c | 2 +- gcc/testsuite/g++.dg/warn/Wuse-after-free3.C | 3 +- .../gcc.dg/Wuse-after-free-pr109123.c | 41 +++++++++++++++++++ 4 files changed, 57 insertions(+), 17 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/Wuse-after-free-pr109123.c diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc index a8ad7a6df65..d0809d321ea 100644 --- a/gcc/gimple-ssa-warn-access.cc +++ b/gcc/gimple-ssa-warn-access.cc @@ -3907,7 +3907,8 @@ pass_waccess::warn_invalid_pointer (tree ref, gimple *use_stmt, if (is_gimple_call (inval_stmt)) { - if ((equality && warn_use_after_free < 3) + if (!m_early_checks_p + || (equality && warn_use_after_free < 3) || (maybe && warn_use_after_free < 2) || warning_suppressed_p (use_stmt, OPT_Wuse_after_free)) return; @@ -4300,19 +4301,18 @@ pass_waccess::check_call (gcall *stmt) if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)) check_builtin (stmt); - if (!m_early_checks_p) - if (tree callee = gimple_call_fndecl (stmt)) - { - /* Check for uses of the pointer passed to either a standard - or a user-defined deallocation function. */ - unsigned argno = fndecl_dealloc_argno (callee); - if (argno < (unsigned) call_nargs (stmt)) - { - tree arg = call_arg (stmt, argno); - if (TREE_CODE (arg) == SSA_NAME) - check_pointer_uses (stmt, arg); - } - } + if (tree callee = gimple_call_fndecl (stmt)) + { + /* Check for uses of the pointer passed to either a standard + or a user-defined deallocation function. */ + unsigned argno = fndecl_dealloc_argno (callee); + if (argno < (unsigned) call_nargs (stmt)) + { + tree arg = call_arg (stmt, argno); + if (TREE_CODE (arg) == SSA_NAME) + check_pointer_uses (stmt, arg); + } + } check_call_access (stmt); check_call_dangling (stmt); diff --git a/gcc/testsuite/c-c++-common/Wuse-after-free-6.c b/gcc/testsuite/c-c++-common/Wuse-after-free-6.c index 581b1a0a024..0c17a2545f4 100644 --- a/gcc/testsuite/c-c++-common/Wuse-after-free-6.c +++ b/gcc/testsuite/c-c++-common/Wuse-after-free-6.c @@ -53,7 +53,7 @@ void* warn_cond_return_after_free (void *p, int c) free (p); // PHI handling not fully implemented. if (c) - return p; // { dg-warning "pointer 'p' may be used" "pr??????" { xfail *-*-* } } + return p; // { dg-warning "pointer 'p' may be used" } return 0; } diff --git a/gcc/testsuite/g++.dg/warn/Wuse-after-free3.C b/gcc/testsuite/g++.dg/warn/Wuse-after-free3.C index 1862ac8b09d..e5b157865bf 100644 --- a/gcc/testsuite/g++.dg/warn/Wuse-after-free3.C +++ b/gcc/testsuite/g++.dg/warn/Wuse-after-free3.C @@ -1,7 +1,6 @@ // PR target/104213 // { dg-do compile } // { dg-options "-Wuse-after-free" } -// FIXME: We should not output the warning twice. struct A { @@ -13,4 +12,4 @@ A::~A () { operator delete (this); f (); // { dg-warning "used after" } -} // { dg-warning "used after" } +} diff --git a/gcc/testsuite/gcc.dg/Wuse-after-free-pr109123.c b/gcc/testsuite/gcc.dg/Wuse-after-free-pr109123.c new file mode 100644 index 00000000000..ece066dd28b --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wuse-after-free-pr109123.c @@ -0,0 +1,41 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -Wall" } */ + +typedef long unsigned int size_t; +extern void *realloc (void *__ptr, size_t __size) + __attribute__ ((__nothrow__ , __leaf__)) __attribute__ ((__warn_unused_result__)) __attribute__ ((__alloc_size__ (2))); +struct vector_objective; +typedef struct vector_objective vector_objective; +struct vector_objective { double *_begin; double *_end; double *_capacity; }; +static inline size_t vector_objective_size(const vector_objective * v) { + return v->_end - v->_begin; /* { dg-bogus "used after" } */ +} +static inline size_t vector_objective_capacity(const vector_objective * v) { + return v->_capacity - v->_begin; +} +static inline void vector_objective_reserve(vector_objective * v, size_t n) { + size_t old_capacity = vector_objective_capacity(v); + size_t old_size = vector_objective_size(v); + if (n > old_capacity) { + v->_begin = realloc(v->_begin, sizeof(double) * n); + v->_end = v->_begin + old_size; + v->_capacity = v->_begin + n; + } +} +static inline void vector_objective_push_back(vector_objective * v, double x) { + if (v->_end == v->_capacity) + vector_objective_reserve (v, (vector_objective_capacity (v) == 0) ? 8 : 2 * vector_objective_capacity (v)); + *(v->_end) = x; + v->_end++; +} + +typedef struct { + vector_objective xy; +} eaf_polygon_t; + +int +rectangle_add(eaf_polygon_t * regions, double lx) +{ + vector_objective_push_back(®ions->xy, lx); + return 0; +}