[PATCH] [compiler-rt] Symbolizer refactoring: Unify access to symbolizer tools from POSIXSymbolizer

Alexey Samsonov vonosmas at gmail.com
Tue Mar 3 14:27:38 PST 2015

In http://reviews.llvm.org/D8029#133842, @kubabrecka wrote:

> In http://reviews.llvm.org/D8029#133837, @samsonov wrote:
> > I'm happy with the direction of this change, although:
> >
> > Note that this does introduce a change in behavior - in case both libbacktrace *and* external symbolizer are available, we could fallback from former to latter. Is there a strong reason for removing this fallback? I originally thought that it can be useful in symbolizers you were going to add.
> The initialization of the external symbolizer (in PlatformInit) is wrapped inside `if (!libbacktrace_symbolizer)`, so it doesn't look to me that we currently have such a fallback.  Or am I reading the code wrong?

Ah, you are right. Feel free to submit this change first if it's convenient for you.

> Anyway, I definitely support having fallbacks, and I plan to submit another patch that implements the chain of symbolizers.

Comment at: lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:482
@@ -510,10 +481,3 @@
-  InternalSymbolizer* internal_symbolizer =
-      InternalSymbolizer::get(&symbolizer_allocator_);
-  SymbolizerTool *external_symbolizer = 0;
-  LibbacktraceSymbolizer *libbacktrace_symbolizer = 0;
-  if (!internal_symbolizer) {
-    libbacktrace_symbolizer =
-        LibbacktraceSymbolizer::get(&symbolizer_allocator_);
-    if (!libbacktrace_symbolizer) {
+  SymbolizerTool *tool = InternalSymbolizer::get(&symbolizer_allocator_);
+  if (!tool) {
I would re-write this function using early returns:

  static SymbolizerTool *ChooseSymbolizer() {
    if (!common_flags()->symbolize)
      return nullptr;
    if (SymbolizerTool *tool = InternalSymbolizer::get(..)) {
      return tool;
    if (SymbolizerTool *tool = LiibacktraceSymbolizer::get(..)) {
      return tool;

and then have a single ``new(symbolizer_allocator_) POSIXSymbolizer(ChooseSymbolizer());``



More information about the llvm-commits mailing list