[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