[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