From a0fbdc01b2795b34a5192708d322d2b4c165b019 Mon Sep 17 00:00:00 2001 From: Ellis Hoag Date: Mon, 15 Sep 2025 09:54:54 -0700 Subject: [PATCH 1/4] [Sanitizer] Option to fallback to stderr if unable to open logfile --- .../lib/sanitizer_common/sanitizer_file.cpp | 37 +++++++++++++++---- .../lib/sanitizer_common/sanitizer_file.h | 3 ++ .../lib/sanitizer_common/sanitizer_flags.inc | 2 + .../Posix/sanitizer_set_report_path_fail.cpp | 3 ++ 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_file.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_file.cpp index 9236a458cdb0e..4e691d0768c52 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_file.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_file.cpp @@ -36,9 +36,16 @@ void RawWrite(const char *buffer) { void ReportFile::ReopenIfNecessary() { mu->CheckLocked(); - if (fd == kStdoutFd || fd == kStderrFd) return; + if (!lastOpenFailed) + if (fd == kStdoutFd || fd == kStderrFd) + return; uptr pid = internal_getpid(); + if (lastOpenFailed && fd_pid != pid) { + fd = kInvalidFd; + lastOpenFailed = false; + } + // If in tracer, use the parent's file. if (pid == stoptheworld_tracer_pid) pid = stoptheworld_tracer_ppid; @@ -48,8 +55,7 @@ void ReportFile::ReopenIfNecessary() { // process, close it now. if (fd_pid == pid) return; - else - CloseFile(fd); + CloseFile(fd); } const char *exe_name = GetProcessName(); @@ -71,12 +77,17 @@ void ReportFile::ReopenIfNecessary() { char errmsg[100]; internal_snprintf(errmsg, sizeof(errmsg), " (reason: %d)\n", err); WriteToFile(kStderrFd, errmsg, internal_strlen(errmsg)); - Die(); + if (!common_flags()->log_fallback_to_stderr) + Die(); + const char *errmsg2 = "ERROR: Fallback to stderr\n"; + WriteToFile(kStderrFd, errmsg2, internal_strlen(errmsg2)); + lastOpenFailed = true; + fd = kStderrFd; } fd_pid = pid; } -static void RecursiveCreateParentDirs(char *path) { +static void RecursiveCreateParentDirs(char *path, fd_t &fd) { if (path[0] == '\0') return; for (int i = 1; path[i] != '\0'; ++i) { @@ -90,7 +101,13 @@ static void RecursiveCreateParentDirs(char *path) { WriteToFile(kStderrFd, path, internal_strlen(path)); const char *ErrorMsgSuffix = "\n"; WriteToFile(kStderrFd, ErrorMsgSuffix, internal_strlen(ErrorMsgSuffix)); - Die(); + if (!common_flags()->log_fallback_to_stderr) + Die(); + path[i] = save; + const char *ErrorMsgSuffix2 = "ERROR: Fallback to stderr\n"; + WriteToFile(kStderrFd, ErrorMsgSuffix2, internal_strlen(ErrorMsgSuffix2)); + fd = kStderrFd; + return; } path[i] = save; } @@ -166,7 +183,11 @@ void ReportFile::SetReportPath(const char *path) { WriteToFile(kStderrFd, path, 8); message = "...\n"; WriteToFile(kStderrFd, message, internal_strlen(message)); - Die(); + if (!common_flags()->log_fallback_to_stderr) + Die(); + const char *message2 = "ERROR: Fallback to stderr\n"; + WriteToFile(kStderrFd, message2, internal_strlen(message2)); + path = "stderr"; } } @@ -180,7 +201,7 @@ void ReportFile::SetReportPath(const char *path) { fd = kStdoutFd; } else { ParseAndSetPath(path, path_prefix, kMaxPathLength); - RecursiveCreateParentDirs(path_prefix); + RecursiveCreateParentDirs(path_prefix, fd); } } diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_file.h b/compiler-rt/lib/sanitizer_common/sanitizer_file.h index bef2c842d9f24..f1a791e2aa3b1 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_file.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_file.h @@ -43,6 +43,9 @@ struct ReportFile { // PID of the process that opened fd. If a fork() occurs, // the PID of child will be different from fd_pid. uptr fd_pid; + // Set to true if the last attempt to open the logfile failed, perhaps due to + // permission errors + bool lastOpenFailed = false; private: void ReopenIfNecessary(); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc b/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc index c1e3530618c20..650a4580bbcf0 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc +++ b/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc @@ -65,6 +65,8 @@ COMMON_FLAG( bool, log_to_syslog, (bool)SANITIZER_ANDROID || (bool)SANITIZER_APPLE, "Write all sanitizer output to syslog in addition to other means of " "logging.") +COMMON_FLAG(bool, log_fallback_to_stderr, false, + "When set, fallback to stderr if we are unable to open log path.") COMMON_FLAG( int, verbosity, 0, "Verbosity level (0 - silent, 1 - a bit of output, 2+ - more output).") diff --git a/compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_fail.cpp b/compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_fail.cpp index af5187a0d3265..24adaaa4046c0 100644 --- a/compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_fail.cpp +++ b/compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_fail.cpp @@ -2,9 +2,11 @@ // Case 1: Try setting a path that is an invalid/inaccessible directory. // RUN: not %run %t 2>&1 | FileCheck %s --check-prefix=ERROR1 +// RUN: %env_tool_opts=log_fallback_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefixes=ERROR1,FALLBACK // Case 2: Try setting a path that is too large. // RUN: not %run %t A 2>&1 | FileCheck %s --check-prefix=ERROR2 +// RUN: %env_tool_opts=log_fallback_to_stderr=1 %run %t A 2>&1 | FileCheck %s --check-prefixes=ERROR2,FALLBACK #include #include @@ -22,3 +24,4 @@ int main(int argc, char **argv) { } __sanitizer_set_report_path(buff); } +// FALLBACK: Fallback to stderr From d94674c0cf6f620b76af77d5ed08be1bd6cdceb1 Mon Sep 17 00:00:00 2001 From: Ellis Hoag Date: Mon, 15 Sep 2025 10:23:13 -0700 Subject: [PATCH 2/4] use flag value true --- .../TestCases/Posix/sanitizer_set_report_path_fail.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_fail.cpp b/compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_fail.cpp index 24adaaa4046c0..197e95d2e9ddf 100644 --- a/compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_fail.cpp +++ b/compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_fail.cpp @@ -2,11 +2,11 @@ // Case 1: Try setting a path that is an invalid/inaccessible directory. // RUN: not %run %t 2>&1 | FileCheck %s --check-prefix=ERROR1 -// RUN: %env_tool_opts=log_fallback_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefixes=ERROR1,FALLBACK +// RUN: %env_tool_opts=log_fallback_to_stderr=true %run %t 2>&1 | FileCheck %s --check-prefixes=ERROR1,FALLBACK // Case 2: Try setting a path that is too large. // RUN: not %run %t A 2>&1 | FileCheck %s --check-prefix=ERROR2 -// RUN: %env_tool_opts=log_fallback_to_stderr=1 %run %t A 2>&1 | FileCheck %s --check-prefixes=ERROR2,FALLBACK +// RUN: %env_tool_opts=log_fallback_to_stderr=true %run %t A 2>&1 | FileCheck %s --check-prefixes=ERROR2,FALLBACK #include #include From 0c3b9775faee043499ef16442f1d092e7b4f5ea7 Mon Sep 17 00:00:00 2001 From: Ellis Hoag Date: Tue, 16 Sep 2025 14:47:55 -0700 Subject: [PATCH 3/4] Add comment and check lastOpenFailed first --- compiler-rt/lib/sanitizer_common/sanitizer_file.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_file.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_file.cpp index 4e691d0768c52..cd8d57a1b7d45 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_file.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_file.cpp @@ -36,15 +36,16 @@ void RawWrite(const char *buffer) { void ReportFile::ReopenIfNecessary() { mu->CheckLocked(); - if (!lastOpenFailed) - if (fd == kStdoutFd || fd == kStderrFd) - return; - uptr pid = internal_getpid(); if (lastOpenFailed && fd_pid != pid) { + // If lastOpenFailed is set then we fellback to stderr. If this is a new + // process, mark fd as invalid so we attempt to open again. + CHECK_EQ(fd, kStderrFd); fd = kInvalidFd; lastOpenFailed = false; } + if (fd == kStdoutFd || fd == kStderrFd) + return; // If in tracer, use the parent's file. if (pid == stoptheworld_tracer_pid) From 8a8735b6306993fb9bd4963ea2ee51bc19e161cc Mon Sep 17 00:00:00 2001 From: Ellis Hoag Date: Thu, 18 Sep 2025 11:07:54 -0700 Subject: [PATCH 4/4] emit warning message --- .../lib/sanitizer_common/sanitizer_file.cpp | 37 ++++++++++--------- .../lib/sanitizer_common/sanitizer_file.h | 2 +- .../Posix/sanitizer_set_report_path_fail.cpp | 6 +-- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_file.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_file.cpp index cd8d57a1b7d45..7cfade2bc681f 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_file.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_file.cpp @@ -37,12 +37,12 @@ void RawWrite(const char *buffer) { void ReportFile::ReopenIfNecessary() { mu->CheckLocked(); uptr pid = internal_getpid(); - if (lastOpenFailed && fd_pid != pid) { - // If lastOpenFailed is set then we fellback to stderr. If this is a new - // process, mark fd as invalid so we attempt to open again. + if (fallbackToStderrActive && fd_pid != pid) { + // If fallbackToStderrActive is set then we fellback to stderr. If this is a + // new process, mark fd as invalid so we attempt to open again. CHECK_EQ(fd, kStderrFd); fd = kInvalidFd; - lastOpenFailed = false; + fallbackToStderrActive = false; } if (fd == kStdoutFd || fd == kStderrFd) return; @@ -72,17 +72,18 @@ void ReportFile::ReopenIfNecessary() { error_t err; fd = OpenFile(full_path, WrOnly, &err); if (fd == kInvalidFd) { - const char *ErrorMsgPrefix = "ERROR: Can't open file: "; + bool fallback = common_flags()->log_fallback_to_stderr; + const char *ErrorMsgPrefix = + fallback ? "WARNING: Can't open file, falling back to stderr: " + : "ERROR: Can't open file: "; WriteToFile(kStderrFd, ErrorMsgPrefix, internal_strlen(ErrorMsgPrefix)); WriteToFile(kStderrFd, full_path, internal_strlen(full_path)); char errmsg[100]; internal_snprintf(errmsg, sizeof(errmsg), " (reason: %d)\n", err); WriteToFile(kStderrFd, errmsg, internal_strlen(errmsg)); - if (!common_flags()->log_fallback_to_stderr) + if (!fallback) Die(); - const char *errmsg2 = "ERROR: Fallback to stderr\n"; - WriteToFile(kStderrFd, errmsg2, internal_strlen(errmsg2)); - lastOpenFailed = true; + fallbackToStderrActive = true; fd = kStderrFd; } fd_pid = pid; @@ -97,16 +98,17 @@ static void RecursiveCreateParentDirs(char *path, fd_t &fd) { continue; path[i] = '\0'; if (!DirExists(path) && !CreateDir(path)) { - const char *ErrorMsgPrefix = "ERROR: Can't create directory: "; + bool fallback = common_flags()->log_fallback_to_stderr; + const char *ErrorMsgPrefix = + fallback ? "WARNING: Can't create directory, falling back to stderr: " + : "ERROR: Can't create directory: "; WriteToFile(kStderrFd, ErrorMsgPrefix, internal_strlen(ErrorMsgPrefix)); WriteToFile(kStderrFd, path, internal_strlen(path)); const char *ErrorMsgSuffix = "\n"; WriteToFile(kStderrFd, ErrorMsgSuffix, internal_strlen(ErrorMsgSuffix)); - if (!common_flags()->log_fallback_to_stderr) + if (!fallback) Die(); path[i] = save; - const char *ErrorMsgSuffix2 = "ERROR: Fallback to stderr\n"; - WriteToFile(kStderrFd, ErrorMsgSuffix2, internal_strlen(ErrorMsgSuffix2)); fd = kStderrFd; return; } @@ -179,15 +181,16 @@ void ReportFile::SetReportPath(const char *path) { if (path) { uptr len = internal_strlen(path); if (len > sizeof(path_prefix) - 100) { - const char *message = "ERROR: Path is too long: "; + bool fallback = common_flags()->log_fallback_to_stderr; + const char *message = + fallback ? "WARNING: Path is too long, falling back to stderr: " + : "ERROR: Path is too long: "; WriteToFile(kStderrFd, message, internal_strlen(message)); WriteToFile(kStderrFd, path, 8); message = "...\n"; WriteToFile(kStderrFd, message, internal_strlen(message)); - if (!common_flags()->log_fallback_to_stderr) + if (!fallback) Die(); - const char *message2 = "ERROR: Fallback to stderr\n"; - WriteToFile(kStderrFd, message2, internal_strlen(message2)); path = "stderr"; } } diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_file.h b/compiler-rt/lib/sanitizer_common/sanitizer_file.h index f1a791e2aa3b1..b3a5fed922da9 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_file.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_file.h @@ -45,7 +45,7 @@ struct ReportFile { uptr fd_pid; // Set to true if the last attempt to open the logfile failed, perhaps due to // permission errors - bool lastOpenFailed = false; + bool fallbackToStderrActive = false; private: void ReopenIfNecessary(); diff --git a/compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_fail.cpp b/compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_fail.cpp index 197e95d2e9ddf..819678e956d37 100644 --- a/compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_fail.cpp +++ b/compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_fail.cpp @@ -16,12 +16,12 @@ int main(int argc, char **argv) { if (argc == 1) { // Case 1 sprintf(buff, "%s/report", argv[0]); - // ERROR1: Can't create directory: {{.*}} + // ERROR1: Can't create directory } else { // Case 2 snprintf(buff, sizeof(buff), "%04095d", 42); - // ERROR2: Path is too long: 00000000... + // ERROR2: Path is too long } __sanitizer_set_report_path(buff); } -// FALLBACK: Fallback to stderr +// FALLBACK: falling back to stderr