[PATCH] [compiler-rt] Symbolizer refactoring: Merge common parts of POSIXSymbolizer and WinSymbolizer

Timur Iskhodzhanov timurrrr at google.com
Fri Mar 6 08:04:13 PST 2015


Deferring to Alexey.

Some comments inline:


================
Comment at: lib/sanitizer_common/sanitizer_symbolizer.cc:98
@@ +97,3 @@
+  if (!FindModuleNameAndOffsetForAddress(addr, &module_name, &module_offset))
+    return res;
+  // Always fill data about module name and offset.
----------------
Interesting: is it possible for a PC to not belong to any module, but still be symbolizable by SymbolizerTools?  JITed code maybe?

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer.h:132
@@ -128,1 +131,3 @@
   static StaticSpinMutex init_mu_;
+  BlockingMutex mu_;
+
----------------
I encourage you to write a comment explaining that `mu_` not only synchronizes calls to `Symbolizer`, but should actually be used to synchronize all the calls to the underlying symbolizers (e.g. `dbghelp` on Windows).  Not sure where's the best place to put this comment.

Random thought: I wonder if it's time to create a new `lib/sanitizer_common/symbolization` dir?..

http://reviews.llvm.org/D8105

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list