Use ECF_MAY_BE_ALLOCA for __builtin_alloca_with_align (PR tree-optimization/68680)

Hi!

As mentioned in the PR, GCC 4.7+ seems to have regressed for
-fstack-protector*, functions containing VLAs and no other arrays are not
protected anymore.  Before 4.7, VLAs were gimplified as __builtin_alloca
call, which sets ECF_MAY_BE_ALLOCA and in turn cfun->calls_alloca.
These two are used in various places:
1) for stack protector purposes (this issue), early during expansion
2) in the inliner
3) for tail call optimization
4) for some non-NULL optimizations
and tons of places in RTL.  As 4.7+ emits __builtin_alloca_with_align
instead and special_function_p has not been adjusted, this does not happen
any longer, though cfun->calls_alloca gets set during the expansion of
__builtin_alloca_with_align, so for RTL optimizers it is already set.

The following patch restores the previous behavior, making VLAs be
ECF_MAY_BE_ALLOCA and cfun->calls_alloca already during GIMPLE passes.
It could be also done by testing the name, but I thought that it would be
too ugly (would need another case anyway, as the current tests are for
names with length <= 16).

1) and 4) surely want to treat the VLAs like the patch does, I'm not 100%
sure about 2) and 3), as VLAs are slightly different, they release
the stack afterwards at the end of scope of the VLA var.  If we wanted to
treat the two differently, maybe we'd need another ECF* flag and another
cfun bitfield for VLAs.

The following patch has been bootstrapped/regtested on x86_64-linux and
i686-linux.

2015-12-03  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/68680
	* calls.c (special_function_p): Return ECF_MAY_BE_ALLOCA for
	BUILT_IN_ALLOCA{,_WITH_ALIGN}.

	* gcc.target/i386/pr68680.c: New test.


	Jakub

On Thu, 3 Dec 2015, Jakub Jelinek wrote:

> Hi!
> 
> As mentioned in the PR, GCC 4.7+ seems to have regressed for
> -fstack-protector*, functions containing VLAs and no other arrays are not
> protected anymore.  Before 4.7, VLAs were gimplified as __builtin_alloca
> call, which sets ECF_MAY_BE_ALLOCA and in turn cfun->calls_alloca.
> These two are used in various places:
> 1) for stack protector purposes (this issue), early during expansion
> 2) in the inliner
> 3) for tail call optimization
> 4) for some non-NULL optimizations
> and tons of places in RTL.  As 4.7+ emits __builtin_alloca_with_align
> instead and special_function_p has not been adjusted, this does not happen
> any longer, though cfun->calls_alloca gets set during the expansion of
> __builtin_alloca_with_align, so for RTL optimizers it is already set.
> 
> The following patch restores the previous behavior, making VLAs be
> ECF_MAY_BE_ALLOCA and cfun->calls_alloca already during GIMPLE passes.
> It could be also done by testing the name, but I thought that it would be
> too ugly (would need another case anyway, as the current tests are for
> names with length <= 16).
> 
> 1) and 4) surely want to treat the VLAs like the patch does, I'm not 100%
> sure about 2) and 3), as VLAs are slightly different, they release
> the stack afterwards at the end of scope of the VLA var.  If we wanted to
> treat the two differently, maybe we'd need another ECF* flag and another
> cfun bitfield for VLAs.
> 
> The following patch has been bootstrapped/regtested on x86_64-linux and
> i686-linux.

The patch is ok - it looks like you could have removed the
__builtin_alloca strcmp with it though.

Does the patch mean we inlined __builtin_alloca_with_align ()
functions?  We might run into the issue Eric fixed lately with
mixing alloca and VLAs (don't see the patch being committed though).

Richard.

> 2015-12-03  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/68680
> 	* calls.c (special_function_p): Return ECF_MAY_BE_ALLOCA for
> 	BUILT_IN_ALLOCA{,_WITH_ALIGN}.
> 
> 	* gcc.target/i386/pr68680.c: New test.
> 
> --- gcc/calls.c.jj	2015-11-26 11:17:25.000000000 +0100
> +++ gcc/calls.c	2015-12-03 19:03:59.342306457 +0100
> @@ -553,6 +553,17 @@ special_function_p (const_tree fndecl, i
>  	flags |= ECF_NORETURN;
>      }
>  
> +  if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
> +    switch (DECL_FUNCTION_CODE (fndecl))
> +      {
> +      case BUILT_IN_ALLOCA:
> +      case BUILT_IN_ALLOCA_WITH_ALIGN:
> +	flags |= ECF_MAY_BE_ALLOCA;
> +	break;
> +      default:
> +	break;
> +      }
> +
>    return flags;
>  }
>  
> --- gcc/testsuite/gcc.target/i386/pr68680.c.jj	2015-12-03 19:10:14.836037923 +0100
> +++ gcc/testsuite/gcc.target/i386/pr68680.c	2015-12-03 19:09:57.000000000 +0100
> @@ -0,0 +1,15 @@
> +/* PR tree-optimization/68680 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fstack-protector-strong" } */
> +
> +int foo (char *);
> +
> +int
> +bar (unsigned long x)
> +{
> +  char a[x];
> +  return foo (a);
> +}
> +
> +/* Verify that this function is stack protected.  */
> +/* { dg-final { scan-assembler "stack_chk_fail" } } */
> 
> 	Jakub
> 
>

> Does the patch mean we inlined __builtin_alloca_with_align ()
> functions?  We might run into the issue Eric fixed lately with
> mixing alloca and VLAs (don't see the patch being committed though).

But I'm about to do it (I was waiting for the approval of the aarch64 
specific part).  Note that I'm not really sure if we want it for 5.x too.