Re: [PATCH] Fix PR64242

[Christophe Lyon <christophe dot lyon at linaro dot org>]

On Mon, 3 Dec 2018 at 17:26, Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> Here is a fix for the testcase, so that it doesn't FAIL pretty much
> everywhere.
>
> On Fri, Nov 30, 2018 at 04:07:31PM -0700, Jeff Law wrote:
> > >     PR middle-end/64242
> > >     * gcc.c-torture/execute/pr64242.c: New test.
> > THanks for tracking this down.  I'd like to have this run through my
> > next testing cycle, so I went ahead and installed  it for you.
>
> What I've tested:
> 1) x86_64-linux {-m32,-m64} - without the testcase patch, the testcase FAILs
>    without or with the builtins.c change; with the testcase patch and
>    witout the builtins.c change, there is
> FAIL: gcc.c-torture/execute/pr64242.c   -O2  execution test
> FAIL: gcc.c-torture/execute/pr64242.c   -O3 -g  execution test
> FAIL: gcc.c-torture/execute/pr64242.c   -Os  execution test
>    for -m32 and no FAILs for -m64, with the builtins.c change the tests
>    passes on both -m32 and -m64
> 2) powerpc64-linux {-m32,-m64} - without the testcase patch, the testcase
>    FAILs without and with the builtins.c change for -m32.  With the testcase
>    patch and without the builtins.c change, there is
> FAIL: gcc.c-torture/execute/pr64242.c   -O0  execution test
> FAIL: gcc.c-torture/execute/pr64242.c   -O1  execution test
> FAIL: gcc.c-torture/execute/pr64242.c   -O2  execution test
> FAIL: gcc.c-torture/execute/pr64242.c   -O3 -g  execution test
> FAIL: gcc.c-torture/execute/pr64242.c   -Os  execution test
>    for -m32 and
> FAIL: gcc.c-torture/execute/pr64242.c   -O0  execution test
> FAIL: gcc.c-torture/execute/pr64242.c   -O1  execution test
>    for -m64, with the builtins.c change everything passes
> 3) aarch64-linux - both without and with the testcase patch, the
>    testcase FAILs without the builtins.c change and passes with it
>
> Ok for trunk?
>
> 2018-12-03  Jakub Jelinek  <jakub@redhat.com>
>
>         PR middle-end/64242
>         * gcc.c-torture/execute/pr64242.c (foo, bar): New functions.
>         (p): Make it void *volatile instead of volatile void *.
>         (q): New variable.
>         (main): Add a dummy 32-byte aligned variable and escape its address.
>         Don't require that the two __builtin_alloca (0) calls return the
>         same address, just require that their difference is smaller than
>         1024 bytes.
>

Hi Jakub,

This commit didn't fix the failure I reported on some arm targets, and
it introduced
a regression on aarch64-none-elf with -mabi=ilp32:
FAIL: gcc.c-torture/execute/pr64242.c   -O0  execution test
Il seems the test timed-out.
Higher optimization levels still pass.

Christophe


> --- gcc/testsuite/gcc.c-torture/execute/pr64242.c.jj    2018-12-01 00:25:08.082009500 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr64242.c       2018-12-03 16:51:51.869797742 +0100
> @@ -3,7 +3,7 @@
>  extern void abort (void);
>
>  __attribute ((noinline)) void
> -broken_longjmp(void *p)
> +broken_longjmp (void *p)
>  {
>    void *buf[5];
>    __builtin_memcpy (buf, p, 5 * sizeof (void*));
> @@ -11,20 +11,41 @@ broken_longjmp(void *p)
>    __builtin_longjmp (buf, 1);
>  }
>
> +__attribute ((noipa)) __UINTPTR_TYPE__
> +foo (void *p)
> +{
> +  return (__UINTPTR_TYPE__) p;
> +}
> +
> +__attribute ((noipa)) void
> +bar (void *p)
> +{
> +  asm volatile ("" : : "r" (p));
> +}
> +
>  volatile int x = 0;
> -volatile void *p;
> +void *volatile p;
> +void *volatile q;
> +
>  int
> -main (void)
> +main ()
>  {
>    void *buf[5];
> +  struct __attribute__((aligned (32))) S { int a[4]; } s;
> +  bar (&s);
>    p = __builtin_alloca (x);
> -
>    if (!__builtin_setjmp (buf))
>      broken_longjmp (buf);
>
>    /* Fails if stack pointer corrupted.  */
> -  if (p != __builtin_alloca (x))
> -    abort();
> +  q = __builtin_alloca (x);
> +  if (foo (p) < foo (q))
> +    {
> +      if (foo (q) - foo (p) >= 1024)
> +       abort ();
> +    }
> +  else if (foo (p) - foo (q) >= 1024)
> +    abort ();
>
>    return 0;
>  }
>
>         Jakub