[PATCH] D36032: [sanitizer_common] Fuchsia-specific symbolizer

[Roland McGrath via Phabricator via llvm-commits]

mcgrathr added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_stacktrace_printer.cc:20
 
+// sanitizer_symbolizer_fuchsia.cc implements these differently for Fuchsia.
+#if !SANITIZER_FUCHSIA
----------------
vitalybuka wrote:
> I don't think this comment is necessary
You really prefer few comments???


================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_fuchsia.cc:32
+// This is used by UBSan for type names, and by ASan for global variable names.
+constexpr const char *kFormatDemangle = "{{{symbol:%s}}}";
+constexpr uptr kFormatDemangleMax = 1024;  // Arbitrary.
----------------
vitalybuka wrote:
> Regular array is shorter
> ```
> const char kFormatDemangle[] = 
> ```
constexpr is correct for this use.  The compiler should know that it's just a vanilla string constant and doesn't need to be an independent link-time object.


================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_fuchsia.cc:47
+// Symbolizer class supported linker initialization.
+Symbolizer *Symbolizer::GetOrInit() {
+  SpinMutexLock l(&init_mu_);
----------------
vitalybuka wrote:
> Why not just override PlatformInit?
Because then GetOrInit would be the only thing from sanitizer_symbolizer_libcdep.cc actually used and it's cleaner to #if out the entire file.
I'd be happy to move the generic GetOrInit from sanitizer_symbolizer_libcdep.cc to sanitizer_symbolizer.cc if you prefer that.


https://reviews.llvm.org/D36032