[compiler-rt] Add AtosSymbolizer and DlAddrSymbolizer as fallbacks for OS X

[kubamracek]

Summary

This patch changes the symbolizer chain on OS X (which currently only uses 1 symbolizer at most) to use this behavior:

• By default, use LLVMSymbolizer -> AtosSymbolizer -> DlAddrSymbolizer.
• If the llvm-symbolizer binary is not found, use AtosSymbolizer -> DlAddrSymbolizer.
• If the user specifies ASAN_SYMBOLIZER_PATH=.../atos, then use AtosSymbolizer -> DlAddrSymbolizer.

There are several reasons for these fallbacks, e.g. when fork is disabled or when the llvm-symbolizer is just not present or found on the system.

[samsonov]

The logic here is very convoluted. First of all, I don't like that you ripped parts of ChooseMainSymbolizerTools out of this function, and it now doesn't do what it says. Let's keep all this logic contained. Now, more questions:

1. would you need to fallback to DlAddrSymbolizer if internal/libbacktrace is available (I know that for now they are not available on Mac, but still). I don't think so.
2. do you need to fallback to atos if the user *explicitly* specified llvm-symbolizer in ASAN_SYMBOLIZER_PATH?
3. do you need to fallback to atos if llvm-symbolizer was detected by FindPathToBinary call?

That is, I'd first like to figure out if there's a case when we need more than one out-of-process symbolizer. If not, we can restructure the code like this (pseudocode):

if (path_to_external && path_to_external[0] == '\0') { /* add nothing */ }
else if (EndsWith(path_to_external, "llvm-symbolizer(.exe)?") { /* use llvm-symbolizer */ }
else if (EndsWith(path_to_external, "atos")) { /* use atos */ }
else if (EndsWith(path_to_external, "addr2line")) { /* use addr2line */ }
else {
  assert(path_to_external == nullptr);  // Otherwise symbolizer program is unknown
  /* choose which one of llvm-symbolizer, atos, or addr2line is available and use it */
}

You can move *this* piece (figuring out external symbolizer to use) in a separate function.

[kubamracek]

In D8285#140171, @samsonov wrote:

else if (EndsWith(path_to_external, "llvm-symbolizer(.exe)?") { /* use llvm-symbolizer */ }

Is llvm-symbolizer.exe really a thing? I though on Windows we only support the DbgHelp symbolizer.

[kubamracek]

Updating the patch. Extracted a ChooseExternalSymbolizer which selects one external symbolizer that will be used (I guess having multiple external symbolizers doesn't make sense).

[samsonov]

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

420

return nullptr here

430

Just write Die() here.

439

Die()

441

No need for else{}

[kubamracek]

Updating patch.

[samsonov]

Looks reasoable. Is sandbox-exec available on all Mac OS X systems?

test/asan/TestCases/Darwin/dyld-root-path.cc

9

Please merge this test case into test/asan/TestCases/Darwin/atos-symbolizer.cc

test/asan/TestCases/Darwin/sandbox-symbolizer.cc

13

You can reduce the number of compilations in this test:
first compile with -O0, then run two tests with sandbox-exec, then compile with -O3, then run two more tests with sandbox-exec.

[kubamracek]

Addressing review comments.

Is sandbox-exec available on all Mac OS X systems?

Yes, sandbox-exec is available on OS X versions that sanitizers are supposed to work on (10.7+).

[samsonov]

LGTM

[kubamracek]

Landed in r232908. Thanks for the review and your patience!

[acmorrow]

This change appears to have had the unfortunate effect that setting ASAN_SYMBOLIZER_PATH=/usr/bin/llvm-symbolizer-3.8 in the environment no longer works, because the strcmp on line 423 is looking for the exact string 'llvm-symbolizer'.

That is unfortunate, since on some Linux distributions like Ubuntu the llvm-symbolizer binaries are always installed with a version suffix. On my Ubuntu 16.04 system, for instance, the symbolizer is in /usr/bin/llvm-symbolizer-3.8. Adjusting PATH doesn't help in that case, for obvious reasons. So it appears that after the above change the only way to deal with this on Ubuntu is to make a temporary copy of /usr/bin/llvm-symbolizer-3.8 to a temp directory under the name llvm-symbolizer, and then either add that to PATH or point ASAN_SYMBOLIZER_PATH at the temporary llvm-symbolizer file.

If this was an intended behavior change, then OK, but if unintended it feels like a regression in 3.7 and 3.8.

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

 #include "sanitizer_common.h"
 #include "sanitizer_flags.h"
 #include "sanitizer_internal_defs.h"
 #include "sanitizer_linux.h"
 #include "sanitizer_placement_new.h"
 #include "sanitizer_procmaps.h"
 #include "sanitizer_symbolizer_internal.h"
 #include "sanitizer_symbolizer_libbacktrace.h"
+#include "sanitizer_symbolizer_mac.h"
 
 #include <unistd.h>
 
 // C++ demangling function, as required by Itanium C++ ABI. This is weak,
 // because we do not require a C++ ABI library to be linked to a program
 // using sanitizers; if it's not present, we'll just use the mangled name.
 namespace __cxxabiv1 {
   extern "C" SANITIZER_WEAK_ATTRIBUTE
#endif
   // 16K loaded modules should be enough for everyone.
   static const uptr kMaxNumberOfModuleContexts = 1 << 14;
   LoadedModule *modules_;  // Array of module descriptions is leaked.
   uptr n_modules_;
   // If stale, need to reload the modules before looking up addresses.
   bool modules_fresh_;
 };
 
-static SymbolizerTool *ChooseSymbolizer(LowLevelAllocator *allocator) {
-  if (!common_flags()->symbolize) {
-    VReport(2, "Symbolizer is disabled.\n");
-    return nullptr;
-  }
+static void ChooseMainSymbolizerTools(IntrusiveList<SymbolizerTool> *list,
+                                      LowLevelAllocator *allocator) {
   if (SymbolizerTool *tool = InternalSymbolizer::get(allocator)) {
     VReport(2, "Using internal symbolizer.\n");
-    return tool;
+    list->push_back(tool);
+    return;
   }
   if (SymbolizerTool *tool = LibbacktraceSymbolizer::get(allocator)) {
     VReport(2, "Using libbacktrace symbolizer.\n");
-    return tool;
+    list->push_back(tool);
+    return;
   }
   const char *path_to_external = common_flags()->external_symbolizer_path;
   if (path_to_external && path_to_external[0] == '\0') {
     // External symbolizer is explicitly disabled. Do nothing.
-    return nullptr;
+    return;
   }
   // Find path to llvm-symbolizer if it's not provided.
   if (!path_to_external) {
     path_to_external = FindPathToBinary("llvm-symbolizer");
   }
+#if SANITIZER_MAC
+  // On Darwin, if we can't find llvm-symbolizer, try atos.
+  if (!path_to_external) {
+    path_to_external = FindPathToBinary("atos");
+  }
+  if (internal_strcmp(StripModuleName(path_to_external), "atos") == 0) {
+    VReport(2, "Using atos at path: %s\n", path_to_external);
+    list->push_back(new(*allocator)
+                        AtosSymbolizer(path_to_external, allocator));
+    return;
+  }
+#endif
   if (path_to_external) {
     VReport(2, "Using llvm-symbolizer at path: %s\n", path_to_external);
-    return new(*allocator) LLVMSymbolizer(path_to_external, allocator);
+    list->push_back(new(*allocator)
+                        LLVMSymbolizer(path_to_external, allocator));
+#if SANITIZER_MAC
+    if (const char *atos_path = FindPathToBinary("atos")) {
+      VReport(2, "Using atos as fallback at path: %s\n", atos_path);
+      list->push_back(new(*allocator)
+                          AtosSymbolizer(atos_path, allocator));
+    }
+#endif
+    return;
   }
   if (common_flags()->allow_addr2line) {
     // If llvm-symbolizer is not found, try to use addr2line.
     if (const char *addr2line_path = FindPathToBinary("addr2line")) {
       VReport(2, "Using addr2line at path: %s\n", addr2line_path);
-      return new(*allocator) Addr2LinePool(addr2line_path, allocator);
+      list->push_back(new(*allocator) Addr2LinePool(addr2line_path, allocator));
+      return;
     }
   }
-  VReport(2, "No internal or external symbolizer found.\n");
-  return nullptr;
+  VReport(2, "No main internal or external symbolizer found.\n");
+  return;
 }
 
 Symbolizer *Symbolizer::PlatformInit() {
   IntrusiveList<SymbolizerTool> list;
   list.clear();
-  if (SymbolizerTool *tool = ChooseSymbolizer(&symbolizer_allocator_)) {
-    list.push_back(tool);
+  if (!common_flags()->symbolize) {
+    VReport(2, "Symbolizer is disabled.\n");
+  } else {
+    ChooseMainSymbolizerTools(&list, &symbolizer_allocator_);
+#if SANITIZER_MAC
+    VReport(2, "Using dladdr symbolizer as fallback.\n");
+    list.push_back(new(symbolizer_allocator_) DlAddrSymbolizer());
+#endif  // SANITIZER_MAC
   }
   return new(symbolizer_allocator_) POSIXSymbolizer(list);
 }
 
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_POSIX

test/asan/TestCases/Darwin/atos-symbolizer.cc

+// Check that the `atos` symbolizer works.
+
+// RUN: %clangxx_asan -O0 %s -o %t 
+// RUN: ASAN_OPTIONS=verbosity=2 ASAN_SYMBOLIZER_PATH=$(which atos) not %run %t 2>&1 | FileCheck %s
+
+#include <stdlib.h>
+#include <string.h>
+int main(int argc, char **argv) {
+  char *x = (char*)malloc(10 * sizeof(char));
+  memset(x, 0, 10);
+  int res = x[argc];
+  free(x);
+  free(x + argc - 1);  // BOOM
+  // CHECK: AddressSanitizer: attempting double-free{{.*}}in thread T0
+  // CHECK-DLADDR: Using atos at path
+  // CHECK: #0 0x{{.*}} in {{.*}}free
+  // CHECK: #1 0x{{.*}} in main {{.*}}atos-symbolizer.cc:[[@LINE-4]]
+  // CHECK: freed by thread T0 here:
+  // CHECK: #0 0x{{.*}} in {{.*}}free
+  // CHECK: #1 0x{{.*}} in main {{.*}}atos-symbolizer.cc:[[@LINE-8]]
+  // CHECK: allocated by thread T0 here:
+  // CHECK: atos-symbolizer.cc:[[@LINE-13]]
+  return res;
+}

test/asan/TestCases/Darwin/dyld-root-path.cc

+// Check that when having a DYLD_ROOT_PATH set, the symbolizer still works.
+// RUN: %clangxx_asan -O0 %s -o %t -framework Foundation
+// RUN: DYLD_ROOT_PATH="/" ASAN_OPTIONS=verbosity=2 ASAN_SYMBOLIZER_PATH=$(which atos) \
+// RUN:   not %run %t 2>&1 | FileCheck %s
+
+#include <stdlib.h>
+
+// CHECK: Using atos at path:
+
+int main() {
+  char *x = (char*)malloc(10 * sizeof(char));
+  free(x);
+  return x[5];
+  // CHECK: {{AddressSanitizer: heap-use-after-free on address}}
+  // CHECK: {{READ of size 1 at 0x.* thread T0}}
+  // CHECK-NOT: atos returned an error
+  // CHECK: {{#0 0x.* in main .*dyld-root-path.cc:}}[[@LINE-4]]
+}

test/asan/TestCases/Darwin/sandbox-symbolizer.cc

+// In a non-forking sandbox, we can't spawn an external symbolizer, but dladdr()
+// should still work and provide function names. No line numbers though.
+
+// RUN: %clangxx_asan -O0 %s -o %t 
+// RUN: not %run sandbox-exec -p '(version 1)(allow default)(deny process-fork)' %t 2>&1 | FileCheck %s
+// RUN: %clangxx_asan -O3 %s -o %t 
+// RUN: not %run sandbox-exec -p '(version 1)(allow default)(deny process-fork)' %t 2>&1 | FileCheck %s
+
+// `atos` symbolizer can't inspect a process that has an inaccessible task port,
+// in which case we should again fallback to dladdr gracefully.
+
+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: not %run sandbox-exec -p '(version 1)(allow default)(deny mach-priv-task-port)' %t 2>&1 | FileCheck %s
+// RUN: %clangxx_asan -O3 %s -o %t
+// RUN: not %run sandbox-exec -p '(version 1)(allow default)(deny mach-priv-task-port)' %t 2>&1 | FileCheck %s
+
+#include <stdlib.h>
+int main() {
+  char *x = (char*)malloc(10 * sizeof(char));
+  free(x);
+  return x[5];
+  // CHECK: {{.*ERROR: AddressSanitizer: heap-use-after-free on address}}
+  // CHECK: {{READ of size 1 at 0x.* thread T0}}
+  // CHECK: {{    #0 0x.* in main}}
+  // CHECK: {{freed by thread T0 here:}}
+  // CHECK: {{    #0 0x.* in wrap_free}}
+  // CHECK: {{    #1 0x.* in main}}
+  // CHECK: {{previously allocated by thread T0 here:}}
+  // CHECK: {{    #0 0x.* in wrap_malloc}}
+  // CHECK: {{    #1 0x.* in main}}
+}

test/asan/TestCases/Darwin/suppressions-sandbox.cc

+// Check that without suppressions, we catch the issue.
+// RUN: %clangxx_asan -O0 %s -o %t -framework Foundation
+// RUN: not %run %t 2>&1 | FileCheck --check-prefix=CHECK-CRASH %s
+
+// Check that suppressing a function name works within a no-fork sandbox
+// RUN: echo "interceptor_via_fun:CFStringCreateWithBytes" > %t.supp
+// RUN: ASAN_OPTIONS=suppressions=%t.supp \
+// RUN:   sandbox-exec -p '(version 1)(allow default)(deny process-fork)' \
+// RUN:   %run %t 2>&1 | FileCheck --check-prefix=CHECK-IGNORE %s
+
+#include <CoreFoundation/CoreFoundation.h>
+
+int main() {
+  char *a = (char *)malloc(6);
+  strcpy(a, "hello");
+  CFStringRef str =
+      CFStringCreateWithBytes(kCFAllocatorDefault, (unsigned char *)a, 10,
+                              kCFStringEncodingUTF8, FALSE);  // BOOM
+  fprintf(stderr, "Ignored.\n");
+  free(a);
+}
+
+// CHECK-CRASH: AddressSanitizer: heap-buffer-overflow
+// CHECK-CRASH-NOT: Ignored.
+// CHECK-IGNORE-NOT: AddressSanitizer: heap-buffer-overflow
+// CHECK-IGNORE: Ignored.