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

[Reid Kleckner via llvm-commits]

rnk marked 3 inline comments as done.

================
Comment at: CMakeLists.txt:358
@@ -357,1 +357,3 @@
 
+set(COMPILER_RT_LLD_PATH ${LLVM_MAIN_SRC_DIR}/tools/lld)
+if(EXISTS ${COMPILER_RT_LLD_PATH}/)
----------------
samsonov wrote:
> Please land this (together with lit config and `test/cfi` changes) in a separate small commit.
rL244549

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc:306
@@ +305,3 @@
+
+  // If we got a function name, we probably had debug info.
+  return last->info.function != nullptr;
----------------
samsonov wrote:
> Why do you need to optionally return false (and fallback to the next symbolizer in chain here?
I guess I don't anymore, now that the test case updates have landed. Previously I was trying to make it so that asan would use dbghelp for modules with PDBs and llvm-symbolizer for modules with DWARF. What I discovered was that llvm-symbolizer handles both PDBs and DWARF *and* provides column information, which dbghelp does not. At that point, I made the test case updates and reordered things to prefer llvm-symbolizer over dbghelp.

I probably don't need this functionality anymore, but it seems useful to me. llvm-symbolizer might not be able to answer a particular query, and this lets some other tool try.

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc:420
@@ +419,3 @@
+  bool success = WriteToFile(output_fd_, buffer, length, &write_len, &err);
+  if (!success || write_len == 0 || write_len == (uptr)-1) {
+    Report("WARNING: Can't write to symbolizer at fd %d (error %d)\n",
----------------
samsonov wrote:
> Same here: `write_len` can't be -1 if `success` is true. In fact, these changes (and `CloseFile` change above) are bugfixes, because checking for -1 was wrong in the first place - one should have called `internal_iserror` on the result of `internal_write` instead. I would appreciate if you will replace internal_read / internal_write / internal_close with ReadFromFile / WriteToFile / CloseFile as a separate small commit preceding this one.
rL244546


http://reviews.llvm.org/D11791