On 30 May 10:59, Jeff Law wrote:
> On 05/29/14 05:05, Ilya Enkovich wrote:
> >Hi,
> >
> >This patch allows to perform function versioning when some structures are
> >not available yet. It is required to make clones for Pointer Bounds Checker
> >right after SSA build.
> >
> >Bootstrapped and tested on linux-x86_64.
> >
> >Thanks,
> >Ilya
> >--
> >gcc/
> >
> >2014-05-29 Ilya Enkovich <ilya.enkov...@intel.com>
> >
> > * tree-inline.c (copy_cfg_body): Check loop tree
> > existence before accessing it.
> > (tree_function_versioning): Check DF info existence
> > before accessing it.
> >
> >diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> >index 4293241..23fef90 100644
> >--- a/gcc/tree-inline.c
> >+++ b/gcc/tree-inline.c
> >@@ -2544,7 +2544,8 @@ copy_cfg_body (copy_body_data * id, gcov_type count,
> >int frequency_scale,
> >
> > /* If the loop tree in the source function needed fixup, mark the
> > destination loop tree for fixup, too. */
> >- if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
> >+ if (loops_for_fn (src_cfun)
> >+ && loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
> > loops_state_set (LOOPS_NEED_FIXUP);
> Hmm, so if I understand things correctly, src_fun has no loop
> structures attached, thus there's nothing to copy. Presumably at
> some later point we build loop structures for the copy from scratch?
I suppose it is just a simple bug with absent NULL pointer check. Here is
original code:
/* Duplicate the loop tree, if available and wanted. */
if (loops_for_fn (src_cfun) != NULL
&& current_loops != NULL)
{
copy_loops (id, entry_block_map->loop_father,
get_loop (src_cfun, 0));
/* Defer to cfgcleanup to update loop-father fields of basic-blocks. */
loops_state_set (LOOPS_NEED_FIXUP);
}
/* If the loop tree in the source function needed fixup, mark the
destination loop tree for fixup, too. */
if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
loops_state_set (LOOPS_NEED_FIXUP);
As you may see we have check for absent loops structure in the first
if-statement and no check in the second one. I hit segfault and added the
check.
>
> Similarly for the PTA info, we just build it from scratch in the
> copy at some point?
Here we also have conditional access like
/* Reset the escaped solution. */
if (cfun->gimple_df)
pt_solution_reset (&cfun->gimple_df->escaped);
and following unconditional I've fixed.
>
> Assuming the answers to both are yes, then this patch is OK for the
> trunk when the rest of the patches are approved. It's not great,
> bit it's OK.
Thanks!
Ilya
>
> jeff
>
