Skip to content

Commit 923cf89

Browse files
committed
Avoid failing a CHECK in DlAddrSymbolizer::SymbolizePC.
Summary: It turns out the `CHECK(addr >= reinterpret_cast<upt>(info.dli_saddr)` can fail because on armv7s on iOS 9.3 `dladdr()` returns `info.dli_saddr` with an address larger than the address we provided. We should avoid crashing here because crashing in the middle of reporting an issue is very unhelpful. Instead we now try to compute a function offset if the value we get back from `dladdr()` looks sane, otherwise we don't set the function offset. A test case is included. It's basically a slightly modified version of the existing `test/sanitizer_common/TestCases/Darwin/symbolizer-function-offset-dladdr.cpp` test case that doesn't run on iOS devices right now. More details: In the concrete scenario on armv7s `addr` is `0x2195c870` and the returned `info.dli_saddr` is `0x2195c871`. This what LLDB says when disassembling the code. ``` (lldb) dis -a 0x2195c870 libdyld.dylib`<redacted>: 0x2195c870 <+0>: nop 0x2195c872 <+2>: blx 0x2195c91c ; symbol stub for: exit 0x2195c876 <+6>: trap ``` The value returned by `dladdr()` doesn't make sense because it points into the middle of a instruction. There might also be other bugs lurking here because I noticed that the PCs we gather during stackunwinding (before changing them with `StackTrace::GetPreviousInstructionPc()`) look a little suspicious (e.g. the PC stored for the frame with fail to symbolicate is 0x2195c873) as they don't look properly aligned. This probably warrants further investigation in the future. rdar://problem/65621511 Reviewers: kubamracek, yln Subscribers: kristof.beyls, llvm-commits, #sanitizers Tags: #sanitizers Differential Revision: https://reviews.llvm.org/D84262
1 parent 13bfe4b commit 923cf89

File tree

2 files changed

+57
-6
lines changed

2 files changed

+57
-6
lines changed

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,15 @@ bool DlAddrSymbolizer::SymbolizePC(uptr addr, SymbolizedStack *stack) {
3333
int result = dladdr((const void *)addr, &info);
3434
if (!result) return false;
3535

36-
CHECK(addr >= reinterpret_cast<uptr>(info.dli_saddr));
37-
stack->info.function_offset = addr - reinterpret_cast<uptr>(info.dli_saddr);
36+
// Compute offset if possible. `dladdr()` doesn't always ensure that `addr >=
37+
// sym_addr` so only compute the offset when this holds. Failure to find the
38+
// function offset is not treated as a failure because it might still be
39+
// possible to get the symbol name.
40+
uptr sym_addr = reinterpret_cast<uptr>(info.dli_saddr);
41+
if (addr >= sym_addr) {
42+
stack->info.function_offset = addr - sym_addr;
43+
}
44+
3845
const char *demangled = DemangleSwiftAndCXX(info.dli_sname);
3946
if (!demangled) return false;
4047
stack->info.function = internal_strdup(demangled);
@@ -219,10 +226,10 @@ bool AtosSymbolizer::SymbolizePC(uptr addr, SymbolizedStack *stack) {
219226
start_address = reinterpret_cast<uptr>(info.dli_saddr);
220227
}
221228

222-
// Only assig to `function_offset` if we were able to get the function's
223-
// start address.
224-
if (start_address != AddressInfo::kUnknown) {
225-
CHECK(addr >= start_address);
229+
// Only assign to `function_offset` if we were able to get the function's
230+
// start address and we got a sensible `start_address` (dladdr doesn't always
231+
// ensure that `addr >= sym_addr`).
232+
if (start_address != AddressInfo::kUnknown && addr >= start_address) {
226233
stack->info.function_offset = addr - start_address;
227234
}
228235
return true;
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// FIXME(dliew): Duplicated from `test/sanitizer_common/TestCases/Darwin/symbolizer-function-offset-dladdr.cpp`.
2+
// This case can be dropped once sanitizer_common tests work on iOS devices (rdar://problem/47333049).
3+
4+
// NOTE: `detect_leaks=0` is necessary because with LSan enabled the dladdr
5+
// symbolizer actually leaks memory because the call to
6+
// `__sanitizer::DemangleCXXABI` leaks memory which LSan detects
7+
// (rdar://problem/42868950).
8+
9+
// RUN: %clangxx_asan %s -O0 -o %t
10+
// RUN: %env_asan_opts=detect_leaks=0,verbosity=2,external_symbolizer_path=,stack_trace_format='"function_name_%f___function_offset_%q"' %run %t > %t.output 2>&1
11+
// RUN: FileCheck -input-file=%t.output %s
12+
#include <sanitizer/common_interface_defs.h>
13+
#include <stdio.h>
14+
15+
void baz() {
16+
printf("Do stuff in baz\n");
17+
__sanitizer_print_stack_trace();
18+
}
19+
20+
void bar() {
21+
printf("Do stuff in bar\n");
22+
baz();
23+
}
24+
25+
void foo() {
26+
printf("Do stuff in foo\n");
27+
bar();
28+
}
29+
30+
int main() {
31+
printf("Do stuff in main\n");
32+
foo();
33+
return 0;
34+
}
35+
36+
// CHECK: External symbolizer is explicitly disabled
37+
// CHECK: Using dladdr symbolizer
38+
39+
// These `function_offset` patterns are designed to disallow `0x0` which is the
40+
// value printed for `kUnknown`.
41+
// CHECK: function_name_baz{{(\(\))?}}___function_offset_0x{{0*[1-9a-f][0-9a-f]*$}}
42+
// CHECK: function_name_bar{{(\(\))?}}___function_offset_0x{{0*[1-9a-f][0-9a-f]*$}}
43+
// CHECK: function_name_foo{{(\(\))?}}___function_offset_0x{{0*[1-9a-f][0-9a-f]*$}}
44+
// CHECK: function_name_main{{(\(\))?}}___function_offset_0x{{0*[1-9a-f][0-9a-f]*$}}

0 commit comments

Comments
 (0)