[RFC] PR64703, glibc sysdeps/powerpc/powerpc64/dl-machine.h miscompile

[Richard Biener]

On Sat, Jan 24, 2015 at 12:23 AM, Alan Modra <amodra@gmail.com> wrote:
> How does this look as a potential fix for PR64703?  I haven't made
> many forays into gimple code, so even though this patch passes
> bootstrap and regression testing on powerpc64-linux it's quite
> possible this is the wrong place to change.  If it does look to be OK,
> then I'll fill out the targetm changes, include a testcase etc.

It looks mostly ok, comments below.

>         PR target/64703
>         * tree-ssa-alias.c (pt_solution_includes_base): New function,
>         extracted from..
>         (ref_maybe_used_by_call_p_1): ..here.  Delete dead code checking
>         for NULL return from ao_ref_base.  Handle potential memory
>         reference by indirect calls on targets using function descriptors.
>
> Index: gcc/tree-ssa-alias.c
> ===================================================================
> --- gcc/tree-ssa-alias.c        (revision 220025)
> +++ gcc/tree-ssa-alias.c        (working copy)
> @@ -1532,6 +1532,23 @@ refs_output_dependent_p (tree store1, tree store2)
>    return refs_may_alias_p_1 (&r1, &r2, false);
>  }
>
> +static bool
> +pt_solution_includes_base (struct pt_solution *pt, tree base)

Needs a comment.

> +{
> +  if (DECL_P (base))
> +    return pt_solution_includes (pt, base);
> +
> +  if ((TREE_CODE (base) == MEM_REF
> +       || TREE_CODE (base) == TARGET_MEM_REF)
> +      && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME)
> +    {
> +      struct ptr_info_def *pi = SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0));
> +      if (pi)
> +       return pt_solutions_intersect (pt, &pi->pt);
> +    }
> +  return true;
> +}
> +
>  /* If the call CALL may use the memory reference REF return true,
>     otherwise return false.  */
>
> @@ -1542,15 +1559,24 @@ ref_maybe_used_by_call_p_1 (gcall *call, ao_ref *r
>    unsigned i;
>    int flags = gimple_call_flags (call);
>
> +  base = ao_ref_base (ref);

You dropped the

  if (!base)
    return true;

check - please put it back.

> +  callee = gimple_call_fn (call);
> +  if (callee && TREE_CODE (callee) == SSA_NAME

Do we never propagate the address of a function descriptor here?
That is, can we generate a testcase like

   descr fn;
   <setup fn>
   foo (&fn);

void foo (fn)
{
   (*fn) (a, b);
}

and then inline foo so the call becomes

  (*&fn) (a, b);

?  We'd then still implicitely read from 'fn'.  I also wonder whether
(and where!) we resolve such a descriptor reference to the actual
function on GIMPLE?

That is - don't you simply want to use

  if (targetm.function_descriptors
      && ptr_deref_mayalias_ref_p_1 (callee, ref))
    return true;

here?

Thanks,
Richard.

> +      /* && targetm.function_descriptors */)
> +    {
> +      /* Handle indirect call.  When a target defines the address of a
> +        function as that of a function descriptor, then dereferencing
> +        a function pointer implicitly references memory.  */
> +      struct ptr_info_def *pi = SSA_NAME_PTR_INFO (callee);
> +      if (pi && pt_solution_includes_base (&pi->pt, base))
> +       return true;
> +    }
> +
>    /* Const functions without a static chain do not implicitly use memory.  */
>    if (!gimple_call_chain (call)
>        && (flags & (ECF_CONST|ECF_NOVOPS)))
>      goto process_args;
>
> -  base = ao_ref_base (ref);
> -  if (!base)
> -    return true;
> -
>    /* A call that is not without side-effects might involve volatile
>       accesses and thus conflicts with all other volatile accesses.  */
>    if (ref->volatile_p)
> @@ -1564,7 +1590,7 @@ ref_maybe_used_by_call_p_1 (gcall *call, ao_ref *r
>        && !is_global_var (base))
>      goto process_args;
>
> -  callee = gimple_call_fndecl (call);
> +  callee = gimple_call_addr_fndecl (callee);
>
>    /* Handle those builtin functions explicitly that do not act as
>       escape points.  See tree-ssa-structalias.c:find_func_aliases
> @@ -1803,23 +1829,7 @@ ref_maybe_used_by_call_p_1 (gcall *call, ao_ref *r
>      }
>
>    /* Check if the base variable is call-used.  */
> -  if (DECL_P (base))
> -    {
> -      if (pt_solution_includes (gimple_call_use_set (call), base))
> -       return true;
> -    }
> -  else if ((TREE_CODE (base) == MEM_REF
> -           || TREE_CODE (base) == TARGET_MEM_REF)
> -          && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME)
> -    {
> -      struct ptr_info_def *pi = SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0));
> -      if (!pi)
> -       return true;
> -
> -      if (pt_solutions_intersect (gimple_call_use_set (call), &pi->pt))
> -       return true;
> -    }
> -  else
> +  if (pt_solution_includes_base (gimple_call_use_set (call), base))
>      return true;
>
>    /* Inspect call arguments for passed-by-value aliases.  */
>
> --
> Alan Modra
> Australia Development Lab, IBM