[sanitizer] Check if directory exists before trying to create

[tejohnson]

Summary

Add a DirExists mechanism, modeled after FileExists. Use it to guard
creation of the report path directory.

This should avoid failures running the sanitizer in a sandbox where the
file creation attempt causes hard failures, even for an existing
directory. Problem reported on D109794 for ChromeOS in sandbox
(https://issuetracker.google.com/209296420).

[vitalybuka]

it would be nice to have unittests:
TEST(SanitizerCommon, FileExists)
TEST(SanitizerCommon, DirExists)
TEST(SanitizerCommon, RecursiveCreateParentDirs)

compiler-rt/lib/sanitizer_common/sanitizer_file.cpp

87–93

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

502–513

please clang-format

compiler-rt/lib/sanitizer_common/sanitizer_win.cpp

93

hm looks like existing code is wrong

[tejohnson]

Address comments

[tejohnson]

In D119495#3313173, @vitalybuka wrote:

it would be nice to have unittests:
TEST(SanitizerCommon, FileExists)
TEST(SanitizerCommon, DirExists)
TEST(SanitizerCommon, RecursiveCreateParentDirs)

With D109794 I added TEST(SanitizerCommon, ReportFile) which does test RecursiveCreateParentDirs. Since RecursiveCreateParentDirs is a static function I can't call it directly from the unit tests.

I've added the FileExists and DirExists unit tests, however.

compiler-rt/lib/sanitizer_common/sanitizer_win.cpp

93

Yeah that's what I thought too. I didn't want to change that part of the existing behavior, however, since it doesn't seem to be causing an issue.

compiler-rt/lib/sanitizer_common/sanitizer_file.h

struct FileCloser {
   fd_t fd;
 };
 
 bool SupportsColoredOutput(fd_t fd);
 
 // OS
 const char *GetPwd();
 bool FileExists(const char *filename);
+bool DirExists(const char *path);
 char *FindPathToBinary(const char *name);
 bool IsPathSeparator(const char c);
 bool IsAbsolutePath(const char *path);
 // Returns true on success, false on failure.
 bool CreateDir(const char *pathname);
 // Starts a subprocess and returs its pid.
 // If *_fd parameters are not kInvalidFd their corresponding input/output
 // streams will be redirect to the file. The files will always be closed

compiler-rt/lib/sanitizer_common/sanitizer_file.cpp

 static void RecursiveCreateParentDirs(char *path) {
   if (path[0] == '\0')
     return;
   for (int i = 1; path[i] != '\0'; ++i) {
     char save = path[i];
     if (!IsPathSeparator(path[i]))
       continue;
     path[i] = '\0';
-    /* Some of these will fail, because the directory exists, ignore it. */
-    CreateDir(path);
+    if (!DirExists(path) && !CreateDir(path)) {
+      const char *ErrorMsgPrefix = "ERROR: Can't create directory: ";
+      WriteToFile(kStderrFd, ErrorMsgPrefix, internal_strlen(ErrorMsgPrefix));
+      WriteToFile(kStderrFd, path, internal_strlen(path));
+      Die();
+    }
     path[i] = save;
   }
 }
 
 void ReportFile::SetReportPath(const char *path) {
   if (path) {
     uptr len = internal_strlen(path);
     if (len > sizeof(path_prefix) - 100) {
       Report("ERROR: Path is too long: %c%c%c%c%c%c%c%c...\n", path[0], path[1],

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

 #else
   if (internal_stat(filename, &st))
 #endif
     return false;
   // Sanity check: filename is a regular file.
   return S_ISREG(st.st_mode);
 }
 
-#if !SANITIZER_NETBSD
+bool DirExists(const char *path) {
+  struct stat st;
+#  if SANITIZER_USES_CANONICAL_LINUX_SYSCALLS
+  if (internal_syscall(SYSCALL(newfstatat), AT_FDCWD, path, &st, 0))
+#  else
+  if (internal_stat(path, &st))
+#  endif
+    return false;
+  return S_ISDIR(st.st_mode);
+}
+
+#  if !SANITIZER_NETBSD
 tid_t GetTid() {
 #if SANITIZER_FREEBSD
   long Tid;
   thr_self(&Tid);
   return Tid;
 #elif SANITIZER_SOLARIS
   return thr_self();
 #else

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

if (ShouldMockFailureToOpen(filename))
     return false;
   struct stat st;
   if (stat(filename, &st))
     return false;
   // Sanity check: filename is a regular file.
   return S_ISREG(st.st_mode);
 }
 
+bool DirExists(const char *path) {
+  struct stat st;
+  if (stat(path, &st))
+    return false;
+  return S_ISDIR(st.st_mode);
+}
+
 tid_t GetTid() {
   tid_t tid;
   pthread_threadid_np(nullptr, &tid);
   return tid;
 }
 
 void GetThreadStackTopAndBottom(bool at_initialization, uptr *stack_top,
                                 uptr *stack_bottom) {

compiler-rt/lib/sanitizer_common/sanitizer_win.cpp

uptr GetMaxUserVirtualAddress() {
   return (uptr)si.lpMaximumApplicationAddress;
 }
 
 uptr GetMaxVirtualAddress() {
   return GetMaxUserVirtualAddress();
 }
 
 bool FileExists(const char *filename) {
   return ::GetFileAttributesA(filename) != INVALID_FILE_ATTRIBUTES;
 }
 
+bool DirExists(const char *path) {
+  auto attr = ::GetFileAttributesA(path);
+  return (attr != INVALID_FILE_ATTRIBUTES) && (attr & FILE_ATTRIBUTE_DIRECTORY);
+}
+
 uptr internal_getpid() {
   return GetProcessId(GetCurrentProcess());
 }
 
 int internal_dlinfo(void *handle, int request, void *p) {
   UNIMPLEMENTED();
 }
 

compiler-rt/lib/sanitizer_common/tests/sanitizer_libc_test.cpp

TEST(SanitizerCommon, mem_is_zero) {
   delete [] x;
 }
 
 struct stat_and_more {
   struct stat st;
   unsigned char z;
 };
 
+static void get_temp_dir(char *buf, size_t bufsize) {
+#if SANITIZER_WINDOWS
+  buf[0] = '\0';
+  if (!::GetTempPathA(bufsize, buf))
+    return;
+#else
+  const char *tmpdir = "/tmp";
+#  if SANITIZER_ANDROID
+  tmpdir = GetEnv("TMPDIR");
+#  endif
+  internal_snprintf(buf, bufsize, "%s", tmpdir);
+#endif
+}
+
 static void temp_file_name(char *buf, size_t bufsize, const char *prefix) {
 #if SANITIZER_WINDOWS
   buf[0] = '\0';
   char tmp_dir[MAX_PATH];
   if (!::GetTempPathA(MAX_PATH, tmp_dir))
     return;
   // GetTempFileNameA needs a MAX_PATH buffer.
   char tmp_path[MAX_PATH];
temp_file_name(tmpfile, sizeof(tmpfile),
                  "dir/sanitizer_common.reportfile.tmp.");
   report_file.SetReportPath(tmpfile);
   const char *path = report_file.GetReportPath();
   EXPECT_EQ(internal_strncmp(tmpfile, path, strlen(tmpfile)), 0);
   // This will close tmpfile.
   report_file.SetReportPath("stderr");
   Unlink(tmpfile);
 }
+
+TEST(SanitizerCommon, FileExists) {
+  char tmpfile[128];
+  temp_file_name(tmpfile, sizeof(tmpfile), "sanitizer_common.fileexists.tmp.");
+  fd_t fd = OpenFile(tmpfile, WrOnly);
+  ASSERT_NE(fd, kInvalidFd);
+  EXPECT_TRUE(FileExists(tmpfile));
+  CloseFile(fd);
+  Unlink(tmpfile);
+}
+
+TEST(SanitizerCommon, DirExists) {
+  char tmpdir[128];
+  get_temp_dir(tmpdir, sizeof(tmpdir));
+  EXPECT_TRUE(DirExists(tmpdir));
+}

compiler-rt/test/asan/TestCases/log-path_test.cpp

 // RUN: not %run %t 2> %t.out
 // RUN: FileCheck %s --check-prefix=CHECK-ERROR < %t.out
 
 // Good log_path.
 // RUN: rm -f %t.log.*
 // RUN: %env_asan_opts=log_path=%t.log not %run %t 2> %t.out
 // RUN: FileCheck %s --check-prefix=CHECK-ERROR < %t.log.*
 
-// Invalid log_path.
-// RUN: %env_asan_opts=log_path=/dev/null/INVALID not %run %t 2> %t.out
+// Invalid log_path in existing directory.
+// RUN: %env_asan_opts=log_path=/INVALID not %run %t 2> %t.out
 // RUN: FileCheck %s --check-prefix=CHECK-INVALID < %t.out
 
+// Directory of log_path can't be created.
+// RUN: %env_asan_opts=log_path=/dev/null/INVALID not %run %t 2> %t.out
+// RUN: FileCheck %s --check-prefix=CHECK-BAD-DIR < %t.out
+
 // Too long log_path.
 // RUN: %env_asan_opts=log_path=`for((i=0;i<10000;i++)); do echo -n $i; done` \
 // RUN:   not %run %t 2> %t.out
 // RUN: FileCheck %s --check-prefix=CHECK-LONG < %t.out
 
 // Run w/o errors should not produce any log.
 // RUN: rm -f %t.log.*
 // RUN: %env_asan_opts=log_path=%t.log  %run %t ARG ARG ARG
 // RUN: not cat %t.log.*
 
 // FIXME: log_path is not supported on Windows yet.
 // XFAIL: windows-msvc
 
 #include <stdlib.h>
 #include <string.h>
 int main(int argc, char **argv) {
   if (argc > 2) return 0;
   char *x = (char*)malloc(10);
   memset(x, 0, 10);
   int res = x[argc * 10];  // BOOOM
   free(x);
   return res;
 }
 // CHECK-ERROR: ERROR: AddressSanitizer
-// CHECK-INVALID: ERROR: Can't open file: /dev/null/INVALID
+// CHECK-INVALID: ERROR: Can't open file: /INVALID
+// CHECK-BAD-DIR: ERROR: Can't create directory: /dev/null
 // CHECK-LONG: ERROR: Path is too long: 01234

compiler-rt/test/memprof/TestCases/log_path_test.cpp

 // The for loop in the backticks below requires bash.
 // REQUIRES: shell
 //
 // RUN: %clangxx_memprof  %s -o %t
 
-// stderr print_text=true:log_path
+// stderr log_path
 // RUN: %env_memprof_opts=print_text=true:log_path=stderr %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-GOOD --dump-input=always
 
-// Good print_text=true:log_path.
+// Good log_path.
 // RUN: rm -f %t.log.*
 // RUN: %env_memprof_opts=print_text=true:log_path=%t.log %run %t
 // RUN: FileCheck %s --check-prefix=CHECK-GOOD --dump-input=always < %t.log.*
 
-// Invalid print_text=true:log_path.
-// RUN: %env_memprof_opts=print_text=true:log_path=/dev/null/INVALID not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID --dump-input=always
+// Invalid log_path.
+// RUN: %env_memprof_opts=print_text=true:log_path=/INVALID not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID --dump-input=always
 
-// Too long print_text=true:log_path.
+// Directory of log_path can't be created.
+// RUN: %env_memprof_opts=print_text=true:log_path=/dev/null/INVALID not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-BAD-DIR --dump-input=always
+
+// Too long log_path.
 // RUN: %env_memprof_opts=print_text=true:log_path=`for((i=0;i<10000;i++)); do echo -n $i; done` \
 // RUN:   not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-LONG --dump-input=always
 
 // Specifying the log name via the __memprof_profile_filename variable.
 // Note we use an invalid path since that is sufficient for checking that the
 // specified __memprof_profile_filename value is passed through to the runtime.
 // Using an automatically generated name via %t can cause weird issues with
 // unexpected macro expansion if the path includes tokens that match a build
 // system macro (e.g. "linux").
-// RUN: %clangxx_memprof  %s -o %t -DPROFILE_NAME_VAR="/dev/null/INVALID"
+// RUN: %clangxx_memprof  %s -o %t -DPROFILE_NAME_VAR="/INVALID"
 // RUN: not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID --dump-input=always
 
 #include <sanitizer/memprof_interface.h>
 
 #ifdef PROFILE_NAME_VAR
 #define xstr(s) str(s)
 #define str(s) #s
 char __memprof_profile_filename[] = xstr(PROFILE_NAME_VAR);
 #endif
 
 #include <stdlib.h>
 #include <string.h>
 int main(int argc, char **argv) {
   char *x = (char *)malloc(10);
   memset(x, 0, 10);
   free(x);
   __memprof_profile_dump();
   return 0;
 }
 // CHECK-GOOD: Memory allocation stack id
-// CHECK-INVALID: ERROR: Can't open file: /dev/null/INVALID
+// CHECK-INVALID: ERROR: Can't open file: /INVALID
+// CHECK-BAD-DIR: ERROR: Can't create directory: /dev/null
 // CHECK-LONG: ERROR: Path is too long: 01234

compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_test.cpp

 // Test __sanitizer_set_report_path and __sanitizer_get_report_path:
+// RUN: rm -rf %t.report_path
 // RUN: %clangxx -O2 %s -o %t
 // RUN: %run %t | FileCheck %s
+// Try again with a directory without write access.
+// RUN: rm -rf %t.report_path && mkdir -p %t.report_path
+// RUN: chmod u-w %t.report_path || true
+// RUN: not %run %t 2>&1 | FileCheck %s --check-prefix=FAIL
 
 #include <assert.h>
 #include <sanitizer/common_interface_defs.h>
 #include <stdio.h>
 #include <string.h>
 
 volatile int *null = 0;
 
 int main(int argc, char **argv) {
   char buff[1000];
   sprintf(buff, "%s.report_path/report", argv[0]);
   __sanitizer_set_report_path(buff);
   assert(strncmp(buff, __sanitizer_get_report_path(), strlen(buff)) == 0);
   printf("Path %s\n", __sanitizer_get_report_path());
 }
 
 // CHECK: Path {{.*}}Posix/Output/sanitizer_set_report_path_test.cpp.tmp.report_path/report.
+// FAIL: ERROR: Can't open file: {{.*}}Posix/Output/sanitizer_set_report_path_test.cpp.tmp.report_path/report.