Fix sanitizer tool list used to generate sanitizer_common tests to be up-to-date.

[delcypher]

Summary

This replaces the sanitizer tool list (used for generating
sanitizer_common configurations) with a tool list derived from
existing build system information.

Previously sanitizer_common had its own list of supported sanitizer
tools. This was bad because it was out of sync with the rest of the
build system. Notably it meant that the sanitizer_common runtime was
only being tested on Darwin the ASan dylib and not the other sanitizer
dylibs that are built for Darwin (LSan, TSan, and UBSan).

Unfortunately enabling the tests against other sanitizer dylibs has lead
to some test failures on Darwin. For now they've been marked as
unsupported until the failures can investigated properly.

[krytarowski]

I propose to keep UNSUPPORTED for tests that are not applicable for Darwin or are randomly hanging/crashing.

[delcypher]

In D55740#1332296, @krytarowski wrote:

I propose to keep UNSUPPORTED for tests that are not applicable for Darwin or are randomly hanging/crashing.

Are you saying you'd rather I use XFAIL?

[krytarowski]

In D55740#1332339, @delcypher wrote:

In D55740#1332296, @krytarowski wrote:

I propose to keep UNSUPPORTED for tests that are not applicable for Darwin or are randomly hanging/crashing.

Are you saying you'd rather I use XFAIL?

Yes, unless a test is flaky and produces random success.

[delcypher]

test/sanitizer_common/CMakeLists.txt

51

Should we guard against ANDROID and WINDOWS here? We weren't running on those platforms previously and it's likely that this change would cause the tests to be run for those platforms.

[delcypher]

In D55740#1332340, @krytarowski wrote:

In D55740#1332339, @delcypher wrote:

In D55740#1332296, @krytarowski wrote:

I propose to keep UNSUPPORTED for tests that are not applicable for Darwin or are randomly hanging/crashing.

Are you saying you'd rather I use XFAIL?

Yes, unless a test is flaky and produces random success.

Okay. I'll change it. The intention is that I'll fix these tests at a later date because some of the failures look like real bugs to me.

[delcypher]

Switch from annotating failing tests on Darwin from UNSUPPORTED to XFAIL.

[delcypher]

Fix up abort_on_error.cc test. It won't work for LSan.

[delcypher]

Don't run on Android or Windows for now.

[delcypher]

Don't run LSan-i386 tests on Darwin by default because LSan is broken in that case.

[vitalybuka]

test/sanitizer_common/CMakeLists.txt

22

we run sanitizer_common tests with asan on Android
I guess we have COMPILER_RT_HAS_UBSAN on android as well, but we don't sanitizer_common tests there

[delcypher]

test/sanitizer_common/CMakeLists.txt

22

@vitalybuka Oops. Sorry I screwed up there. What would like to do here? If you do ship UBSan on Android I would suggest running the sanitizer_common tests against UBSan. However, I don't have any android devices so I can't easily fix any breakages that this introduces, so perhaps for Android I should use the existing behaviour of only running against ASan?

[delcypher]

@waltl Does this have any impact for RTEMS ?
@phosek Does this have any impact on Fuchsia?

I don't know how testing is performed for those platforms.

[vitalybuka]

test/sanitizer_common/CMakeLists.txt

28

Could you please and submit patch add printing of SUPPORTED_TOOLS into existing code and wait some time
so we will have something on buildbots to compare after the current one is landed

[delcypher]

test/sanitizer_common/CMakeLists.txt

28

@vitalybuka Just to clarify what you're asking for (I've got this wrong before)

• Submit a separate patch that prints out what SUPPORTED_TOOLS is now (i.e. not what this patch tries to make SUPPORTED_TOOLS become).
• Wait for some builds to happen
• Rebase this patch and then look at the output of several bots to see in which scenarios we are running tests today and try to decide if we should change this patch to try and preserve the existing scenarios or expand on them.

Correct?

[waltl]

In D55740#1335964, @delcypher wrote:

@waltl Does this have any impact for RTEMS ?

We don't use CMake for RTEMS so this change doesn't affect it.

[vitalybuka]

test/sanitizer_common/CMakeLists.txt

28

yes.

[delcypher]

test/sanitizer_common/CMakeLists.txt

28

@vitalybuka Sorry for the delay. Here's the CMake patch --> https://reviews.llvm.org/D55939

[vitalybuka]

test/sanitizer_common/CMakeLists.txt

26

1. Remove: "if (NOT (ANDROID OR WINDOWS))"
2. Create the list
3. Print the list
4. for windows and android:
   • replace the list and with hardcoded one
   • print the new list and with some FIXME warning about this replacement

test/sanitizer_common/TestCases/symbolize_stack.cc

6

why is this not XFAIL?

[delcypher]

• Put legacy list back but only use it for Windows and Android.
• Print warning when we use the legacy list.
• Print the generated SUPPORTED_TOOLS list.

[delcypher]

test/sanitizer_common/CMakeLists.txt

26

@vitalybuka I've updated the patch with what (I think) you asked for.

test/sanitizer_common/TestCases/symbolize_stack.cc

6

I think the original reason was the for LSan on i386 it would hang (crashed while doing stop-the-world). However because I prevent those tests being run from ninja check-sanitizer in the CMakeLists.txt file, using XFAIL, is probably okay.

[vitalybuka]

test/sanitizer_common/CMakeLists.txt

43

For simplicity you can drop SUPPORTED_TOOLS_LEGACY and just use:
If Windows SUPPORTED_TOOLS=""
If Android SUPPORTED_TOOLS="asan"

test/sanitizer_common/TestCases/symbolize_stack.cc

6

Thanks.
Yes, question was just about XFAIL vs UNSOPPORTED. Former has some benefits if we expect to fix it (and it fails consistently).

[delcypher]

• Remove SUPPORTED_TOOLS_LEGACY

[delcypher]

@vitalybuka Sorry the delay in getting to this. Is this good to go?

[vitalybuka]

Thank you!
LGTM

test/sanitizer_common/CMakeLists.txt

 set(SANITIZER_COMMON_LIT_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 
 set(SANITIZER_COMMON_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
 set(SANITIZER_COMMON_TESTSUITES)
 
-set(SUPPORTED_TOOLS)
+# FIXME(dliew): Remove this once we are happy that the new `SUPPORTED_TOOLS`
+# doesn't break the build for all platforms we care about.
+# Temporary helper for https://reviews.llvm.org/D55740
+set(SUPPORTED_TOOLS_LEGACY)
 if(CMAKE_SYSTEM_NAME MATCHES "Darwin|Linux|FreeBSD|NetBSD|SunOS")
-  list(APPEND SUPPORTED_TOOLS asan)
+  list(APPEND SUPPORTED_TOOLS_LEGACY asan)
 endif()
 if(CMAKE_SYSTEM_NAME MATCHES "NetBSD" OR (CMAKE_SYSTEM_NAME MATCHES "Linux" AND NOT ANDROID))
-  list(APPEND SUPPORTED_TOOLS tsan)
-  list(APPEND SUPPORTED_TOOLS msan)
-  list(APPEND SUPPORTED_TOOLS ubsan)
+  list(APPEND SUPPORTED_TOOLS_LEGACY tsan)
+  list(APPEND SUPPORTED_TOOLS_LEGACY msan)
+  list(APPEND SUPPORTED_TOOLS_LEGACY ubsan)
 endif()
 if(CMAKE_SYSTEM_NAME MATCHES "Linux" AND NOT ANDROID)
-  list(APPEND SUPPORTED_TOOLS lsan)
+  list(APPEND SUPPORTED_TOOLS_LEGACY lsan)
+endif()
+
+# FIXME(dliew): We should switch to COMPILER_RT_SANITIZERS_TO_BUILD instead of
+# the hard coded `SUPPORTED_TOOLS_INIT` list once we know that the other
+# sanitizers work.
+set(SUPPORTED_TOOLS_INIT asan lsan msan tsan ubsan)
+set(SUPPORTED_TOOLS)
+  foreach(SANITIZER_TOOL ${SUPPORTED_TOOLS_INIT})
+    string(TOUPPER ${SANITIZER_TOOL} SANITIZER_TOOL_UPPER)
+    if (COMPILER_RT_HAS_${SANITIZER_TOOL_UPPER})
+      list(APPEND SUPPORTED_TOOLS ${SANITIZER_TOOL})
+    endif()
+  endforeach()
+
+# FIXME(dliew): Remove this.
+# Temporary helper for https://reviews.llvm.org/D55740
+message(
+  STATUS
+  "Generated Sanitizer SUPPORTED_TOOLS list on \"${CMAKE_SYSTEM_NAME}\" is"
+  " \"${SUPPORTED_TOOLS}\"")
+
+# FIXME(dliew): These tests should be made to work on all platforms.
+if (ANDROID OR WINDOWS)
+  set(SUPPORTED_TOOLS ${SUPPORTED_TOOLS_LEGACY})
+  message(
+    AUTHOR_WARNING
+    "Replacing Sanitizer SUPPORTED_TOOLS list (${SUPPORTED_TOOLS}) with "
+    "SUPPORTED_TOOLS_LEGACY (${SUPPORTED_TOOLS_LEGACY})")
 endif()
 
 # FIXME(dliew): Remove this.
 # Temporary helper for https://reviews.llvm.org/D55740
 message(
   STATUS
   "sanitizer_common tests on \"${CMAKE_SYSTEM_NAME}\" will run against "
   "\"${SUPPORTED_TOOLS}\"")
 
 # Create a separate config for each tool we support.
 foreach(tool ${SUPPORTED_TOOLS})
   string(TOUPPER ${tool} tool_toupper)
   if(${tool_toupper}_SUPPORTED_ARCH AND NOT COMPILER_RT_STANDALONE_BUILD)
     list(APPEND SANITIZER_COMMON_TEST_DEPS ${tool})
   endif()
   set(TEST_ARCH ${${tool_toupper}_SUPPORTED_ARCH})
   if(APPLE)
     darwin_filter_host_archs(${tool_toupper}_SUPPORTED_ARCH TEST_ARCH)
   endif()
 
+  # TODO(dliew): We should iterate over the different
+  # Apple platforms, not just macOS.
   foreach(arch ${TEST_ARCH})
     set(SANITIZER_COMMON_LIT_TEST_MODE ${tool})
     set(SANITIZER_COMMON_TEST_TARGET_ARCH ${arch})
     get_test_cc_for_arch(${arch} SANITIZER_COMMON_TEST_TARGET_CC SANITIZER_COMMON_TEST_TARGET_CFLAGS)
     set(CONFIG_NAME ${tool}-${arch}-${OS_NAME})
     configure_lit_site_cfg(
       ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in
       ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME}/lit.site.cfg)
+    # FIXME(dliew): LSan i386 on Darwin is completly broken right now.
+    # so don't run the tests by default.
+    if (NOT (CMAKE_SYSTEM_NAME MATCHES "Darwin" AND
+             ${tool} STREQUAL "lsan" AND
+             ${arch} STREQUAL "i386"))
       list(APPEND SANITIZER_COMMON_TESTSUITES
            ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME})
+    endif()
   endforeach()
 endforeach()
 
 # Unit tests.
 if(COMPILER_RT_INCLUDE_TESTS)
   configure_lit_site_cfg(
     ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.in
     ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg)

test/sanitizer_common/TestCases/Darwin/abort_on_error.cc

 // Check that sanitizers on OS X crash the process by default (i.e.
 // abort_on_error=1). See also Linux/abort_on_error.cc.
 
-// RUN: %clangxx %s -o %t
+// RUN: %clangxx -DUSING_%tool_name %s -o %t
 
 // Intentionally don't inherit the default options.
 // RUN: env %tool_options='' not --crash %run %t 2>&1
 
 // When we use lit's default options, we shouldn't crash.
 // RUN: not %run %t 2>&1
 
+// Leak detection isn't treated as an error so `abort_on_error=1` doesn't work.
+// UNSUPPORTED: lsan
+
 int global;
 
 int main() {
+#if defined(USING_ubsan)
+  int value = 5;
+  int computation = value / 0; // Division by zero.
+#else
   volatile int *a = new int[100];
   delete[] a;
-  global = a[0];  // use-after-free: triggers ASan report.
+  global = a[0]; // use-after-free: triggers ASan/TSan report.
+#endif
   return 0;
 }

test/sanitizer_common/TestCases/Darwin/print-stack-trace.cc

 // RUN: %clangxx -O0 %s -o %t && %env_tool_opts=stack_trace_format=DEFAULT %run %t 2>&1 | FileCheck %s
 // RUN: %env_tool_opts=stack_trace_format='"frame:%n lineno:%l"' %run %t 2>&1 | FileCheck %s --check-prefix=CUSTOM
 
+// FIXME(dliew): Make this test work with other sanitizers
+// XFAIL: darwin && (lsan || tsan || ubsan)
+
 #include <sanitizer/common_interface_defs.h>
 
 static inline void FooBarBaz() {
   __sanitizer_print_stack_trace();
 }
 
 int main() {
   FooBarBaz();
   return 0;
 }
 // CHECK: {{    #0 0x.* in __sanitizer_print_stack_trace}}
 // CHECK: {{    #1 0x.* in FooBarBaz(\(\))? .*}}print-stack-trace.cc:[[@LINE-8]]
 // CHECK: {{    #2 0x.* in main.*}}print-stack-trace.cc:[[@LINE-5]]
 
 // CUSTOM: frame:1 lineno:[[@LINE-11]]
 // CUSTOM: frame:2 lineno:[[@LINE-8]]

test/sanitizer_common/TestCases/Posix/dedup_token_length_test.cc

 // Test dedup_token_length
 // RUN: %clangxx -O0 %s -o %t
 // RUN: env %tool_options='abort_on_error=0'                       not %run %t 2>&1   | FileCheck %s --check-prefix=CHECK0 --match-full-lines
 // RUN: env %tool_options='abort_on_error=0, dedup_token_length=0' not %run %t 2>&1   | FileCheck %s --check-prefix=CHECK0 --match-full-lines
 // RUN: env %tool_options='abort_on_error=0, dedup_token_length=1' not %run %t 2>&1   | FileCheck %s --check-prefix=CHECK1 --match-full-lines
 // RUN: env %tool_options='abort_on_error=0, dedup_token_length=2' not %run %t 2>&1   | FileCheck %s --check-prefix=CHECK2 --match-full-lines
 // RUN: env %tool_options='abort_on_error=0, dedup_token_length=3' not %run %t 2>&1   | FileCheck %s --check-prefix=CHECK3 --match-full-lines
 
 // REQUIRES: stable-runtime
 
 // XFAIL: netbsd && !asan
 
+// FIXME(dliew): Make this test work with other sanitizers
+// XFAIL: darwin && (lsan || tsan || ubsan)
+
 volatile int *null = 0;
 
 namespace Xyz {
   template<class A, class B> void Abc() {
     *null = 0;
   }
 }
 

test/sanitizer_common/TestCases/Posix/sanitizer_set_death_callback_test.cc

 // RUN: %clangxx -O2 %s -o %t && not %run %t 2>&1 | FileCheck %s
 
 // REQUIRES: stable-runtime
 
 // XFAIL: ubsan
+// FIXME(dliew): Make this test work on Darwin with LSan
+// XFAIL: darwin && lsan
 
 #include <sanitizer/common_interface_defs.h>
 #include <stdio.h>
 
 volatile char *zero = 0;
 
 void Death() {
   fprintf(stderr, "DEATH CALLBACK EXECUTED\n");

test/sanitizer_common/TestCases/symbolize_stack.cc

 // RUN: %clangxx -O0 %s -o %t && %run %t 2>&1 | FileCheck %s
 
 // Test that symbolizer does not crash on frame with large function name.
 
+// FIXME(dliew): Make this test work with the other sanitizers.
+// XFAIL: darwin && (lsan || tsan || ubsan)
+
 #include <sanitizer/common_interface_defs.h>
 #include <vector>
 
 template <int N> struct A {
   template <class T> void RecursiveTemplateFunction(const T &t);
 };
 
 template <int N>