Re: [PATCH] Fix ICE with bogus posix_memalign call (PR middle-end/67222)

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Aug 18, 2015 at 12:28 PM, Marek Polacek <polacek@redhat.com> wrote:
> On Tue, Aug 18, 2015 at 10:47:44AM +0200, Richard Biener wrote:
>> On Mon, Aug 17, 2015 at 8:01 PM, Marek Polacek <polacek@redhat.com> wrote:
>> > Here we were crashing on an invalid call to posix_memalign.  The code in
>> > lower_builtin_posix_memalign assumed that the call had valid arguments.
>> > The reason the C FE doesn't reject this code is, in short, that
>> > int <T> () is compatible with int <T> (void **, size_t, size_t) and we
>> > use the former -- so convert_arguments doesn't complain.
>> >
>> > So I think let's validate the arguments in lower_stmt.  I decided to
>> > give an error if we see an invalid usage of posix_memalign, since
>> > other code (e.g. alias machinery) assumes correct arguments as well.
>> >
>> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
>>
>> I don't think you can give errors here.  Note that the "appropriate"
>> way to do the check is to simply use
>
> Yeah, because the weirdo call is only undefined if it's reached at runtime,
> right.
>
>> if (gimple_builtin_call_types_compatible_p (stmt, decl))
>
> Nice, dunno how could I not find that.
>
>> not lowering in case it's not compatible is ok.
>
> In that case I also need to check a place in tree-ssa-alias.c.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk and 5?

Please instead change the tree-ssa-alias.c code to do

  if (callee != NULL
      && gimple_call_builtin_p (call, BUILT_IN_NORMAL))
    switch (DECL_FUNCTION_CODE (callee))
      ...

which should also fix quite a few issues in the other builtin handlings.
Likewise can you change stmt_kills_ref_p and ref_maybe_used_by_call_p_1
in a similar way?

Thanks,
Richard.

> 2015-08-18  Marek Polacek  <polacek@redhat.com>
>
>         PR middle-end/67222
>         * gimple-low.c (lower_stmt): Check the posix_memalign call.
>         * tree-ssa-alias.c (call_may_clobber_ref_p_1): Likewise.
>
>         * gcc.dg/torture/pr67222.c: New test.
>
> diff --git gcc/gimple-low.c gcc/gimple-low.c
> index d4697e2..4eae3a0 100644
> --- gcc/gimple-low.c
> +++ gcc/gimple-low.c
> @@ -346,7 +346,8 @@ lower_stmt (gimple_stmt_iterator *gsi, struct lower_data *data)
>                 return;
>               }
>             else if (DECL_FUNCTION_CODE (decl) == BUILT_IN_POSIX_MEMALIGN
> -                    && flag_tree_bit_ccp)
> +                    && flag_tree_bit_ccp
> +                    && gimple_builtin_call_types_compatible_p (stmt, decl))
>               {
>                 lower_builtin_posix_memalign (gsi);
>                 return;
> diff --git gcc/testsuite/gcc.dg/torture/pr67222.c gcc/testsuite/gcc.dg/torture/pr67222.c
> index e69de29..739f869 100644
> --- gcc/testsuite/gcc.dg/torture/pr67222.c
> +++ gcc/testsuite/gcc.dg/torture/pr67222.c
> @@ -0,0 +1,19 @@
> +/* PR middle-end/67222 */
> +/* { dg-do compile } */
> +
> +void
> +foo (void **p)
> +{
> +  posix_memalign (); /* { dg-warning "implicit declaration" } */
> +  posix_memalign (p);
> +  posix_memalign (0);
> +  posix_memalign (p, 1);
> +  posix_memalign (p, "foo");
> +  posix_memalign ("gnu", "gcc");
> +  posix_memalign (1, p);
> +  posix_memalign (1, 2);
> +  posix_memalign (1, 2, 3);
> +  posix_memalign (p, p, p);
> +  posix_memalign (p, "qui", 3);
> +  posix_memalign (p, 1, 2);
> +}
> diff --git gcc/tree-ssa-alias.c gcc/tree-ssa-alias.c
> index e103220..e96a00c 100644
> --- gcc/tree-ssa-alias.c
> +++ gcc/tree-ssa-alias.c
> @@ -2039,6 +2039,10 @@ call_may_clobber_ref_p_1 (gcall *call, ao_ref *ref)
>            by its first argument.  */
>         case BUILT_IN_POSIX_MEMALIGN:
>           {
> +           /* For weirdo calls we cannot say much so stay conservative.  */
> +           tree decl = gimple_call_fndecl (call);
> +           if (!gimple_builtin_call_types_compatible_p (call, decl))
> +             return true;
>             tree ptrptr = gimple_call_arg (call, 0);
>             ao_ref dref;
>             ao_ref_init_from_ptr_and_size (&dref, ptrptr,
>
>         Marek