[PATCH] D46462: [sanitizer] Allow Fuchsia symbolizer to be reused by Myriad RTEMS

Roland McGrath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 4 14:33:20 PDT 2018


mcgrathr added a comment.

I'd prefer to keep all the markup string constants together.  Moving them all to a header would be fine, but I think it should be a separate sanitizer_symbolizer_fuchsia.h so that non-Fuchsia builds aren't using symbolizer_fuchsia.h, which is about OS-specific stuff distinct from the symbolizer markup format.  But nothing in this patch actually needs the constant to be visible outside sanitizer_symbolizer_fuchsia.cc, so I'd like to see some explanation of the need for that.

I'd be very glad to see a more thorough refactoring to make the symbolizer markup an option independent of OS.  I think renaming symbolizer_fuchsia* to symbolizer_markup* probably makes sense.  Ideally we'd nicely support both the Fuchsia/RTEMS configs where we want to support only markup and not have OS dependencies for things like files and subprocesses the other symbolizer code has; and also make markup a runtime selectable option for symbolizer style for the other OS builds that today have several runtime selectable choices.



================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_fuchsia.cc:115
 
+#if !SANITIZER_RTEMS
 struct UnwindTraceArg {
----------------
Let's make this something like SANITIZER_USE_UNWIND, i.e. more generic to what it's doing.  Then you can define that to 1/0 depending on the OS defines in one of the headers.


Repository:
  rL LLVM

https://reviews.llvm.org/D46462





More information about the llvm-commits mailing list