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

[Alexey Samsonov via llvm-commits]

samsonov accepted this revision.
samsonov added a comment.
This revision is now accepted and ready to land.

LGTM. Please watch the bots after the commit. Thank you!


================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc:273-291
@@ +272,21 @@
+    CHECK(file_line_info);
+    // Parse the last :<int>, which must be there.
+    char *last_colon = internal_strrchr(file_line_info, ':');
+    CHECK(last_colon);
+    int line_or_column;
+    ExtractInt(last_colon + 1, "", &line_or_column);
+    *last_colon = '\0';
+    last_colon = internal_strrchr(file_line_info, ':');
+    if (last_colon && IsDigit(last_colon[1])) {
+      // If the colon before this one is followed by a digit, it must be the
+      // line number, and the previous parsed number was a column.
+      info->line = internal_atoll(last_colon + 1);
+      info->column = line_or_column;
+      *last_colon = '\0';
+    } else {
+      // Otherwise, we have line info but no column info.
+      info->line = line_or_column;
+      info->column = 0;
+    }
+    ExtractToken(file_line_info, "", &info->file);
+    InternalFree(file_line_info);
----------------
rnk wrote:
> This tokenization change here might be worth looking at closely, it's pretty horrible.
Yep, I've noticed this horridness. Might worth moving into a separate function btw.

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc:283
@@ +282,3 @@
+      // line number, and the previous parsed number was a column.
+      info->line = internal_atoll(last_colon + 1);
+      info->column = line_or_column;
----------------
Why not ExtractInt(last_colon + 1, "", &info->line)?

================
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;
----------------
I see, but I'd rather avoid changing the behavior here. If llvm-symbolizer (on Linux/Mac) failed to get function name, it indeed probably means that there is no debug info, and no way for us to get meaningful information for stack trace - so why try the alternatives? If there *is* some debug info, and llvm-symbolizer weren't able to locate it - we want to actually learn about this and fix a bug.


http://reviews.llvm.org/D11791