Prefer atos to llvm-symbolizer on Darwin

[george.karpenkov]

Summary

atos is the default symbolizer on Apple's compiler for quite a few years now.
llvm-symbolizer is quite fragile on Darwin: for example, unless a .dSYM file was explicitly generated symbolication would not work.
It is also very convenient when the behavior of LLVM open source compiler matches to that of Apple's compiler on Apple's platform.
Furthermore, llvm-symbolizer is not installed on Apple's platform by default, which leads to strange behavior during debugging: the test might fail under lit (where it has llvm-symbolizer) but would run properly when launched on the command line (where it does not, and atos would be used).

Indeed, there's a downside: atos does not work properly with inlined functions, hence the test change.
We do not think that this is a major problem, as users would often compile with -O0 when debugging, and in any case it is preferable to symbolizer not being able to symbolize.

[kcc]

Code LG

test/asan/TestCases/suppressions-function.cc

1

I'd prefer if you keep this test unchanged (only extend the UNSUPPORTED) and add a separate test to test/asan/TestCases/Darwin

[george.karpenkov]

Applied the comments

[kcc]

LGTM with a nit
Please remove all non-darwin things (windows, android, etc) from the new file test/asan/TestCases/Darwin/suppressions-function.cc

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

#endif  // SANITIZER_MAC
     Report("ERROR: External symbolizer path is set to '%s' which isn't "
            "a known symbolizer. Please set the path to the llvm-symbolizer "
            "binary or other known tool.\n", path);
     Die();
   }
 
   // Otherwise symbolizer program is unknown, let's search $PATH
   CHECK(path == nullptr);
-  if (const char *found_path = FindPathToBinary("llvm-symbolizer")) {
-    VReport(2, "Using llvm-symbolizer found at: %s\n", found_path);
-    return new(*allocator) LLVMSymbolizer(found_path, allocator);
-  }
 #if SANITIZER_MAC
   if (const char *found_path = FindPathToBinary("atos")) {
     VReport(2, "Using atos found at: %s\n", found_path);
     return new(*allocator) AtosSymbolizer(found_path, allocator);
   }
 #endif  // SANITIZER_MAC
+  if (const char *found_path = FindPathToBinary("llvm-symbolizer")) {
+    VReport(2, "Using llvm-symbolizer found at: %s\n", found_path);
+    return new(*allocator) LLVMSymbolizer(found_path, allocator);
+  }
   if (common_flags()->allow_addr2line) {
     if (const char *found_path = FindPathToBinary("addr2line")) {
       VReport(2, "Using addr2line found at: %s\n", found_path);
       return new(*allocator) Addr2LinePool(found_path, allocator);
     }
   }
   return nullptr;
 }

test/asan/TestCases/Helpers/crashing-function.cc

+// Helper source for suppressions-functions-*.cc tests:
+// checking that we can suppress the failure properly in the optimized
+// and unoptimized mode.
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+void crash_function() {
+  char *a = (char *)malloc(6);
+  free(a);
+  size_t len = strlen(a); // BOOM
+  fprintf(stderr, "strlen ignored, len = %zu\n", len);
+}
+
+int main() {
+  crash_function();
+}

test/asan/TestCases/suppressions-function.cc

-// Check that without suppressions, we catch the issue.
-// RUN: %clangxx_asan -O0 %s -o %t
-// RUN: not %run %t 2>&1 | FileCheck --check-prefix=CHECK-CRASH %s
-
-// RUN: echo "interceptor_via_fun:crash_function" > %t.supp
-// RUN: %clangxx_asan -O0 %s -o %t && %env_asan_opts=suppressions='"%t.supp"' %run %t 2>&1 | FileCheck --check-prefix=CHECK-IGNORE %s
-// RUN: %clangxx_asan -O3 %s -o %t && %env_asan_opts=suppressions='"%t.supp"' %run %t 2>&1 | FileCheck --check-prefix=CHECK-IGNORE %s
-
-// FIXME: Windows symbolizer needs work to make this pass.
-// XFAIL: android,win32
-// UNSUPPORTED: ios
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-
-void crash_function() {
-  char *a = (char *)malloc(6);
-  free(a);
-  size_t len = strlen(a); // BOOM
-  fprintf(stderr, "strlen ignored, len = %zu\n", len);
-}
-
-int main() {
-  crash_function();
-}
-
-// CHECK-CRASH: AddressSanitizer: heap-use-after-free
-// CHECK-CRASH-NOT: strlen ignored
-// CHECK-IGNORE-NOT: AddressSanitizer: heap-use-after-free
-// CHECK-IGNORE: strlen ignored

test/asan/TestCases/suppressions-functions-optimized.cc

+// RUN: echo "interceptor_via_fun:crash_function" > %t.supp
+// RUN: %clangxx_asan -O3 %S/Helpers/crashing-function.cc -o %t && %env_asan_opts=suppressions='"%t.supp"' %run %t 2>&1 | FileCheck --check-prefix=CHECK-IGNORE %s
+
+// FIXME: Windows symbolizer needs work to make this pass.
+// XFAIL: android,win32
+// UNSUPPORTED: ios
+
+// FIXME: atos does not work for inlined functions, yet llvm-symbolizer
+// does not always work with debug info on Darwin.
+// UNSUPPORTED: darwin
+
+// CHECK-IGNORE-NOT: AddressSanitizer: heap-use-after-free
+// CHECK-IGNORE: strlen ignored

test/asan/TestCases/suppressions-functions-unoptimized.cc

+// Check that without suppressions, we catch the issue.
+// RUN: %clangxx_asan -O0 %S/Helpers/crashing-function.cc -o %t
+// RUN: not %run %t 2>&1 | FileCheck --check-prefix=CHECK-CRASH %s
+
+// RUN: echo "interceptor_via_fun:crash_function" > %t.supp
+// RUN: %clangxx_asan -O0 %S/Helpers/crashing-function.cc -o %t && %env_asan_opts=suppressions='"%t.supp"' %run %t 2>&1 | FileCheck --check-prefix=CHECK-IGNORE %s
+
+// FIXME: Windows symbolizer needs work to make this pass.
+// XFAIL: android,win32
+// UNSUPPORTED: ios
+
+// CHECK-CRASH: AddressSanitizer: heap-use-after-free
+// CHECK-CRASH-NOT: strlen ignored
+// CHECK-IGNORE-NOT: AddressSanitizer: heap-use-after-free
+// CHECK-IGNORE: strlen ignored