[PATCH] [Sanitizer] Implement Symbolizer class on Windows

Timur Iskhodzhanov timurrrr at google.com
Wed Dec 18 06:32:47 PST 2013



================
Comment at: lib/lsan/lsan_common.cc:483
@@ -482,3 +482,3 @@
   for (uptr i = 0; i < kMaxAddrFrames; i++) new (&addr_frames[i]) AddressInfo();
-  uptr addr_frames_num = Symbolizer::Get()->SymbolizeCode(
+  uptr addr_frames_num = Symbolizer::Get()->SymbolizePC(
       addr, addr_frames.data(), kMaxAddrFrames);
----------------
Alexey Samsonov wrote:
> Please commit this renaming as a separate change.
Done, r197569.

================
Comment at: lib/sanitizer_common/sanitizer_stacktrace.cc:87
@@ +86,3 @@
+        } else {
+          if (info.function && info.function_offset != AddressInfo::kUnknown)
+            frame_desc.append("+0x%zx", info.function_offset);
----------------
Alexey Samsonov wrote:
> I suggest to put this part under "if (info.function)" above
>   if (info.function) {
>     frame_desc.append(...);
>     // Print offset in function if we don't know the source file
>     if (!info.file && info.function_offset != AddressInfo::kUnknown)
>       frame_desc.append(...);
>   }
> 
> In this way code corresponds to output format:
> [<function>[+<offset>]] [file-line-info | module+offset]
OK, agreed.

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer.h:47
@@ -40,3 +46,3 @@
 
   // Deletes all strings and sets all fields to zero.
   void Clear() {
----------------
Alexey Samsonov wrote:
> update the comment (or delete it)
Now it's 
`// Deletes all strings and resets all fields.`

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_win.cc:67
@@ +66,3 @@
+    IMAGEHLP_MODULE64 mod_info;
+    memset(&mod_info, 0, sizeof(mod_info));
+    mod_info.SizeOfStruct = sizeof(mod_info);
----------------
Alexey Samsonov wrote:
> internal_memset please
oh um yeah

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_win.cc:83
@@ +82,3 @@
+  BlockingMutex dbghelp_mu_;
+  bool initialized_;
+};
----------------
Alexey Samsonov wrote:
> initialized_ should be initialized in the constructor.
Done already

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_win.cc:88
@@ +87,3 @@
+  static bool called_once = false;
+  CHECK(!called_once && "Shouldn't create more than onesymbolizer");
+  called_once = true;
----------------
Alexey Samsonov wrote:
> s/onesymbolizer/one symbolizer/
Done already (wasn't worth another email, so I didn't upload)


http://llvm-reviews.chandlerc.com/D2434



More information about the llvm-commits mailing list