[PATCH, Pointer Bounds Checker 13/x] Early versioning

[Ilya Enkovich]

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.enkovich@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
>