* [PATCH][RFC] location and BLOCK checker
@ 2013-01-10 15:38 Richard Biener
2013-01-10 15:43 ` Jakub Jelinek
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2013-01-10 15:38 UTC (permalink / raw)
To: gcc-patches
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?
Thanks,
Richard.
Index: gcc/tree-cfg.c
===================================================================
*** gcc/tree-cfg.c.orig 2013-01-10 12:21:01.000000000 +0100
--- gcc/tree-cfg.c 2013-01-10 13:39:46.478923857 +0100
*************** verify_eh_throw_stmt_node (void **slot,
*** 4519,4524 ****
--- 4519,4581 ----
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;
--- 4583,4602 ----
{
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,4557 ****
--- 4617,4629 ----
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);
*************** verify_gimple_in_cfg (struct function *f
*** 4562,4567 ****
--- 4634,4653 ----
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 ****
--- 4672,4678 ----
}
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 ****
--- 4684,4698 ----
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 ****
--- 4747,4753 ----
pointer_set_destroy (visited);
pointer_set_destroy (visited_stmts);
+ pointer_set_destroy (blocks);
verify_histograms ();
timevar_pop (TV_TREE_STMT_VERIFY);
}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH][RFC] location and BLOCK checker
2013-01-10 15:38 [PATCH][RFC] location and BLOCK checker Richard Biener
@ 2013-01-10 15:43 ` Jakub Jelinek
2013-01-10 16:02 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2013-01-10 15:43 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
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.
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH][RFC] location and BLOCK checker
2013-01-10 15:43 ` Jakub Jelinek
@ 2013-01-10 16:02 ` Richard Biener
0 siblings, 0 replies; 3+ messages in thread
From: Richard Biener @ 2013-01-10 16:02 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
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 <rguenther@suse.de>
* 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);
}
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-01-10 16:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-10 15:38 [PATCH][RFC] location and BLOCK checker Richard Biener
2013-01-10 15:43 ` Jakub Jelinek
2013-01-10 16:02 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).