[PATCH] D60644: [compiler-rt][builtins][sanitizers] Guard test cases with macros to run when specific version of GLIBC is detected

Mon Apr 15 20:32:54 PDT 2019

hubert.reinterpretcast added a comment.

In D60644#1467676 <https://reviews.llvm.org/D60644#1467676>, @amyk wrote:

> In terms of `qdiv_test` and `divsc3_test`, do you think checking perhaps checking the version of GCC is a more suitable fix?

The current pattern being used in the patch reports tests as passing when failures are being suppressed. An approach using XFAIL (based on the GCC version used to build compiler-rt) would be more suitable. The failure should also be filed as a bug (and the bug mentioned when adding the XFAIL) since the affected version of GCC is still currently supported as a build compiler (albeit deprecated). I am not sure if there is any motivation to really fix said bug; but, if there is no such motivation, then I also question the motivation behind building with the affected version of GCC and the motivation for suppressing the failures when such a build is done. In other words, if your goal is to build with a compiler that causes these test failures, then what needs to be done in the end is to implement a workaround in the compiler-rt source (and not the tests) for the build compiler problem. If people are happy to just avoid building with the affected version of GCC, then I think it makes sense to just leave these two tests alone.

> I could be mistaken, but doesn't the `getpw_getgr` test still succeed regardless if `-D_GLIBCXX_USE_CXX11_ABI` is set (within a toolchain that supports setting this)?

I am not sure if `_GLIBCXX_USE_CXX11_ABI` is `1` by default for all toolchains that support this value of the macro.

> Would it be better if we checked for the macro definition `_GLIBCXX_USE_CXX11_ABI` instead?

That would make more sense, but is having the test "pass" the right thing to do? Users can set `-D_GLIBCXX_USE_CXX11_ABI=0` for their applications. Is the failure caused by a genuine problem with libstdc++?

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60644/new/

https://reviews.llvm.org/D60644