[gcc-c-torture] Disable frame-address.c test due to incorrect tail call prevention

[DavidSpickett]

Summary

These checks were added to the original test by:
https://github.com/gcc-mirror/gcc/commit/0f149d5215a22a03406a82ec1669bf65b329a4df

Since 6b545db83c5a4e2c79e0b289e840be2c5fbbf327, clang has learned
to see through this tail call prevention.

First seen by us on our SVE bot which runs the test suite at -O3:
https://lab.llvm.org/buildbot/#/builders/197/builds/4192

Clang is correct here, this is why:

int check_fa_work (const char *c, const char *f)
{
  const char d = 0;

  if (c >= &d)
    return c >= f && f >= &d;
  else
    return c <= f && f <= &d;
}

This function only ever returns 0 or 1 due to the &&.

int check_fa_mid (const char *c)
{
  const char *f = __builtin_frame_address (0);

  /* Prevent a tail call to check_fa_work, eliding the current stack frame.  */
  return check_fa_work (c, f) != 0;
}

This function returns whether the result of check_fa_work is not equal to 0.

Clang has realised that if check_fa_work returns 0, check_fa_mid does also.
Same for 1. Meaning you can tail call check_fa_work.

check_fa_work returns 0, 0 != 0 is False, so check_fa_mid returns 0
check_fa_work returns 1, 1 != 0 is True, so check_fa_mid returns 1

Giving you asm like:

check_fa_mid:                           // @check_fa_mid
        stp     x29, x30, [sp, #-16]!           // 16-byte Folded Spill
        mov     x29, sp
        mov     x1, x29
        ldp     x29, x30, [sp], #16             // 16-byte Folded Reload
        b       check_fa_work

Which means we have one less frame and the test fails.

A correct way to do it would be "==" which would have to invert the result.
However we cannot modify the test files, so disable the test instead.

I have filed a bug with gcc:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109146

[DavidSpickett]

We first saw this on our SVE bot https://lab.llvm.org/buildbot/#/builders/197/builds/4192. Turns out it's not because of vectors or SVE, it happens on plain AArch64 as well, but the SVE bot was the only one not using -O0 for the test suite.

I'm not sure if we're supposed to be changing these files. Either way I will file a bug over in gcc, though gcc does not yet do this optimisation.

[DavidSpickett]

The current gcc test: https://github.com/gcc-mirror/gcc/blob/master/gcc/testsuite/gcc.c-torture/execute/frame-address.c

It's exactly the same as what we currently have, it's using !=.

[DavidSpickett]

Submitted https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109146.

[lenary]

Don't change the source code, please change the CMakeLists.txt to disable this test if it's incorrect.

[DavidSpickett]

Disable test instead of changing the source.

[DavidSpickett]

A colleague suggests that maybe there are other reasons clang shouldn't tail call here, but in the interests of getting the bot green I will land this.

Feel free to correct my reasoning.

[DavidSpickett]

Thinking about it more, the test may also be using UB. Which would explain why a change to not require noundef would have caused this. Perhaps clang has always been able to create a tail call here, if the called function did not have UB. That restriction is now gone.

The test compares a bunch of pointers that are of the same type but do not point to the same objects. https://www.open-std.org/jtc1/sc22/WG14/www/docs/n1570.pdf 6.5.8 Relational operators, if we follow item 6, I think we end up at "In all other cases, the behavior is undefined.".

Does that make sense?

[DavidSpickett]

After talking to a colleague, yes, it does make sense. I've updated the comment https://github.com/llvm/llvm-test-suite/commit/099d3c1617e0bf30723c26de3fdb24df3fe797f1.

SingleSource/Regression/C/gcc-c-torture/execute/CMakeLists.txt

file(GLOB UnsupportedTests CONFIGURE_DEPENDS
 
   # Clang at -O0 does not enable -finstrument-functions
   # (https://bugs.llvm.org/show_bug.cgi?id=49143)
   eeprof-1.c
 
   # Size of array element is not a multiple of its alignment.
   pr36093.c
   pr43783.c
+
+  # Uses an incorrect method to prevent tailcalls which clang at -O1 or above
+  # can see through and tail call regardless.
+  frame-address.c
 )
 list(APPEND TestsToSkip ${UnsupportedTests})
 
 # Tests where clang currently has bugs or issues
 file(GLOB FailingTests CONFIGURE_DEPENDS
 
   # Handling of bitfields is different between clang and GCC:
   # http://lists.llvm.org/pipermail/llvm-dev/2017-October/118507.html