[compiler-rt] Allow suppression file paths be relative to the location of the executable.

[zaks.anna]

Summary

The ASanified executable could be launched from different locations. When we cannot find the suppression file relative to the current directory, try to see if the specified path is relative to the location of the executable.

[kcc]

Alexey, any comments?
We already pick up the default blacklist from the compiler distribution at compile time.
The default suppressions are better to be similar.

lib/sanitizer_common/sanitizer_suppressions.cc

87

this is probably unfriendly to windows.

113

ditto

[samsonov]

Thanks for working on this!

kcc@: We don't have file with default suppressions, do we?

lib/sanitizer_common/sanitizer_mac.cc

216

This can cause memory corruption if the length of the result is larger than buf_len. If you intend to call realpath(), you should probably store the result in the pre-allocated buffer of size at least kMaxPathLength, and then internal_strncpy it into buf.
Alternatively, assert that buf_len is large enough.

lib/sanitizer_common/sanitizer_suppressions.cc

83

Please make the output parameter last.

87

+1. Looks like we need a named constant for path separator that would be different on POSIX/Windows. It can also be used in functions like StripPathPrefix or StripModuleName.

129

You might add a VPrintf() that would tell the actual path of the file with suppressions used by the runtime.

[kcc]

kcc@: We don't have file with default suppressions, do we?

We do not, but I would rather prefer to have default suppressions than to work like suggested here.

[zaks.anna]

In D7309#118880, @kcc wrote:

kcc@: We don't have file with default suppressions, do we?

We do not, but I would rather prefer to have default suppressions than to work like suggested here.

I assume that the default suppressions are the bugs in libraries we already know about, correct?
My understanding is that those are currently baked into the ASan library.

This workflow is needed for cases where a user runs into an issue in a third party library, which they want to suppress. Allowing them to add the suppressions file can unblock them immediately.

[kcc]

me being stupid... For some reason I thought you are trying to read suppressions from a path relative to the compiler tool chain.
Ignore my previous comments. :)

[zaks.anna]

• Made this friendly to Windows. (Introduced "IsPathSeparator()" instead of a constant since multiple characters can serve as separators on Windows.)
• Addressed all other comments.
• Included a testcase, which I forgot to add the first time around.

[samsonov]

lib/sanitizer_common/sanitizer_mac.cc

217

You can merge these conditions with &&

lib/sanitizer_common/sanitizer_suppressions.cc

83

Function name looks confusing: it return an absolute path, not path relative to executable.

86

if (!ReadBinaryName(exec.data(), exec.size()))
  return false;
...
return true;

89

You can have a buffer overflow on rel_file_path here. Consider taking a length of rel_file_path buffer as an input argument and/or using internal_strncat.

108

const

lib/sanitizer_common/sanitizer_win.cc

320

return c == '\\';

[zaks.anna]

Updated to address the review comments from Alexey.

[samsonov]

LGTM after addressing a few comments below. Thanks!

lib/sanitizer_common/sanitizer_posix.cc

301

return path != nullptr && IsPathSeparator(path[0])

lib/sanitizer_common/sanitizer_suppressions.cc

90

Max(path_to_exec_len, new_file_path_size - 1);

107

Move these declarations closer to the place where they are actually used.

127

should this be file_path?

[glider]

Looks good in general

lib/sanitizer_common/sanitizer_common.h

221

Nit: how about "GetBinaryName"? After all, you're not reading it on Mac.

[zaks.anna]

Comments addressed and committed in r230723.

lib/sanitizer_common/sanitizer_common.h

221

The function only gets the file on Mac, but not on other platforms, so I don't think it should be renamed. (I've moved this from sanitizer_linux.h)

lib/sanitizer_common/sanitizer_common.h

 // OS
 void DisableCoreDumperIfNecessary();
 void DumpProcessMap();
 bool FileExists(const char *filename);
 const char *GetEnv(const char *name);
 bool SetEnv(const char *name, const char *value);
 const char *GetPwd();
 char *FindPathToBinary(const char *name);
+bool IsPathSeparator(const char c);
+bool IsAbsolutePath(const char *path);
+
+// Returns the path to the main executable.
+uptr ReadBinaryName(/*out*/char *buf, uptr buf_len);
 u32 GetUid();
 void ReExec();
 bool StackSizeIsUnlimited();
 void SetStackSizeLimitInBytes(uptr limit);
 bool AddressSpaceIsUnlimited();
 void SetAddressSpaceUnlimited();
 void AdjustStackSize(void *attr);
 void PrepareForSandboxing(__sanitizer_sandbox_arguments *args);

lib/sanitizer_common/sanitizer_linux.h

 uptr ThreadDescriptorSize();
 uptr ThreadSelf();
 uptr ThreadSelfOffset();
 
 // Matches a library's file name against a base name (stripping path and version
 // information).
 bool LibraryNameIs(const char *full_name, const char *base_name);
 
-// Read the name of the current binary from /proc/self/exe.
-uptr ReadBinaryName(/*out*/char *buf, uptr buf_len);
 // Cache the value of /proc/self/exe.
 void CacheBinaryName();
 
 // Call cb for each region mapped by map.
 void ForEachMappedRegion(link_map *map, void (*cb)(const void *, uptr));
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_FREEBSD || SANITIZER_LINUX
 #endif  // SANITIZER_LINUX_H

lib/sanitizer_common/sanitizer_mac.cc

 #include "sanitizer_internal_defs.h"
 #include "sanitizer_libc.h"
 #include "sanitizer_mac.h"
 #include "sanitizer_placement_new.h"
 #include "sanitizer_procmaps.h"
 
 #include <crt_externs.h>  // for _NSGetEnviron
 #include <fcntl.h>
+#include <mach-o/dyld.h>
 #include <pthread.h>
 #include <sched.h>
 #include <signal.h>
+#include <stdlib.h>
 #include <sys/mman.h>
 #include <sys/resource.h>
 #include <sys/stat.h>
 #include <sys/sysctl.h>
 #include <sys/types.h>
 #include <unistd.h>
 #include <libkern/OSAtomic.h>
 #include <errno.h>
if (len > name_len) {
         return *environ + name_len + 1;  // String starting after =.
       }
     }
     environ++;
   }
   return 0;
 }
 
+uptr ReadBinaryName(/*out*/char *buf, uptr buf_len) {
+  CHECK_LE(kMaxPathLength, buf_len);
+
+  // On OS X the executable path is saved to the stack by dyld. Reading it
+  // from there is much faster than calling dladdr, especially for large
+  // binaries with symbols.
+  InternalScopedString exe_path(kMaxPathLength);
+  uint32_t size = exe_path.size();
+  if (_NSGetExecutablePath(exe_path.data(), &size) == 0 &&
+      realpath(exe_path.data(), buf) != 0) {
+    return internal_strlen(buf);
+  }
+  return 0;
+}
+
 void ReExec() {
   UNIMPLEMENTED();
 }
 
 void PrepareForSandboxing(__sanitizer_sandbox_arguments *args) {
   (void)args;
   // Nothing here for now.
 }

lib/sanitizer_common/sanitizer_posix.cc

if (prefix_len + name_len + 2 <= kMaxPathLength) {
         return internal_strdup(buffer.data());
     }
     if (*end == '\0') break;
     beg = end + 1;
   }
   return 0;
 }
 
+bool IsPathSeparator(const char c) {
+  return c == '/';
+}
+
+bool IsAbsolutePath(const char *path) {
+  return IsPathSeparator(path[0]);
+}
+
 void ReportFile::Write(const char *buffer, uptr length) {
   SpinMutexLock l(mu);
   static const char *kWriteError =
       "ReportFile::Write() can't output requested buffer!\n";
   ReopenIfNecessary();
   if (length != internal_write(fd, buffer, length)) {
     internal_write(fd, kWriteError, internal_strlen(kWriteError));
     Die();

lib/sanitizer_common/sanitizer_suppressions.cc

SuppressionContext::SuppressionContext() : suppressions_(1), can_parse_(true) {
   internal_memset(has_suppresson_type_, 0, sizeof(has_suppresson_type_));
 }
 
 SuppressionContext *SuppressionContext::Get() {
   CHECK(suppression_ctx);
   return suppression_ctx;
 }
 
+static bool GetPathAssumingFileIsRelativeToExec(const char *file_path,
+                                                /*out*/char *new_file_path,
+                                                uptr new_file_path_size) {
+  InternalScopedString exec(kMaxPathLength);
+  if (ReadBinaryName(exec.data(), exec.size())) {
+    const char *file_name_pos = StripModuleName(exec.data());
+    uptr path_to_exec_len = file_name_pos - exec.data();
+    internal_strncat(new_file_path, exec.data(), path_to_exec_len);
+    internal_strncat(new_file_path, file_path,
+                     new_file_path_size - internal_strlen(new_file_path) - 1);
+    return true;
+  }
+  return false;
+}
+
 void SuppressionContext::InitIfNecessary() {
   if (suppression_ctx)
     return;
   suppression_ctx = new(placeholder) SuppressionContext;
   if (common_flags()->suppressions[0] == '\0')
     return;
+
+  const char *file_path = common_flags()->suppressions;
   char *suppressions_from_file;
   uptr buffer_size;
+  const uptr max_len = 1 << 26;
+
+  // If we cannot find the file, check if its location is relative to
+  // the location of the executable.
+  InternalScopedString new_file_path(kMaxPathLength);
+  if (!FileExists(file_path) && !IsAbsolutePath(file_path) &&
+      GetPathAssumingFileIsRelativeToExec(file_path, new_file_path.data(),
+                                          new_file_path.size())) {
+    file_path = new_file_path.data();
+  }
+
   uptr contents_size =
-      ReadFileToBuffer(common_flags()->suppressions, &suppressions_from_file,
-                       &buffer_size, 1 << 26 /* max_len */);
+    ReadFileToBuffer(file_path, &suppressions_from_file, &buffer_size, max_len);
+
+  VPrintf(1, "%s: reading suppressions file at %s\n",
+          SanitizerToolName, file_path);
+
   if (contents_size == 0) {
     Printf("%s: failed to read suppressions file '%s'\n", SanitizerToolName,
            common_flags()->suppressions);
     Die();
   }
   suppression_ctx->Parse(suppressions_from_file);
 }
 
 bool SuppressionContext::Match(const char *str, SuppressionType type,
                                Suppression **s) {
   if (!has_suppresson_type_[type])
     return false;
   can_parse_ = false;

lib/sanitizer_common/sanitizer_win.cc

void SetAddressSpaceUnlimited() {
   UNIMPLEMENTED();
 }
 
 char *FindPathToBinary(const char *name) {
   // Nothing here for now.
   return 0;
 }
 
+uptr ReadBinaryName(/*out*/char *buf, uptr buf_len) {
+  // Nothing here for now.
+  return 0;
+}
+
+bool IsPathSeparator(const char c) {
+  return c == '\\' || c == '/';
+}
+
+bool IsAbsolutePath(const char *path) {
+  UNIMPLEMENTED();
+}
+
 void SleepForSeconds(int seconds) {
   Sleep(seconds * 1000);
 }
 
 void SleepForMillis(int millis) {
   Sleep(millis);
 }
 

test/asan/TestCases/suppressions-exec-relative-location.cc

+// Check that without suppressions, we catch the issue.
+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck --check-prefix=CHECK-CRASH %s
+
+// If the executable is started from a different location, we should still
+// find the suppression file located relative to the location of the executable.
+// RUN: rm -rf %T/suppressions-exec-relative-location
+// RUN: mkdir -p %T/suppressions-exec-relative-location
+// RUN: %clangxx_asan -O0 %s -o %T/suppressions-exec-relative-location/exec
+// RUN: echo "interceptor_via_fun:crash_function" > \
+// RUN:   %T/suppressions-exec-relative-location/supp.txt
+// RUN: ASAN_OPTIONS="suppressions=supp.txt" \
+// RUN:   %run %T/suppressions-exec-relative-location/exec 2>&1 | \
+// RUN:   FileCheck --check-prefix=CHECK-IGNORE %s
+// RUN: rm -rf %T/suppressions-exec-relative-location
+
+// If the wrong absolute path is given, we don't try to construct
+// a relative path with it.
+// RUN: ASAN_OPTIONS="suppressions='/absolute/path'" not %run %t 2>&1 | \
+// RUN:   FileCheck --check-prefix=CHECK-WRONG-FILE-NAME %s
+
+// Test that we reject directory as filename.
+// RUN: ASAN_OPTIONS="suppressions='folder/only/'" not %run %t 2>&1 | \
+// RUN:   FileCheck --check-prefix=CHECK-WRONG-FILE-NAME %s
+
+// XFAIL: android
+// XFAIL: win32
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+void crash_function() {
+  char *a = (char *)malloc(6);
+  free(a);
+  size_t len = strlen(a); // BOOM
+  fprintf(stderr, "strlen ignored, len = %zu\n", len);
+}
+
+int main() {
+  crash_function();
+}
+
+// CHECK-CRASH: AddressSanitizer: heap-use-after-free
+// CHECK-IGNORE-NOT: AddressSanitizer: heap-buffer-overflow
+// CHECK-IGNORE: ignored
+// CHECK-WRONG-FILE-NAME: failed to read suppressions file