Skip to content

Commit 8efc3cc

Browse files
committed
[Darwin] Fix a bug where the symbolizer would examine the wrong process.
Summary: Previously `AtosSymbolizer` would set the PID to examine in the constructor which is called early on during sanitizer init. This can lead to incorrect behaviour in the case of a fork() because if the symbolizer is launched in the child it will be told examine the parent process rather than the child. To fix this the PID is determined just before the symbolizer is launched. A test case is included that triggers the buggy behaviour that existed prior to this patch. The test observes the PID that `atos` was called on. It also examines the symbolized stacktrace. Prior to this patch `atos` failed to symbolize the stacktrace giving output that looked like... ``` #0 0x100fc3bb5 in __sanitizer_print_stack_trace asan_stack.cpp:86 #1 0x10490dd36 in PrintStack+0x56 (/path/to/print-stack-trace-in-code-loaded-after-fork.cpp.tmp_shared_lib.dylib:x86_64+0xd36) rust-lang#2 0x100f6f986 in main+0x4a6 (/path/to/print-stack-trace-in-code-loaded-after-fork.cpp.tmp_loader:x86_64+0x100001986) rust-lang#3 0x7fff714f1cc8 in start+0x0 (/usr/lib/system/libdyld.dylib:x86_64+0x1acc8) ``` After this patch stackframes `#1` and `rust-lang#2` are fully symbolized. This patch is also a pre-requisite refactor for rdar://problem/58789439. Reviewers: kubamracek, yln Subscribers: #sanitizers, llvm-commits Tags: #sanitizers Differential Revision: https://reviews.llvm.org/D77623
1 parent 2169568 commit 8efc3cc

File tree

2 files changed

+68
-5
lines changed

2 files changed

+68
-5
lines changed

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp

+8-5
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,19 @@ bool DlAddrSymbolizer::SymbolizeData(uptr addr, DataInfo *datainfo) {
5252

5353
class AtosSymbolizerProcess : public SymbolizerProcess {
5454
public:
55-
explicit AtosSymbolizerProcess(const char *path, pid_t parent_pid)
55+
explicit AtosSymbolizerProcess(const char *path)
5656
: SymbolizerProcess(path, /*use_posix_spawn*/ true) {
57-
// Put the string command line argument in the object so that it outlives
58-
// the call to GetArgV.
59-
internal_snprintf(pid_str_, sizeof(pid_str_), "%d", parent_pid);
57+
pid_str_[0] = '\0';
6058
}
6159

6260
private:
6361
bool StartSymbolizerSubprocess() override {
6462
// Configure sandbox before starting atos process.
63+
64+
// Put the string command line argument in the object so that it outlives
65+
// the call to GetArgV.
66+
internal_snprintf(pid_str_, sizeof(pid_str_), "%d", internal_getpid());
67+
6568
return SymbolizerProcess::StartSymbolizerSubprocess();
6669
}
6770

@@ -138,7 +141,7 @@ static bool ParseCommandOutput(const char *str, uptr addr, char **out_name,
138141
}
139142

140143
AtosSymbolizer::AtosSymbolizer(const char *path, LowLevelAllocator *allocator)
141-
: process_(new(*allocator) AtosSymbolizerProcess(path, getpid())) {}
144+
: process_(new (*allocator) AtosSymbolizerProcess(path)) {}
142145

143146
bool AtosSymbolizer::SymbolizePC(uptr addr, SymbolizedStack *stack) {
144147
if (!process_) return false;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// RUN: %clangxx %s -g -DSHARED_LIB -shared -o %t_shared_lib.dylib
2+
// RUN: %clangxx %s -g -USHARED_LIB -o %t_loader
3+
// RUN: %env_tool_opts=verbosity=3 %run %t_loader %t_shared_lib.dylib > %t_loader_output.txt 2>&1
4+
// RUN: FileCheck -input-file=%t_loader_output.txt %s
5+
// RUN: FileCheck -check-prefix=CHECK-STACKTRACE -input-file=%t_loader_output.txt %s
6+
7+
#include <stdio.h>
8+
9+
#ifdef SHARED_LIB
10+
#include <sanitizer/common_interface_defs.h>
11+
12+
extern "C" void PrintStack() {
13+
fprintf(stderr, "Calling __sanitizer_print_stack_trace\n");
14+
// CHECK-STACKTRACE: #0{{( *0x.* *in *)?}} __sanitizer_print_stack_trace
15+
// CHECK-STACKTRACE: #1{{( *0x.* *in *)?}} PrintStack {{.*}}print-stack-trace-in-code-loaded-after-fork.cpp:[[@LINE+1]]
16+
__sanitizer_print_stack_trace();
17+
}
18+
#else
19+
#include <assert.h>
20+
#include <dlfcn.h>
21+
#include <stdlib.h>
22+
#include <sys/wait.h>
23+
#include <unistd.h>
24+
25+
typedef void (*PrintStackFnPtrTy)(void);
26+
27+
int main(int argc, char **argv) {
28+
assert(argc == 2);
29+
pid_t pid = fork();
30+
if (pid != 0) {
31+
// Parent
32+
pid_t parent_pid = getpid();
33+
fprintf(stderr, "parent: %d\n", parent_pid);
34+
int status = 0;
35+
pid_t child = waitpid(pid, &status, /*options=*/0);
36+
assert(pid == child);
37+
bool clean_exit = WIFEXITED(status) && WEXITSTATUS(status) == 0;
38+
return !clean_exit;
39+
}
40+
// Child.
41+
pid = getpid();
42+
// CHECK: child: [[CHILD_PID:[0-9]+]]
43+
fprintf(stderr, "child: %d\n", pid);
44+
// We load new code into the child process that isn't loaded into the parent.
45+
// When we symbolize in `PrintStack` if the symbolizer is told to symbolize
46+
// the parent (an old bug) rather than the child then symbolization will
47+
// fail.
48+
const char *library_to_load = argv[1];
49+
void *handle = dlopen(library_to_load, RTLD_NOW | RTLD_LOCAL);
50+
assert(handle);
51+
PrintStackFnPtrTy PrintStackFnPtr = (PrintStackFnPtrTy)dlsym(handle, "PrintStack");
52+
assert(PrintStackFnPtr);
53+
// Check that the symbolizer is told examine the child process.
54+
// CHECK: Launching Symbolizer process: {{.+}}atos -p [[CHILD_PID]]
55+
// CHECK-STACKTRACE: #2{{( *0x.* *in *)?}} main {{.*}}print-stack-trace-in-code-loaded-after-fork.cpp:[[@LINE+1]]
56+
PrintStackFnPtr();
57+
return 0;
58+
}
59+
60+
#endif

0 commit comments

Comments
 (0)