[PATCH] D11681: [asan] Print VAs instead of RVAs for module offsets on Windows

[Alexey Samsonov]

samsonov added a comment.

LGTM. I'm slightly worried about existing clients who might depend on ASan stack traces containing RVAs. Does it mean that they should just drop "--relative-address" flag from their llvm-symbolizer.exe invocation? If yes, please specify this in the commit message.


================
Comment at: lib/sanitizer_common/sanitizer_win.cc:328
@@ +327,3 @@
+// Scoped file handle closer.
+struct FileCloser {
+  FileCloser(fd_t fd) : fd(fd) {}
----------------
You can probably move this to sanitizer_common.h

================
Comment at: lib/sanitizer_common/sanitizer_win.cc:340
@@ +339,3 @@
+// this address.
+static uptr GetPreferredBase(char *modname) {
+  fd_t fd = OpenFile(modname, RdOnly, nullptr);
----------------
`const char *modname`

================
Comment at: lib/sanitizer_common/sanitizer_win.cc:372
@@ +371,3 @@
+  char *pe_sig = &buf[0];
+  if (memcmp(pe_sig, "PE\0\0", 4) != 0)
+    return 0;
----------------
`internal_memcmp`

================
Comment at: lib/sanitizer_common/sanitizer_win.cc:421
@@ +420,3 @@
+    // TODO: Handle paths larger than MAX_PATH.
+    wchar_t modname_utf16[MAX_PATH];
+    int modname_utf16_len = GetModuleFileNameW(handle, modname_utf16, MAX_PATH);
----------------
I'd vote for `kMaxPathLength`,  but I see that MAX_PATH is already used in several places for Windows.

================
Comment at: lib/sanitizer_common/sanitizer_win.cc:503
@@ -420,3 +502,3 @@
 bool ReadFromFile(fd_t fd, void *buff, uptr buff_size, uptr *bytes_read,
-                  error_t *error_p) {
-  UNIMPLEMENTED();
+                  error_t *last_error) {
+  CHECK(fd != kInvalidFd);
----------------
Keep `error_p` to be consistent with function decl?


http://reviews.llvm.org/D11681