From patchwork Wed Nov 13 23:11:02 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Law X-Patchwork-Id: 291073 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 219B42C007B for ; Thu, 14 Nov 2013 10:11:35 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:content-type; q= dns; s=default; b=YPD7bVn5FT0wWX6n9rGPrwTRY9iBEYrgMdGj5ZCcESE/ZT 7j6xMCeHiThVgH1rNvWmoyOIWx3fIsYmLVuN3N6Ss33LUhbmOm8I0GeTdJPPdo7m IAWBwl9u9+tGTBIZkN7Tsst7sTND5De4pg1uhpvwzN5oKVJOGEppYetg50HaU= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:content-type; s= default; bh=q0ew5a6wXiH/IyI+h1MAANtGxR0=; b=k3PMhPwE6yri7t1kkeaK kX66eHkB33VrdKwORHQDGtl8bmMcMFZ8V+3mFQy9MSi/HnIrqI3nwAfTEoqeVwoh GGjLV1sUAnHSOGt+uWl80n5VVhuAxzatr5uTLaWu1iDWJvpQdC6LKA/nwQiqgyTS 6uWB/kY+wtHZhpTReuXYprI= Received: (qmail 22255 invoked by alias); 13 Nov 2013 23:11:23 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 22242 invoked by uid 89); 13 Nov 2013 23:11:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.8 required=5.0 tests=AWL, BAYES_50, RDNS_NONE, SPF_HELO_PASS, SPF_PASS autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from Unknown (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 13 Nov 2013 23:11:11 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rADNB3ub006162 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 13 Nov 2013 18:11:03 -0500 Received: from stumpy.slc.redhat.com (ovpn-113-161.phx2.redhat.com [10.3.113.161]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id rADNB2JX027907 for ; Wed, 13 Nov 2013 18:11:03 -0500 Message-ID: <52840706.9040403@redhat.com> Date: Wed, 13 Nov 2013 16:11:02 -0700 From: Jeff Law User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.0 MIME-Version: 1.0 To: gcc-patches Subject: [PATCH][PR middle-end/59119] Avoid referencing released SSA_NAMEs X-IsSubscribed: yes We have a known API wart with the SSA_NAME manager. Specifically, once you start releasing names, you can't have dangling references (say in unreachable blocks) and allococate new names. The erroneous path optimization could run afoul of that requirement as it removed statements after an explicit erroneous statement, particularly in the code to adjust debug statements. That resulted in a segfault after we tried to us a NULL TREE_TYPE of a released SSA_NAME. I think I know how we can fix the name manager, but that's future work. For the erroneous path optimizer, two changes avoid these problems. First on a local basis, when we remove statements we do so from the end of the block up to, but not including the builtin_trap. Second, on a global basis, we first find all the implicit erroneous paths that have to be isolated. That process creates SSA_NAMEs, but does not remove edges in the CFG (it redirects edges). So it should be safe. After we find all the implicit erroneous paths in the current function, we then look for any explicit erroneous statements in each block and optimize those (which can create unreachable code and dangling references). Bootstrapped and regression tested on x86_64-unknown-linux-gnu. Installed on the trunk. * PR middle-end/59119 * gimple-ssa-isolate-paths.c (find_implicit_erroneous_behaviour): New function, extracted from gimple_ssa_isolate_erroneous_paths. (find_explicit_erroneous_behaviour): Similarly. (insert_trap_and_remove_trailing_statements): Remove statements in reverse order. * PR middle-end/59119 * gcc.c-torture/compile/pr59119.c: New test. diff --git a/gcc/gimple-ssa-isolate-paths.c b/gcc/gimple-ssa-isolate-paths.c index 2d8f176..0051da2 100644 --- a/gcc/gimple-ssa-isolate-paths.c +++ b/gcc/gimple-ssa-isolate-paths.c @@ -100,14 +100,16 @@ insert_trap_and_remove_trailing_statements (gimple_stmt_iterator *si_p, tree op) else gsi_insert_before (si_p, seq, GSI_NEW_STMT); - /* The iterator points to the __builtin_trap. Advance the iterator - and delete everything else in the block. */ - gsi_next (si_p); - for (; !gsi_end_p (*si_p);) + /* We must remove statements from the end of the block so that we + never reference a released SSA_NAME. */ + basic_block bb = gimple_bb (gsi_stmt (*si_p)); + for (gimple_stmt_iterator si = gsi_last_bb (bb); + gsi_stmt (si) != gsi_stmt (*si_p); + si = gsi_last_bb (bb)) { - stmt = gsi_stmt (*si_p); + stmt = gsi_stmt (si); unlink_stmt_vdef (stmt); - gsi_remove (si_p, true); + gsi_remove (&si, true); release_defs (stmt); } } @@ -192,40 +194,19 @@ isolate_path (basic_block bb, basic_block duplicate, return duplicate; } -/* Search the function for statements which, if executed, would cause - the program to fault such as a dereference of a NULL pointer. - - Such a program can't be valid if such a statement was to execute - according to ISO standards. - - We detect explicit NULL pointer dereferences as well as those implied - by a PHI argument having a NULL value which unconditionally flows into - a dereference in the same block as the PHI. - - In the former case we replace the offending statement with an - unconditional trap and eliminate the outgoing edges from the statement's - basic block. This may expose secondary optimization opportunities. - - In the latter case, we isolate the path(s) with the NULL PHI - feeding the dereference. We can then replace the offending statement - and eliminate the outgoing edges in the duplicate. Again, this may - expose secondary optimization opportunities. +/* Look for PHI nodes which feed statements in the same block where + the value of the PHI node implies the statement is erroneous. - A warning for both cases may be advisable as well. + For example, a NULL PHI arg value which then feeds a pointer + dereference. - Other statically detectable violations of the ISO standard could be - handled in a similar way, such as out-of-bounds array indexing. */ - -static unsigned int -gimple_ssa_isolate_erroneous_paths (void) + When found isolate and optimize the path associated with the PHI + argument feeding the erroneous statement. */ +static void +find_implicit_erroneous_behaviour (void) { basic_block bb; - initialize_original_copy_tables (); - - /* Search all the blocks for edges which, if traversed, will - result in undefined behaviour. */ - cfg_altered = false; FOR_EACH_BB (bb) { gimple_stmt_iterator si; @@ -288,6 +269,21 @@ gimple_ssa_isolate_erroneous_paths (void) } } } + } +} + +/* Look for statements which exhibit erroneous behaviour. For example + a NULL pointer dereference. + + When found, optimize the block containing the erroneous behaviour. */ +static void +find_explicit_erroneous_behaviour (void) +{ + basic_block bb; + + FOR_EACH_BB (bb) + { + gimple_stmt_iterator si; /* Now look at the statements in the block and see if any of them explicitly dereference a NULL pointer. This happens @@ -318,6 +314,56 @@ gimple_ssa_isolate_erroneous_paths (void) } } } +} +/* Search the function for statements which, if executed, would cause + the program to fault such as a dereference of a NULL pointer. + + Such a program can't be valid if such a statement was to execute + according to ISO standards. + + We detect explicit NULL pointer dereferences as well as those implied + by a PHI argument having a NULL value which unconditionally flows into + a dereference in the same block as the PHI. + + In the former case we replace the offending statement with an + unconditional trap and eliminate the outgoing edges from the statement's + basic block. This may expose secondary optimization opportunities. + + In the latter case, we isolate the path(s) with the NULL PHI + feeding the dereference. We can then replace the offending statement + and eliminate the outgoing edges in the duplicate. Again, this may + expose secondary optimization opportunities. + + A warning for both cases may be advisable as well. + + Other statically detectable violations of the ISO standard could be + handled in a similar way, such as out-of-bounds array indexing. */ + +static unsigned int +gimple_ssa_isolate_erroneous_paths (void) +{ + initialize_original_copy_tables (); + + /* Search all the blocks for edges which, if traversed, will + result in undefined behaviour. */ + cfg_altered = false; + + /* First handle cases where traversal of a particular edge + triggers undefined behaviour. These cases require creating + duplicate blocks and thus new SSA_NAMEs. + + We want that process complete prior to the phase where we start + removing edges from the CFG. Edge removal may ultimately result in + removal of PHI nodes and thus releasing SSA_NAMEs back to the + name manager. + + If the two processes run in parallel we could release an SSA_NAME + back to the manager but we could still have dangling references + to the released SSA_NAME in unreachable blocks. + that any released names not have dangling references in the IL. */ + find_implicit_erroneous_behaviour (); + find_explicit_erroneous_behaviour (); + free_original_copy_tables (); /* We scramble the CFG and loop structures a bit, clean up diff --git a/gcc/testsuite/gcc.c-torture/compile/pr59119.c b/gcc/testsuite/gcc.c-torture/compile/pr59119.c new file mode 100644 index 0000000..b026ba5 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr59119.c @@ -0,0 +1,23 @@ +extern void *memmove (void *, const void *, __SIZE_TYPE__); +extern void *memset (void *, int, __SIZE_TYPE__); + +typedef struct { + long n_prefix; + long n_spadding; +} NumberFieldWidths; + +void +fill_number(char *buf, const NumberFieldWidths *spec) +{ + if (spec->n_prefix) { + memmove(buf, + (char *) 0, + spec->n_prefix * sizeof(char)); + buf += spec->n_prefix; + } + if (spec->n_spadding) { + memset(buf, 0, spec->n_spadding); + buf += spec->n_spadding; + } +} +