[PATCH] [compiler-rt] Symbolizer refactoring: Make WinSymbolizer use SymbolizerTool interface

Timur Iskhodzhanov timurrrr at google.com
Fri Mar 6 05:33:17 PST 2015


Do you have a Windows box where you can test the patch or do you need help with that?


================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_win.cc:26
@@ -24,4 +25,3 @@
 
-class WinSymbolizer : public Symbolizer {
- public:
-  WinSymbolizer() : initialized_(false) {}
+static bool initialized_ = false;
+static bool TrySymInitialize();
----------------
please rename to `is_dbghelp_initialized`;

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_win.cc:69
@@ -31,3 +68,3 @@
 
-    BlockingMutexLock l(&dbghelp_mu_);
-    InitializeIfNeeded();
+static void InitializeIfNeeded() {
+  if (initialized_)
----------------
Please rename to `InitializeDbgHelpIfNeeded` and move to the top of the file.
Please add a comment that calls to this function should be synchronized, as well as other calls to dbghelp.

You may also consider using an anonymous namespace instead of `static` and group other dbghelp-related things in that namespace.

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_win.cc:121
@@ -66,2 +120,3 @@
 
-  bool CanReturnFileLineInfo() override {
+static bool TrySymInitialize() {
+  SymSetOptions(SYMOPT_DEFERRED_LOADS | SYMOPT_UNDNAME | SYMOPT_LOAD_LINES);
----------------
ditto -- please move to the top of the file

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_win.cc:145
@@ +144,3 @@
+ public:
+  explicit WinSymbolizer(SymbolizerTool *tool) : Symbolizer(), tool_(tool) {}
+  SymbolizedStack *SymbolizePC(uptr addr) override {
----------------
nit: please add a CHECK that tool is nonzero.

http://reviews.llvm.org/D8089

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






More information about the llvm-commits mailing list