From patchwork Thu Aug 25 15:51:22 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 111601 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]) by ozlabs.org (Postfix) with SMTP id 07EEBB6F81 for ; Fri, 26 Aug 2011 01:50:27 +1000 (EST) Received: (qmail 23311 invoked by alias); 25 Aug 2011 15:50:22 -0000 Received: (qmail 23300 invoked by uid 22791); 25 Aug 2011 15:50:19 -0000 X-SWARE-Spam-Status: No, hits=-1.5 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_FAIL, TW_EG, TW_FN, TW_TM X-Spam-Check-By: sourceware.org Received: from smtp-vbr12.xs4all.nl (HELO smtp-vbr12.xs4all.nl) (194.109.24.32) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 25 Aug 2011 15:50:05 +0000 Received: from [192.168.1.68] (teejay.xs4all.nl [213.84.119.160]) (authenticated bits=0) by smtp-vbr12.xs4all.nl (8.13.8/8.13.8) with ESMTP id p7PFo2bN083335 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 25 Aug 2011 17:50:03 +0200 (CEST) (envelope-from vries@codesourcery.com) Message-ID: <4E566F7A.4080004@codesourcery.com> Date: Thu, 25 Aug 2011 17:51:22 +0200 From: Tom de Vries User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.18) Gecko/20110617 Lightning/1.0b2 Thunderbird/3.1.11 MIME-Version: 1.0 To: Richard Guenther CC: Jakub Jelinek , "gcc-patches@gcc.gnu.org" Subject: Re: fix for segmentation violation in dump_generic_node References: <4E5624B6.3080005@codesourcery.com> In-Reply-To: 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 Hi Richard, thanks for the review. On 08/25/2011 12:45 PM, Richard Guenther wrote: > On Thu, Aug 25, 2011 at 12:32 PM, Tom de Vries wrote: >> Jakub, >> >> This patch fixes a segmentation violation, which occurs when printing a MEM_REF >> or COMPONENT_REF containing a released ssa name. This can happen when we print >> basic blocks upon removal, enabled by -ftree-dump-tree-*-details (see >> remove_bb:tree-cfg.c). > > Where do we dump stmts there? > In dump_bb: static void remove_bb (basic_block bb) { gimple_stmt_iterator i; if (dump_file) { fprintf (dump_file, "Removing basic block %d\n", bb->index); if (dump_flags & TDF_DETAILS) { dump_bb (bb, dump_file, 0); fprintf (dump_file, "\n"); } } >> Bootstrapped and reg-tested on x86_64. >> >> OK for trunk? > > At least > > TREE_TYPE (TREE_OPERAND (node, 1)) != NULL_TREE > > is always true. > Right. > The comment before the new lines is now in the wrong place and this > check at least needs a comment as well. > Ok, fixed that. > But - it's broken to dump freed stuff, why and where do we do this? > Sorry, I did not realize that. The scenario is as follows: fnsplit splits a function, and as todo cleanup_tree_cfg is called and unreachable blocks are removed, among which blocks 12 and 13. Block 12 contains a use of 45: # BLOCK 12 freq:9100 # PRED: 13 D.13888_46 = *sD.13886_45; Block 13 contains a def of 45: Block 13 # BLOCK 13 # PRED: 11 12 ... # sD.13886_45 = PHI ... if (sizeD.8479_2 > iD.13887_50) goto ; else goto ; # SUCC: 12 14 First block 13 is removed, and remove_phi_nodes_and_edges_for_unreachable_block in remove_bb removes the phi def and releases version 45. Then block 12 is removed, and before removal it is dumped by dump_bb in remove_bb, triggering the segv. The order of removal is determined by the 2nd loop in delete_unreachable_blocks, which is chosen because there is no dominator info present: for (b = EXIT_BLOCK_PTR->prev_bb; b != ENTRY_BLOCK_PTR; b = prev_bb) { prev_bb = b->prev_bb; if (!(b->flags & BB_REACHABLE)) { delete_basic_block (b); changed = true; } } I'm not sure how to fix this. Another occurance of the same segv is in remove_dead_inserted_code: EXECUTE_IF_SET_IN_BITMAP (inserted_exprs, 0, i, bi) { t = SSA_NAME_DEF_STMT (ssa_name (i)); if (!gimple_plf (t, NECESSARY)) { gimple_stmt_iterator gsi; if (dump_file && (dump_flags & TDF_DETAILS)) { fprintf (dump_file, "Removing unnecessary insertion:"); print_gimple_stmt (dump_file, t, 0, 0); } gsi = gsi_for_stmt (t); if (gimple_code (t) == GIMPLE_PHI) remove_phi_node (&gsi, true); else { gsi_remove (&gsi, true); release_defs (t); } } } Here a version is released, while it's used in the defining statement of version+1, which is subsequently printed. This is easy to fix by splitting the loop, I'll make a patch for this. There might be other occurrences (I triggered these 2 doing a gcc build), but I cannot trigger others until delete_unreachable_blocks does not trigger anymore. > Richard. > Updated untested patch attached, I'll test this patch together with the remove_dead_inserted_code patch. Thanks, - Tom >> 2011-08-25 Tom de Vries >> >> * tree-pretty-print (dump_generic_node): Test for NULL_TREE before >> accessing TREE_TYPE. >> Index: gcc/tree-pretty-print.c =================================================================== --- gcc/tree-pretty-print.c (revision 176554) +++ gcc/tree-pretty-print.c (working copy) @@ -809,6 +809,8 @@ dump_generic_node (pretty_printer *buffe infer them and MEM_ATTR caching will share MEM_REFs with differently-typed op0s. */ && TREE_CODE (TREE_OPERAND (node, 0)) != INTEGER_CST + /* Released SSA_NAMES have no TREE_TYPE. */ + && TREE_TYPE (TREE_OPERAND (node, 0)) != NULL_TREE /* Same pointer types, but ignoring POINTER_TYPE vs. REFERENCE_TYPE. */ && (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 0))) @@ -1175,6 +1177,8 @@ dump_generic_node (pretty_printer *buffe can't infer them and MEM_ATTR caching will share MEM_REFs with differently-typed op0s. */ && TREE_CODE (TREE_OPERAND (op0, 0)) != INTEGER_CST + /* Released SSA_NAMES have no TREE_TYPE. */ + && TREE_TYPE (TREE_OPERAND (op0, 0)) != NULL_TREE /* Same pointer types, but ignoring POINTER_TYPE vs. REFERENCE_TYPE. */ && (TREE_TYPE (TREE_TYPE (TREE_OPERAND (op0, 0)))