From 7bc300c8fc4d6e1e49cb79af2109a1c05b922772 Mon Sep 17 00:00:00 2001 From: Sergey Matveev Date: Wed, 4 Dec 2013 14:37:01 +0000 Subject: [PATCH] [sanitizer] Fix log_path behavior with StopTheWorld. Summary: Fix race on report_fd/report_fd_pid between the parent process and the tracer task. Reviewers: samsonov Reviewed By: samsonov CC: llvm-commits, kcc, dvyukov Differential Revision: http://llvm-reviews.chandlerc.com/D2306 llvm-svn: 196385 --- .../lib/sanitizer_common/sanitizer_common.cc | 7 +++++++ .../lib/sanitizer_common/sanitizer_common.h | 2 ++ .../lib/sanitizer_common/sanitizer_posix.cc | 11 ++++++++--- .../sanitizer_stoptheworld_linux_libcdep.cc | 15 +++++++++++++++ 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.cc b/compiler-rt/lib/sanitizer_common/sanitizer_common.cc index 7e870ff65455..c156989d9bcd 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_common.cc +++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.cc @@ -42,6 +42,13 @@ char report_path_prefix[sizeof(report_path_prefix)]; // child thread will be different from |report_fd_pid|. uptr report_fd_pid = 0; +// PID of the tracer task in StopTheWorld. It shares the address space with the +// main process, but has a different PID and thus requires special handling. +uptr stoptheworld_tracer_pid = 0; +// Cached pid of parent process - if the parent process dies, we want to keep +// writing to the same log file. +uptr stoptheworld_tracer_ppid = 0; + static DieCallbackType DieCallback; void SetDieCallback(DieCallbackType callback) { DieCallback = callback; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h index d29f68cedf9b..ab67be21c05e 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h @@ -136,6 +136,8 @@ extern fd_t report_fd; extern bool log_to_file; extern char report_path_prefix[4096]; extern uptr report_fd_pid; +extern uptr stoptheworld_tracer_pid; +extern uptr stoptheworld_tracer_ppid; uptr OpenFile(const char *filename, bool write); // Opens the file 'file_name" and reads up to 'max_len' bytes. diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix.cc b/compiler-rt/lib/sanitizer_common/sanitizer_posix.cc index 5438aca94688..4d2c033c5abd 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_posix.cc +++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix.cc @@ -198,10 +198,15 @@ char *FindPathToBinary(const char *name) { } void MaybeOpenReportFile() { - if (!log_to_file || (report_fd_pid == internal_getpid())) return; + if (!log_to_file) return; + uptr pid = internal_getpid(); + // If in tracer, use the parent's file. + if (pid == stoptheworld_tracer_pid) + pid = stoptheworld_tracer_ppid; + if (report_fd_pid == pid) return; InternalScopedBuffer report_path_full(4096); internal_snprintf(report_path_full.data(), report_path_full.size(), - "%s.%d", report_path_prefix, internal_getpid()); + "%s.%d", report_path_prefix, pid); uptr openrv = OpenFile(report_path_full.data(), true); if (internal_iserror(openrv)) { report_fd = kStderrFd; @@ -214,7 +219,7 @@ void MaybeOpenReportFile() { internal_close(report_fd); } report_fd = openrv; - report_fd_pid = internal_getpid(); + report_fd_pid = pid; } void RawWrite(const char *buffer) { diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc b/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc index a34ddd872209..8a0ffd434e62 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc +++ b/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc @@ -356,6 +356,20 @@ class StopTheWorldScope { int process_was_dumpable_; }; +// When sanitizer output is being redirected to file (i.e. by using log_path), +// the tracer should write to the parent's log instead of trying to open a new +// file. Alert the logging code to the fact that we have a tracer. +struct ScopedSetTracerPID { + explicit ScopedSetTracerPID(uptr tracer_pid) { + stoptheworld_tracer_pid = tracer_pid; + stoptheworld_tracer_ppid = internal_getpid(); + } + ~ScopedSetTracerPID() { + stoptheworld_tracer_pid = 0; + stoptheworld_tracer_ppid = 0; + } +}; + void StopTheWorld(StopTheWorldCallback callback, void *argument) { StopTheWorldScope in_stoptheworld; // Prepare the arguments for TracerThread. @@ -379,6 +393,7 @@ void StopTheWorld(StopTheWorldCallback callback, void *argument) { Report("Failed spawning a tracer thread (errno %d).\n", local_errno); tracer_thread_argument.mutex.Unlock(); } else { + ScopedSetTracerPID scoped_set_tracer_pid(tracer_pid); // On some systems we have to explicitly declare that we want to be traced // by the tracer thread. #ifdef PR_SET_PTRACER