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

[Alexey Samsonov]

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());``

http://reviews.llvm.org/D8029

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