[PATCH] [compiler-rt] Symbolizer refactoring: Link symbolizer tools into a fallback chain

Alexey Samsonov vonosmas at gmail.com
Tue Mar 3 16:30:04 PST 2015


================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_internal.h:31
@@ -30,1 +30,3 @@
  public:
+  SymbolizerTool *next;
+
----------------
You need a SymbolizerTool constructor that would initialize next to `nullptr`. Also, add a comment here why you need it.

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:353
@@ -352,3 +352,3 @@
  public:
-  explicit POSIXSymbolizer(SymbolizerTool *symbolizer_tool)
-      : Symbolizer(), symbolizer_tool_(symbolizer_tool) {}
+  explicit POSIXSymbolizer() : Symbolizer() {}
+
----------------
Consider just taking IntrusiveList<SymbolizerTool> as an input parameter. Otherwise you:
  * need to call tools_.clear() in constructor anyway
  * introduce an implicit assertion that AppendSymbolizerTool should be called before using any other methods.

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:372
@@ +371,3 @@
+      if (tool->SymbolizePC(addr, res)) {
+        break;
+      }
----------------
Why not just return here?

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:392
@@ +391,3 @@
+      if (tool->SymbolizeData(addr, info)) {
+        break;
+      }
----------------
ditto

http://reviews.llvm.org/D8049

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






More information about the llvm-commits mailing list