[PATCH] D91620: [compiler-rt][test] Heed COMPILER_RT_DEBUG when compiling unittests

[Rainer Orth via Phabricator via llvm-commits]

ro added a comment.

I think I've figured out what's wrong.  AFAICS there are three separate issues:

- Stray `;` in compiler flags:

  `cmake --trace` revealed that while `SANITIZER_COMMON_CFLAGS` is added to with `list(APPEND)`, all additions to `COMPILER_RT_TEST_COMPILER_CFLAGS` are with `string(APPEND)`,  which explains the ` ` vs. `;` difference.  While this difference is highly confusing, it's easily fixed.

- Tests `FAIL`ing in `Debug`/`COMPILER_RT_DEBUG` builds:

  E.g `ThreadSanitizer-x86_64 :: Linux/check_memcpy.c` which now contains calls to `memset` when previously there were none.  This can be explained by the change from `-O1` to `-O0`, is to be expected IMO and can be avoided by `XFAIL`ing the test.  The only thing I'm not certain about is if this affects only select targets (`x86_64` so far)  or all.

- Tests `FAIL`ing in `Release` builds:

  There are quite a number of those, e.g. Issue #58169, ` SanitizerCommon-asan-aarch64-Linux :: Posix/weak_hook_test.cpp`.  Comparing compile commands for the latter between Linux/aarch64 and Linux/x86_64 reveals: the former uses `-gline-tables-only -fsanitize=address   -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -O3 -gline-tables-only  -funwind-tables` while the latter only has `-gline-tables-only -fsanitize=address  -m64 -funwind-tables `, i.e. `-O3` among others is lost/missing.  If I build the test on Linux/x86_64 with -O3, it fails the same way as on Linux/aarch64.  I found that `lit.common.configured` has

  set_default("target_cflags", " -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -O3 -gline-tables-only ")

on Linux/x86_64, too, but that's later overridden in `asan/X86_64LinuxConfig/lit.site.cfg.py` which has just

  config.target_cflags = "-m64"

However, there's more: checking `cmake --trace` again shows that before my patch ` COMPILER_RT_TEST_COMPILER_CFLAGS` is only changed/set like

  compiler-rt/CMakeLists.txt(400):  string(APPEND COMPILER_RT_TEST_COMPILER_CFLAGS  ${thread_safety_flags_space_sep} )
  compiler-rt/CMakeLists.txt(538):  string(APPEND COMPILER_RT_TEST_COMPILER_CFLAGS  ${stdlib_flag} )
  compiler-rt/CMakeLists.txt(542):  string(REPLACE   ; COMPILER_RT_UNITTEST_CFLAGS ${COMPILER_RT_TEST_COMPILER_CFLAGS} )

while now there's

  compiler-rt/CMakeLists.txt(400):  string(APPEND COMPILER_RT_TEST_COMPILER_CFLAGS  ${thread_safety_flags_space_sep} )
  compiler-rt/CMakeLists.txt(413):  string(APPEND COMPILER_RT_TEST_COMPILER_CFLAGS  -O3 )
  compiler-rt/CMakeLists.txt(459):  string(APPEND COMPILER_RT_TEST_COMPILER_CFLAGS  -gline-tables-only )
  compiler-rt/CMakeLists.txt(546):  string(APPEND COMPILER_RT_TEST_COMPILER_CFLAGS  ${stdlib_flag} )
  compiler-rt/CMakeLists.txt(550):  string(REPLACE   ; COMPILER_RT_UNITTEST_CFLAGS ${COMPILER_RT_TEST_COMPILER_CFLAGS} )

So, `-O3` is added when it wasn't there before.  While I believe this is correct (a `Release` build should be tested with optimization), but given the fallout this should be omitted for now and handled separately.

In addition to this, I believe I've found a way to define an instantiation of `__sanitizer::integral_constant<bool, true>::value`, based on what GCC's `<type_traits>`.  The only thing I'm unsure about is if we can rely on the presence of `__cpp_inline_variables`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91620