[asan] Search for llvm-symbolizer in the executable's directory instead of only searching in PATH.

[akhuang]

Summary

Currenly asan only looks for llvm-symbolizer in PATH and ASAN_OPTIONS,
but it seems like it would make sense to also look in the same directory as the
executable.

For example, in Chromium, the easiest way to have asan tests use llvm-symbolizer
is to specify the path in ASAN_OPTIONS. It'd be convenient to have asan just find
the symbolizer if it's in the same directory.

[rnk]

For example, in Chromium, the easiest way to have asan tests use llvm-symbolizer
is to specify the path in ASAN_OPTIONS. It'd be convenient to have asan just find
the symbolizer if it's in the same directory.

I think it's worth pointing out that, for developers that don't put LLVM's bin directory on their PATH, it makes LLVM binary ASan report symbolization work out the box. Other projects, such as Chromium, can achieve the same effect by copying llvm-symbolizer into their build directories.

It's also consistent with what LLVM's crash handler does:
https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/Signals.cpp#L128

compiler-rt/lib/sanitizer_common/sanitizer_file.cpp

208

This can be expensive on Linux, where it means reading /proc/self/exe. I'd consider ReadBinaryNameCached.

compiler-rt/lib/sanitizer_common/sanitizer_win.cpp

1050

It seems worth noting that this function is documented to always null terminate the output string.

1054–1056

According to the documentation, the output is always null terminated if you include the null terminator in the input (the +1 here). So, I think you can remove the second = '\0'.

[vitalybuka]

I am not sure about this change. For the most of users it's very unlikely to have llvm-symbolizer next to the tested binary. Looks like this just simplifies few tests. I'd rather use usual approach with ASAN_OPTIONS there.

[rnk]

I think there is a huge amount of value in making asan binaries produce symbolized reports out of the box without having to set any environment variables or wrap the binaries in a script. I'm open to any solution that achieves that, but most users aren't going to read the docs and remember to set ASAN_OPTIONS before running a chrome test binary. Copying (or linking) llvm-symbolizer into the build directory is something reliable that any build system with asan support can do.

One alternative idea we had was to bake this ASAN_OPTION into chrome binaries:

external_symbolizer_path="../../llvm-build/Release+Asserts/bin/llvm-symbolizer.exe"

But this only works so long as the user's CWD is the build directory. Absolute paths aren't allowed for build hermeticity reasons.

Any other suggestions for making this work out of the box?

[eugenis]

1. Find llvm-symbolizer in /usr/bin, same as addr2line before?
2. Support something like $ORIGIN substitution in llvm_symbolizer_path.

I'm a bit uneasy about running random binaries from the current directory. Consider for example if I download an asan binary and run it from the chrome downloads directory - I don't necessarily trust the llvm-symbolizer binary in the same directory.

[rnk]

In D93070#2449228, @eugenis wrote:

1. Find llvm-symbolizer in /usr/bin, same as addr2line before?
2. Support something like $ORIGIN substitution in llvm_symbolizer_path.

I'm a bit uneasy about running random binaries from the current directory. Consider for example if I download an asan binary and run it from the chrome downloads directory - I don't necessarily trust the llvm-symbolizer binary in the same directory.

Fair enough, but ASan today will check if the program exists in the CWD of the program before doing a PATH search.

I'm not that keen on 1, since there's no equivalent to /usr/bin/ on Windows, it doesn't really solve our problem. 2, a metavar like $ORIGIN, would solve the issue. Or a special ASAN_OPTION, similar to external_symbolizer_path.

Amy, do you want to run with the $ORIGIN idea?

compiler-rt/lib/sanitizer_common/sanitizer_file.cpp

182

Note that asan first checks if the program exists in the CWD here.

[akhuang]

Sure, I'm not really familiar with $ORIGIN - how does it work?

[vitalybuka]

Can we embed clang binary location when compiling the program?

[rnk]

In D93070#2449556, @akhuang wrote:

Sure, I'm not really familiar with $ORIGIN - how does it work?

You can search for "rpath origin" for background on the feature. The basic idea is that it's a special variable that expands to the directory containing the binary using the variable.

In Chromium, we'd use it by adding external_symbolizer_path=$ORIGIN/../../third_party/llvm-build/Release+Asserts/bin/llvm-symbolizer(.exe) to the __asan_default_option strings here:
https://source.chromium.org/chromium/chromium/src/+/master:build/sanitizers/sanitizer_options.cc;l=31?q=asan_default_options&ss=chromium

In D93070#2449889, @vitalybuka wrote:

Can we embed clang binary location when compiling the program?

We used to embed paths to clang runtime libraries derived from the path to the clang binary, but it was error prone (https://crbug.com/989372). We removed them here in D65543. If the user (or distributed build system in our case) is not careful and invokes the compiler with an absolute path, any paths derived from the path to the compiler will become absolute, and potentially wrong in a distributed context.

[vitalybuka]

I don't have strong opinion

compiler-rt/lib/sanitizer_common/sanitizer_file.cpp

bool ReadFileToVector(const char *file_name,
   CloseFile(fd);
   buff->resize(read_len);
   return true;
 }
 
 static const char kPathSeparator = SANITIZER_WINDOWS ? ';' : ':';
 
 char *FindPathToBinary(const char *name) {
   if (FileExists(name)) {
     return internal_strdup(name);
   }
 
   const char *path = GetEnv("PATH");
   if (!path)
     return nullptr;
   uptr name_len = internal_strlen(name);
   InternalMmapVector<char> buffer(kMaxPathLength);
   const char *beg = path;
   while (true) {
     const char *end = internal_strchrnul(beg, kPathSeparator);
     uptr prefix_len = end - beg;
     if (prefix_len + name_len + 2 <= kMaxPathLength) {
       internal_memcpy(buffer.data(), beg, prefix_len);
       buffer[prefix_len] = '/';
       internal_memcpy(&buffer[prefix_len + 1], name, name_len);
       buffer[prefix_len + 1 + name_len] = '\0';
       if (FileExists(buffer.data()))
         return internal_strdup(buffer.data());
     }
     if (*end == '\0') break;
     beg = end + 1;
   }
+
+  // If not in PATH, search in the same directory as the executable.
+  if (ReadBinaryName(buffer.data(), kMaxPathLength)) {
+    const char *exec_name_pos = StripModuleName(buffer.data());
+    uptr path_to_exec_len = exec_name_pos - buffer.data();
+    buffer[path_to_exec_len] = '\0';
+    internal_strncat(buffer.data(), name, buffer.size());
+    if (FileExists(buffer.data()))
+      return internal_strdup(buffer.data());
+  }
+
   return nullptr;
 }
 
 } // namespace __sanitizer
 
 using namespace __sanitizer;
 
 extern "C" {

compiler-rt/lib/sanitizer_common/sanitizer_win.cpp

case EXCEPTION_INT_DIVIDE_BY_ZERO:
       return "int-divide-by-zero";
     case EXCEPTION_INT_OVERFLOW:
       return "int-overflow";
   }
   return "unknown exception";
 }
 
 uptr ReadBinaryName(/*out*/char *buf, uptr buf_len) {
-  // FIXME: Actually implement this function.
   CHECK_GT(buf_len, 0);
-  buf[0] = 0;
-  return 0;
+
+  // Get the UTF-16 path and convert to UTF-8.
+  wchar_t binname_utf16[kMaxPathLength];
+  int binname_utf16_len =
+      GetModuleFileNameW(NULL, binname_utf16, kMaxPathLength);
+  if (binname_utf16_len == 0)
+    binname_utf16[0] = '\0';
+  int binary_name_len =
+      ::WideCharToMultiByte(CP_UTF8, 0, binname_utf16, binname_utf16_len + 1,
+                            &buf[0], kMaxPathLength, NULL, NULL);
+  buf[binary_name_len] = '\0';
+  return binary_name_len;
 }
 
 uptr ReadLongProcessName(/*out*/char *buf, uptr buf_len) {
   return ReadBinaryName(buf, buf_len);
 }
 
 void CheckVMASize() {
   // Do nothing.

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

 //===-- sanitizer_common_test.cpp -----------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file is a part of ThreadSanitizer/AddressSanitizer runtime.
 //
 //===----------------------------------------------------------------------===//
 #include <algorithm>
 
 #include "sanitizer_common/sanitizer_allocator_internal.h"
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_file.h"
 #include "sanitizer_common/sanitizer_flags.h"
 #include "sanitizer_common/sanitizer_libc.h"
 #include "sanitizer_common/sanitizer_platform.h"
 

 }
 
 #if SANITIZER_LINUX && !SANITIZER_ANDROID
 TEST(SanitizerCommon, FindPathToBinary) {
   char *true_path = FindPathToBinary("true");
   EXPECT_NE((char*)0, internal_strstr(true_path, "/bin/true"));
   InternalFree(true_path);
   EXPECT_EQ(0, FindPathToBinary("unexisting_binary.ergjeorj"));
+
+  // Search for something in the llvm build directory.
+  char *clang_path = FindPathToBinary("clang");
+  EXPECT_NE((char *)0, internal_strstr(clang_path, "clang"));
 }
 #elif SANITIZER_WINDOWS
 TEST(SanitizerCommon, FindPathToBinary) {
   // ntdll.dll should be on PATH in all supported test environments on all
   // supported Windows versions.
   char *ntdll_path = FindPathToBinary("ntdll.dll");
   EXPECT_NE((char*)0, internal_strstr(ntdll_path, "ntdll.dll"));
   InternalFree(ntdll_path);
   EXPECT_EQ(0, FindPathToBinary("unexisting_binary.ergjeorj"));
+
+  // Search for something in the llvm build directory.
+  char *clang_path = FindPathToBinary("clang-cl.exe");
+  EXPECT_NE((char *)0, internal_strstr(clang_path, "clang-cl.exe"));
 }
 #endif
 
 TEST(SanitizerCommon, StripPathPrefix) {
   EXPECT_EQ(0, StripPathPrefix(0, "prefix"));
   EXPECT_STREQ("foo", StripPathPrefix("foo", 0));
   EXPECT_STREQ("dir/file.cc",
                StripPathPrefix("/usr/lib/dir/file.cc", "/usr/lib/"));

compiler-rt/test/asan/TestCases/suppressions-exec-relative-location.cpp

 // RUN: %env_asan_opts=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: %env_asan_opts=suppressions='"folder/only/"' not %run %t 2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-WRONG-FILE-NAME %s
 
 // XFAIL: android
-// XFAIL: windows-msvc
 // UNSUPPORTED: ios
 
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
 void crash_function() {
   char *a = (char *)malloc(6);