[PATCH] D11791: [Windows] Use llvm-symbolizer before using dbghelp

[Alexey Samsonov via llvm-commits]

samsonov added a comment.

Looks like some auxiliary NFC changes can be reviewed or landed separately to minimize the diff. As for libcdep/nolibc I think that "make check-all" on Linux should properly check for that.


================
Comment at: lib/sanitizer_common/sanitizer_common.cc:347
@@ -346,1 +346,3 @@
 
+char *FindPathToBinary(const char *name) {
+  const char *path = GetEnv("PATH");
----------------
Move this function in a separate change.

================
Comment at: lib/sanitizer_common/sanitizer_common.cc:355
@@ +354,3 @@
+  while (true) {
+    const char *end = internal_strchrnul(beg, SANITIZER_WINDOWS ? ';' : ':');
+    uptr prefix_len = end - beg;
----------------
Use a named constant for pathsep

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_win.cc:196
@@ +195,3 @@
+    CHECK(!internal_strchr(arg, '"') && "quotes in args unsupported");
+    CHECK(arglen > 0 && arg[arglen - 1] &&
+          "args ending in backslash and empty args unsupported");
----------------
Shouldn't you actually check for backslash?

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_win.cc:242
@@ +241,3 @@
+
+  // Add llvm-symbolizer in case the binary has dwarf.
+  const char *user_path = common_flags()->external_symbolizer_path;
----------------
Does Windows properly handle `common_flags()->symbolize`?  We shouldn't create any symbolizers if this flag is true.

================
Comment at: test/asan/CMakeLists.txt:19
@@ +18,3 @@
+  list(APPEND ASAN_TEST_DEPS asan)
+  if(WIN32 AND EXISTS ${CMAKE_SOURCE_DIR}/tools/lld)
+    list(APPEND CFI_TEST_DEPS
----------------
Let's introduce `COMPILER_RT_HAS_LLD_SOURCES` in the same way we use `COMPILER_RT_HAS_LIBCXX_SOURCES`, and use a common variable to check if lld is available somewhere in `test/lit.common.configured.in` instead of `ASAN_HAS_LLD`

================
Comment at: test/asan/CMakeLists.txt:20
@@ +19,3 @@
+  if(WIN32 AND EXISTS ${CMAKE_SOURCE_DIR}/tools/lld)
+    list(APPEND CFI_TEST_DEPS
+      lld
----------------
Copy-paste error

================
Comment at: test/asan/TestCases/Windows/operator_new_left_oob.cc:14
@@ -13,3 +13,3 @@
 // CHECK: allocated by thread T0 here:
-// CHECK:   {{#0 .* operator new }}
+// CHECK:   {{#0 .* operator new}}
 // CHECK:   {{#1 .* main .*operator_new_left_oob.cc}}:[[@LINE-8]]
----------------
Please relax all these test cases in a separate change.


http://reviews.llvm.org/D11791