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 <ja...@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
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
