[PATCH] [sanitizer] Fix log_path behavior with StopTheWorld.
Sergey Matveev
earthdok at google.com
Mon Dec 2 07:18:51 PST 2013
Hi samsonov,
Fix race on report_fd/report_fd_pid between the parent process and the
tracer task.
http://llvm-reviews.chandlerc.com/D2306
Files:
lib/sanitizer_common/sanitizer_common.cc
lib/sanitizer_common/sanitizer_common.h
lib/sanitizer_common/sanitizer_posix.cc
lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
Index: lib/sanitizer_common/sanitizer_common.cc
===================================================================
--- lib/sanitizer_common/sanitizer_common.cc
+++ lib/sanitizer_common/sanitizer_common.cc
@@ -42,6 +42,12 @@
// child thread will be different from |report_fd_pid|.
uptr report_fd_pid = 0;
+// Alternative report FD and PID for use by the tracer task in StopTheWorld. It
+// shares the address space with the main process, but has a different PID and
+// thus requires special handling.
+fd_t tracer_report_fd = kStderrFd;
+uptr tracer_report_fd_pid = 0;
+
static DieCallbackType DieCallback;
void SetDieCallback(DieCallbackType callback) {
DieCallback = callback;
Index: lib/sanitizer_common/sanitizer_common.h
===================================================================
--- lib/sanitizer_common/sanitizer_common.h
+++ lib/sanitizer_common/sanitizer_common.h
@@ -136,6 +136,8 @@
extern bool log_to_file;
extern char report_path_prefix[4096];
extern uptr report_fd_pid;
+extern fd_t tracer_report_fd;
+extern uptr tracer_report_fd_pid;
uptr OpenFile(const char *filename, bool write);
// Opens the file 'file_name" and reads up to 'max_len' bytes.
Index: lib/sanitizer_common/sanitizer_posix.cc
===================================================================
--- lib/sanitizer_common/sanitizer_posix.cc
+++ lib/sanitizer_common/sanitizer_posix.cc
@@ -197,33 +197,53 @@
return 0;
}
-void MaybeOpenReportFile() {
- if (!log_to_file || (report_fd_pid == internal_getpid())) return;
+static uptr DoOpenReportFile(uptr pid) {
InternalScopedBuffer<char> report_path_full(4096);
- internal_snprintf(report_path_full.data(), report_path_full.size(),
- "%s.%d", report_path_prefix, internal_getpid());
+ internal_snprintf(report_path_full.data(), report_path_full.size(), "%s.%d",
+ report_path_prefix, pid);
uptr openrv = OpenFile(report_path_full.data(), true);
if (internal_iserror(openrv)) {
- report_fd = kStderrFd;
+ if (pid == tracer_report_fd_pid)
+ tracer_report_fd = kStderrFd;
+ else
+ report_fd = kStderrFd;
log_to_file = false;
Report("ERROR: Can't open file: %s\n", report_path_full.data());
Die();
}
- if (report_fd != kInvalidFd) {
- // We're in the child. Close the parent's log.
- internal_close(report_fd);
- }
- report_fd = openrv;
- report_fd_pid = internal_getpid();
+ return openrv;
}
+void MaybeOpenReportFile() {
+ if (!log_to_file) return;
+ uptr pid = internal_getpid();
+ bool in_tracer = (pid == tracer_report_fd_pid);
+ if (in_tracer) {
+ if (tracer_report_fd != kInvalidFd)
+ return;
+ uptr openrv = DoOpenReportFile(pid);
+ tracer_report_fd = openrv;
+ } else {
+ if (pid == report_fd_pid)
+ return;
+ uptr openrv = DoOpenReportFile(pid);
+ if (report_fd != kInvalidFd) {
+ // We're in the child. Close the parent's log.
+ internal_close(report_fd);
+ }
+ report_fd = openrv;
+ report_fd_pid = pid;
+ }
+}
void RawWrite(const char *buffer) {
static const char *kRawWriteError =
"RawWrite can't output requested buffer!\n";
uptr length = (uptr)internal_strlen(buffer);
MaybeOpenReportFile();
- if (length != internal_write(report_fd, buffer, length)) {
- internal_write(report_fd, kRawWriteError, internal_strlen(kRawWriteError));
+ uptr fd = (internal_getpid() == tracer_report_fd_pid) ? tracer_report_fd
+ : report_fd;
+ if (length != internal_write(fd, buffer, length)) {
+ internal_write(fd, kRawWriteError, internal_strlen(kRawWriteError));
Die();
}
}
Index: lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
===================================================================
--- lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
+++ lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
@@ -356,6 +356,26 @@
int process_was_dumpable_;
};
+// When sanitizer output is being redirected to file (i.e. by using log_path) we
+// must prevent the tracer task from conflicting with the parent over file
+// descriptors. This scoper manipulates some sanitizer_common globals to achieve
+// that.
+struct ScopedHandleReportFD {
+ explicit ScopedHandleReportFD(uptr tracer_pid) {
+ tracer_report_fd = kInvalidFd;
+ tracer_report_fd_pid = tracer_pid;
+ if (report_fd == kStderrFd || report_fd == kStdoutFd)
+ tracer_report_fd = report_fd;
+ }
+ ~ScopedHandleReportFD() {
+ if (tracer_report_fd != kStderrFd && tracer_report_fd != kStdoutFd &&
+ tracer_report_fd != kInvalidFd)
+ internal_close(tracer_report_fd);
+ tracer_report_fd = kInvalidFd;
+ tracer_report_fd_pid = 0;
+ }
+};
+
void StopTheWorld(StopTheWorldCallback callback, void *argument) {
StopTheWorldScope in_stoptheworld;
// Prepare the arguments for TracerThread.
@@ -379,6 +399,7 @@
Report("Failed spawning a tracer thread (errno %d).\n", local_errno);
tracer_thread_argument.mutex.Unlock();
} else {
+ ScopedHandleReportFD scoped_handle_report_fd(tracer_pid);
// On some systems we have to explicitly declare that we want to be traced
// by the tracer thread.
#ifdef PR_SET_PTRACER
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2306.1.patch
Type: text/x-patch
Size: 5270 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131202/29140d27/attachment.bin>
More information about the llvm-commits
mailing list