From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22231 invoked by alias); 10 Jan 2013 16:02:37 -0000 Received: (qmail 22216 invoked by uid 22791); 10 Jan 2013 16:02:31 -0000 X-SWARE-Spam-Status: No, hits=-6.1 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,TW_TM X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 10 Jan 2013 16:02:25 +0000 Received: from relay1.suse.de (unknown [195.135.220.254]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id EF61EA4386; Thu, 10 Jan 2013 17:02:23 +0100 (CET) Date: Thu, 10 Jan 2013 16:02:00 -0000 From: Richard Biener To: Jakub Jelinek Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH][RFC] location and BLOCK checker In-Reply-To: <20130110154329.GC7269@tucnak.redhat.com> Message-ID: References: <20130110154329.GC7269@tucnak.redhat.com> User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2013-01/txt/msg00550.txt.bz2 On Thu, 10 Jan 2013, Jakub Jelinek wrote: > On Thu, Jan 10, 2013 at 04:36:26PM +0100, Richard Biener wrote: > > > > The following is what I use to track down PR55792 - it adds verification > > that the GIMPLE IL only contains references to BLOCKs that are part > > of the functions BLOCK tree. Otherwise BLOCKs lack a reference that is > > seen by the garbage collector and funny things can happen. > > > > It probably should also scan DEBUG_STMTs. > > > > Do we want this in trunk or should it wait for stage1? > > I'd say we want it in the trunk now (well, as soon as it doesn't report > regressions during bootstrap/regtest). And yes, it should check also > DEBUG_STMTs, eventhough for now we completely ignore their locations. Ok, it seems it already does now. Bootstrapped and tested on x86_64-unknown-linux-gnu with some Fortran testsuite ICEs, LTO profiledbootstrap fails with verification errors. Checking regular LTO bootstrap now. Richard. 2013-01-10 Richard Biener * tree-cfg.c (verify_node_sharing_1): Split out from ... (verify_node_sharing): ... here. (verify_expr_location, verify_expr_location_1, verify_location, collect_subblocks): New functions. (verify_gimple_in_cfg): Verify that locations only reference BLOCKs in the functions BLOCK tree. Index: gcc/tree-cfg.c =================================================================== *** gcc/tree-cfg.c.orig 2013-01-10 16:27:39.000000000 +0100 --- gcc/tree-cfg.c 2013-01-10 16:55:43.644221848 +0100 *************** tree_node_can_be_shared (tree t) *** 4483,4495 **** return false; } ! /* Called via walk_gimple_stmt. Verify tree sharing. */ static tree ! verify_node_sharing (tree *tp, int *walk_subtrees, void *data) { ! struct walk_stmt_info *wi = (struct walk_stmt_info *) data; ! struct pointer_set_t *visited = (struct pointer_set_t *) wi->info; if (tree_node_can_be_shared (*tp)) { --- 4483,4494 ---- return false; } ! /* Called via walk_tree. Verify tree sharing. */ static tree ! verify_node_sharing_1 (tree *tp, int *walk_subtrees, void *data) { ! struct pointer_set_t *visited = (struct pointer_set_t *) data; if (tree_node_can_be_shared (*tp)) { *************** verify_node_sharing (tree *tp, int *walk *** 4503,4508 **** --- 4502,4516 ---- return NULL; } + /* Called via walk_gimple_stmt. Verify tree sharing. */ + + static tree + verify_node_sharing (tree *tp, int *walk_subtrees, void *data) + { + struct walk_stmt_info *wi = (struct walk_stmt_info *) data; + return verify_node_sharing_1 (tp, walk_subtrees, wi->info); + } + static bool eh_error_found; static int verify_eh_throw_stmt_node (void **slot, void *data) *************** verify_eh_throw_stmt_node (void **slot, *** 4519,4524 **** --- 4527,4589 ---- return 1; } + /* Verify if the location LOCs block is in BLOCKS. */ + + static bool + verify_location (pointer_set_t *blocks, location_t loc) + { + tree block = LOCATION_BLOCK (loc); + if (block != NULL_TREE + && !pointer_set_contains (blocks, block)) + { + error ("location references block not in block tree"); + return true; + } + return false; + } + + /* Called via walk_tree. Verify locations of expressions. */ + + static tree + verify_expr_location_1 (tree *tp, int *walk_subtrees, void *data) + { + struct pointer_set_t *blocks = (struct pointer_set_t *) data; + + if (!EXPR_P (*tp)) + { + *walk_subtrees = false; + return NULL; + } + + location_t loc = EXPR_LOCATION (*tp); + if (verify_location (blocks, loc)) + return *tp; + + return NULL; + } + + /* Called via walk_gimple_op. Verify locations of expressions. */ + + static tree + verify_expr_location (tree *tp, int *walk_subtrees, void *data) + { + struct walk_stmt_info *wi = (struct walk_stmt_info *) data; + return verify_expr_location_1 (tp, walk_subtrees, wi->info); + } + + /* Insert all subblocks of BLOCK into BLOCKS and recurse. */ + + static void + collect_subblocks (pointer_set_t *blocks, tree block) + { + tree t; + for (t = BLOCK_SUBBLOCKS (block); t; t = BLOCK_CHAIN (t)) + { + pointer_set_insert (blocks, t); + collect_subblocks (blocks, t); + } + } + /* Verify the GIMPLE statements in the CFG of FN. */ DEBUG_FUNCTION void *************** verify_gimple_in_cfg (struct function *f *** 4526,4537 **** { basic_block bb; bool err = false; ! struct pointer_set_t *visited, *visited_stmts; timevar_push (TV_TREE_STMT_VERIFY); visited = pointer_set_create (); visited_stmts = pointer_set_create (); FOR_EACH_BB_FN (bb, fn) { gimple_stmt_iterator gsi; --- 4591,4610 ---- { basic_block bb; bool err = false; ! struct pointer_set_t *visited, *visited_stmts, *blocks; timevar_push (TV_TREE_STMT_VERIFY); visited = pointer_set_create (); visited_stmts = pointer_set_create (); + /* Collect all BLOCKs referenced by the BLOCK tree of FN. */ + blocks = pointer_set_create (); + if (DECL_INITIAL (fn->decl)) + { + pointer_set_insert (blocks, DECL_INITIAL (fn->decl)); + collect_subblocks (blocks, DECL_INITIAL (fn->decl)); + } + FOR_EACH_BB_FN (bb, fn) { gimple_stmt_iterator gsi; *************** verify_gimple_in_cfg (struct function *f *** 4552,4567 **** err2 |= verify_gimple_phi (phi); for (i = 0; i < gimple_phi_num_args (phi); i++) { tree arg = gimple_phi_arg_def (phi, i); ! tree addr = walk_tree (&arg, verify_node_sharing, visited, NULL); if (addr) { error ("incorrect sharing of tree nodes"); debug_generic_expr (addr); err2 |= true; } } if (err2) --- 4625,4661 ---- err2 |= verify_gimple_phi (phi); + /* Only PHI arguments have locations. */ + if (gimple_location (phi) != UNKNOWN_LOCATION) + { + error ("PHI node with location"); + err2 = true; + } + for (i = 0; i < gimple_phi_num_args (phi); i++) { tree arg = gimple_phi_arg_def (phi, i); ! tree addr = walk_tree (&arg, verify_node_sharing_1, visited, NULL); if (addr) { error ("incorrect sharing of tree nodes"); debug_generic_expr (addr); err2 |= true; } + location_t loc = gimple_phi_arg_location (phi, i); + if (virtual_operand_p (gimple_phi_result (phi)) + && loc != UNKNOWN_LOCATION) + { + error ("virtual PHI with argument locations"); + err2 = true; + } + addr = walk_tree (&arg, verify_expr_location_1, blocks, NULL); + if (addr) + { + debug_generic_expr (addr); + err2 = true; + } + err2 |= verify_location (blocks, loc); } if (err2) *************** verify_gimple_in_cfg (struct function *f *** 4586,4591 **** --- 4680,4686 ---- } err2 |= verify_gimple_stmt (stmt); + err2 |= verify_location (blocks, gimple_location (stmt)); memset (&wi, 0, sizeof (wi)); wi.info = (void *) visited; *************** verify_gimple_in_cfg (struct function *f *** 4597,4602 **** --- 4692,4706 ---- err2 |= true; } + memset (&wi, 0, sizeof (wi)); + wi.info = (void *) blocks; + addr = walk_gimple_op (stmt, verify_expr_location, &wi); + if (addr) + { + debug_generic_expr (addr); + err2 |= true; + } + /* ??? Instead of not checking these stmts at all the walker should know its context via wi. */ if (!is_gimple_debug (stmt) *************** verify_gimple_in_cfg (struct function *f *** 4651,4656 **** --- 4755,4761 ---- pointer_set_destroy (visited); pointer_set_destroy (visited_stmts); + pointer_set_destroy (blocks); verify_histograms (); timevar_pop (TV_TREE_STMT_VERIFY); }