[llvm-branch-commits] [compiler-rt] 8678afc - [Symbolizers] On Darwin compute function offset when possible.

[Dan Liew via llvm-branch-commits]

Author: Dan Liew
Date: 2019-10-28T23:41:24-07:00
New Revision: 8678afce2c8cb2b392a02459350023093ab7eb17

URL: https://github.com/llvm/llvm-project/commit/8678afce2c8cb2b392a02459350023093ab7eb17
DIFF: https://github.com/llvm/llvm-project/commit/8678afce2c8cb2b392a02459350023093ab7eb17.diff

LOG: [Symbolizers] On Darwin compute function offset when possible.

Summary:
The sanitizer symbolizers support printing the function offset
(difference between pc and function start) of a stackframe using the
`%q` format specifier.

Unfortunately this didn't actually work because neither the atos
or dladdr symbolizer set the `AddressInfo::function_offset` field.

This patch teaches both symbolizers to try to compute the function
offset. In the case of the atos symbolizer, atos might not report the
function offset (e.g. it reports a source location instead) so in this
case it fallsback to using `dladdr()` to compute the function offset.

Two test cases are included.

rdar://problem/56695185

Reviewers: kubamracek, yln

Subscribers: #sanitizers, llvm-commits

Tags: #sanitizers, #llvm

Differential Revision: https://reviews.llvm.org/D69549

Added: 
    compiler-rt/test/sanitizer_common/TestCases/Darwin/symbolizer-function-offset-atos.cpp
    compiler-rt/test/sanitizer_common/TestCases/Darwin/symbolizer-function-offset-dladdr.cpp

Modified: 
    compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp
index a619ed092f0b..2e06a513e714 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp
@@ -31,6 +31,9 @@ bool DlAddrSymbolizer::SymbolizePC(uptr addr, SymbolizedStack *stack) {
   Dl_info info;
   int result = dladdr((const void *)addr, &info);
   if (!result) return false;
+
+  CHECK(addr >= reinterpret_cast<uptr>(info.dli_saddr));
+  stack->info.function_offset = addr - reinterpret_cast<uptr>(info.dli_saddr);
   const char *demangled = DemangleSwiftAndCXX(info.dli_sname);
   if (!demangled) return false;
   stack->info.function = internal_strdup(demangled);
@@ -145,12 +148,29 @@ bool AtosSymbolizer::SymbolizePC(uptr addr, SymbolizedStack *stack) {
   const char *buf = process_->SendCommand(command);
   if (!buf) return false;
   uptr line;
+  uptr start_address = AddressInfo::kUnknown;
   if (!ParseCommandOutput(buf, addr, &stack->info.function, &stack->info.module,
-                          &stack->info.file, &line, nullptr)) {
+                          &stack->info.file, &line, &start_address)) {
     process_ = nullptr;
     return false;
   }
   stack->info.line = (int)line;
+
+  // Compute the function offset.
+  uptr function_offset = AddressInfo::kUnknown;
+  if (start_address != AddressInfo::kUnknown) {
+    CHECK(addr >= start_address);
+    function_offset = addr - start_address;
+  } else {
+    // Fallback to dladdr() to get function offset if atos doesn't report it.
+    Dl_info info;
+    int result = dladdr((const void *)addr, &info);
+    if (result) {
+      CHECK(addr >= reinterpret_cast<uptr>(info.dli_saddr));
+      function_offset = addr - reinterpret_cast<uptr>(info.dli_saddr);
+    }
+  }
+  stack->info.function_offset = function_offset;
   return true;
 }
 

diff  --git a/compiler-rt/test/sanitizer_common/TestCases/Darwin/symbolizer-function-offset-atos.cpp b/compiler-rt/test/sanitizer_common/TestCases/Darwin/symbolizer-function-offset-atos.cpp
new file mode 100644
index 000000000000..d7685c727f33
--- /dev/null
+++ b/compiler-rt/test/sanitizer_common/TestCases/Darwin/symbolizer-function-offset-atos.cpp
@@ -0,0 +1,47 @@
+// The no-debug case should cause atos to report the function offset so this should test that path.
+// RUN: rm -rf %t-no-debug.dSYM
+// RUN: %clangxx %s -g0 -O0 -o %t-no-debug
+// RUN: %env_tool_opts=verbosity=2,stack_trace_format='"function_name:%f function_offset:%q"' %run %t-no-debug > %t-no-debug.output 2>&1
+// RUN: FileCheck -input-file=%t-no-debug.output %s
+// RUN: FileCheck -check-prefix=BADADDR -input-file=%t-no-debug.output %s
+
+// The debug info case should cause atos to not report the function offset so this should test the dladdr() fallback path.
+// RUN: %clangxx %s -g -O0 -o %t-with-debug
+// RUN: %env_tool_opts=verbosity=2,stack_trace_format='"function_name:%f function_offset:%q"' %run %t-with-debug > %t-with-debug.output 2>&1
+// RUN: FileCheck -input-file=%t-with-debug.output %s
+// RUN: FileCheck -check-prefix=BADADDR -input-file=%t-with-debug.output %s
+#include <sanitizer/common_interface_defs.h>
+#include <stdio.h>
+
+void baz() {
+  printf("Do stuff in baz\n");
+  __sanitizer_print_stack_trace();
+}
+
+void bar() {
+  printf("Do stuff in bar\n");
+  baz();
+}
+
+void foo() {
+  printf("Do stuff in foo\n");
+  bar();
+}
+
+int main() {
+  printf("Do stuff in main\n");
+  foo();
+  return 0;
+}
+
+// CHECK: Using atos found at:
+
+// CHECK: function_name:baz{{(\(\))?}} function_offset:0x{{[0-9a-f]+}}
+// CHECK: function_name:bar{{(\(\))?}} function_offset:0x{{[0-9a-f]+}}
+// CHECK: function_name:foo{{(\(\))?}} function_offset:0x{{[0-9a-f]+}}
+// CHECK: function_name:main{{(\(\))?}} function_offset:0x{{[0-9a-f]+}}
+
+// BADADDR-NOT: function_name:baz{{(\(\))?}} function_offset:0x0
+// BADADDR-NOT: function_name:bar{{(\(\))?}} function_offset:0x0
+// BADADDR-NOT: function_name:foo{{(\(\))?}} function_offset:0x0
+// BADADDR-NOT: function_name:main{{(\(\))?}} function_offset:0x0

diff  --git a/compiler-rt/test/sanitizer_common/TestCases/Darwin/symbolizer-function-offset-dladdr.cpp b/compiler-rt/test/sanitizer_common/TestCases/Darwin/symbolizer-function-offset-dladdr.cpp
new file mode 100644
index 000000000000..b629adc676b6
--- /dev/null
+++ b/compiler-rt/test/sanitizer_common/TestCases/Darwin/symbolizer-function-offset-dladdr.cpp
@@ -0,0 +1,45 @@
+// UNSUPPORTED: lsan
+// This test fails with LSan enabled because the dladdr symbolizer actually leaks
+// memory because the call to `__sanitizer::DemangleCXXABI` leaks memory which LSan
+// detects (rdar://problem/42868950).
+
+// RUN: %clangxx %s -O0 -o %t
+// RUN: %env_tool_opts=verbosity=2,external_symbolizer_path=,stack_trace_format='"function_name:%f function_offset:%q"' %run %t > %t.output 2>&1
+// RUN: FileCheck -input-file=%t.output %s
+// RUN: FileCheck -check-prefix=BADADDR -input-file=%t.output %s
+#include <sanitizer/common_interface_defs.h>
+#include <stdio.h>
+
+void baz() {
+  printf("Do stuff in baz\n");
+  __sanitizer_print_stack_trace();
+}
+
+void bar() {
+  printf("Do stuff in bar\n");
+  baz();
+}
+
+void foo() {
+  printf("Do stuff in foo\n");
+  bar();
+}
+
+int main() {
+  printf("Do stuff in main\n");
+  foo();
+  return 0;
+}
+
+// CHECK: External symbolizer is explicitly disabled
+// CHECK: Using dladdr symbolizer
+
+// CHECK: function_name:baz{{(\(\))?}} function_offset:0x{{[0-9a-f]+}}
+// CHECK: function_name:bar{{(\(\))?}} function_offset:0x{{[0-9a-f]+}}
+// CHECK: function_name:foo{{(\(\))?}} function_offset:0x{{[0-9a-f]+}}
+// CHECK: function_name:main{{(\(\))?}} function_offset:0x{{[0-9a-f]+}}
+
+// BADADDR-NOT: function_name:baz{{(\(\))?}} function_offset:0x0
+// BADADDR-NOT: function_name:bar{{(\(\))?}} function_offset:0x0
+// BADADDR-NOT: function_name:foo{{(\(\))?}} function_offset:0x0
+// BADADDR-NOT: function_name:main{{(\(\))?}} function_offset:0x0