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

[waltl]

Summary

Rename sanitizer_symbolizer_fuchsia.cc to
sanitizer_symbolizer_markup.cc, and use it for both Fuchsia and RTEMS.

[mcgrathr]

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.

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_fuchsia.cc

115 ↗

(On Diff #145279)

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.

[mcgrathr]

@jakehehrlich is working on an open-source tool at https://fuchsia.googlesource.com/tools that handles this output format as specified in https://fuchsia.googlesource.com/zircon/+/master/docs/symbolizer_markup.md

[waltl]

Address CR comments

[waltl]

In D46462#1088561, @mcgrathr wrote:

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.

I've done the basic refactoring as suggested. Please take a look.

[mcgrathr]

LGTM assuming the Fuchsia build still works (I don't see any problems in the source off hand).
@jakehehrlich will verify the Fuchsia Build.

[waltl]

In D46462#1093349, @mcgrathr wrote:

LGTM assuming the Fuchsia build still works (I don't see any problems in the source off hand).
@jakehehrlich will verify the Fuchsia Build.

Thanks for the review. I will go ahead and submit -- let me know if there is a problem.

lib/sanitizer_common/CMakeLists.txt

 
 set(SANITIZER_SYMBOLIZER_SOURCES
   sanitizer_stackdepot.cc
   sanitizer_stacktrace.cc
   sanitizer_stacktrace_libcdep.cc
   sanitizer_stacktrace_printer.cc
   sanitizer_stacktrace_sparc.cc
   sanitizer_symbolizer.cc
-  sanitizer_symbolizer_fuchsia.cc
   sanitizer_symbolizer_libbacktrace.cc
   sanitizer_symbolizer_libcdep.cc
   sanitizer_symbolizer_mac.cc
+  sanitizer_symbolizer_markup.cc
   sanitizer_symbolizer_posix_libcdep.cc
   sanitizer_symbolizer_report.cc
   sanitizer_symbolizer_win.cc
   sanitizer_unwind_linux_libcdep.cc
   sanitizer_unwind_win.cc)
 
 # Explicitly list all sanitizer_common headers. Not all of these are
 # included in sanitizer_common source files, but we need to depend on
set(SANITIZER_HEADERS
   sanitizer_report_decorator.h
   sanitizer_stackdepot.h
   sanitizer_stackdepotbase.h
   sanitizer_stacktrace.h
   sanitizer_stacktrace_printer.h
   sanitizer_stoptheworld.h
   sanitizer_suppressions.h
   sanitizer_symbolizer.h
+  sanitizer_symbolizer_fuchsia.h
   sanitizer_symbolizer_internal.h
   sanitizer_symbolizer_libbacktrace.h
   sanitizer_symbolizer_mac.h
+  sanitizer_symbolizer_rtems.h
   sanitizer_syscall_generic.inc
   sanitizer_syscall_linux_x86_64.inc
   sanitizer_syscall_linux_aarch64.inc
   sanitizer_syscalls_netbsd.inc
   sanitizer_thread_registry.h
   sanitizer_vector.h
   sanitizer_win.h)
 

lib/sanitizer_common/sanitizer_platform.h

 // Older gcc have issues aligning to a constexpr, and require an integer.
 // See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56859 among others.
 #if defined(__powerpc__) || defined(__powerpc64__)
 # define SANITIZER_CACHE_LINE_SIZE 128
 #else
 # define SANITIZER_CACHE_LINE_SIZE 64
 #endif
 
+// Enable offline markup symbolizer for Fuchsia and RTEMS.
+#if SANITIZER_FUCHSIA || SANITIZER_RTEMS
+#define SANITIZER_SYMBOLIZER_MARKUP 1
+#else
+#define SANITIZER_SYMBOLIZER_MARKUP 0
+#endif
+
 #endif // SANITIZER_PLATFORM_H

lib/sanitizer_common/sanitizer_symbolizer_fuchsia.h

+//===-- sanitizer_symbolizer_fuchsia.h -----------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file is shared between various sanitizers' runtime libraries.
+//
+// Define Fuchsia's string formats and limits for the markup symbolizer.
+//===----------------------------------------------------------------------===//
+#ifndef SANITIZER_SYMBOLIZER_FUCHSIA_H
+#define SANITIZER_SYMBOLIZER_FUCHSIA_H
+
+#include "sanitizer_internal_defs.h"
+
+namespace __sanitizer {
+
+// See the spec at:
+// https://fuchsia.googlesource.com/zircon/+/master/docs/symbolizer_markup.md
+
+// 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.
+
+// Function name or equivalent from PC location.
+constexpr const char *kFormatFunction = "{{{pc:%p}}}";
+constexpr uptr kFormatFunctionMax = 64;  // More than big enough for 64-bit hex.
+
+// Global variable name or equivalent from data memory address.
+constexpr const char *kFormatData = "{{{data:%p}}}";
+
+// One frame in a backtrace (printed on a line by itself).
+constexpr const char *kFormatFrame = "{{{bt:%u:%p}}}";
+
+}  // namespace __sanitizer
+
+#endif  // SANITIZER_SYMBOLIZER_FUCHSIA_H

lib/sanitizer_common/sanitizer_symbolizer_fuchsia.cc

-//===-- sanitizer_symbolizer_fuchsia.cc -----------------------------------===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-//
-// This file is shared between various sanitizers' runtime libraries.
-//
-// Implementation of Fuchsia-specific symbolizer.
-//===----------------------------------------------------------------------===//
-
-#include "sanitizer_platform.h"
-#if SANITIZER_FUCHSIA
-
-#include "sanitizer_fuchsia.h"
-#include "sanitizer_stacktrace.h"
-#include "sanitizer_symbolizer.h"
-
-#include <limits.h>
-#include <unwind.h>
-
-namespace __sanitizer {
-
-// For Fuchsia we don't do any actual symbolization per se.
-// Instead, we emit text containing raw addresses and raw linkage
-// symbol names, embedded in Fuchsia's symbolization markup format.
-// Fuchsia's logging infrastructure emits enough information about
-// process memory layout that a post-processing filter can do the
-// symbolization and pretty-print the markup.  See the spec at:
-// https://fuchsia.googlesource.com/zircon/+/master/docs/symbolizer_markup.md
-
-// 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.
-
-// Function name or equivalent from PC location.
-constexpr const char *kFormatFunction = "{{{pc:%p}}}";
-constexpr uptr kFormatFunctionMax = 64;  // More than big enough for 64-bit hex.
-
-// Global variable name or equivalent from data memory address.
-constexpr const char *kFormatData = "{{{data:%p}}}";
-
-// One frame in a backtrace (printed on a line by itself).
-constexpr const char *kFormatFrame = "{{{bt:%u:%p}}}";
-
-// This is used by UBSan for type names, and by ASan for global variable names.
-// It's expected to return a static buffer that will be reused on each call.
-const char *Symbolizer::Demangle(const char *name) {
-  static char buffer[kFormatDemangleMax];
-  internal_snprintf(buffer, sizeof(buffer), kFormatDemangle, name);
-  return buffer;
-}
-
-// This is used mostly for suppression matching.  Making it work
-// would enable "interceptor_via_lib" suppressions.  It's also used
-// once in UBSan to say "in module ..." in a message that also
-// includes an address in the module, so post-processing can already
-// pretty-print that so as to indicate the module.
-bool Symbolizer::GetModuleNameAndOffsetForPC(uptr pc, const char **module_name,
-                                             uptr *module_address) {
-  return false;
-}
-
-// This is used in some places for suppression checking, which we
-// don't really support for Fuchsia.  It's also used in UBSan to
-// identify a PC location to a function name, so we always fill in
-// the function member with a string containing markup around the PC
-// value.
-// TODO(mcgrathr): Under SANITIZER_GO, it's currently used by TSan
-// to render stack frames, but that should be changed to use
-// RenderStackFrame.
-SymbolizedStack *Symbolizer::SymbolizePC(uptr addr) {
-  SymbolizedStack *s = SymbolizedStack::New(addr);
-  char buffer[kFormatFunctionMax];
-  internal_snprintf(buffer, sizeof(buffer), kFormatFunction, addr);
-  s->info.function = internal_strdup(buffer);
-  return s;
-}
-
-// Always claim we succeeded, so that RenderDataInfo will be called.
-bool Symbolizer::SymbolizeData(uptr addr, DataInfo *info) {
-  info->Clear();
-  info->start = addr;
-  return true;
-}
-
-// We ignore the format argument to __sanitizer_symbolize_global.
-void RenderData(InternalScopedString *buffer, const char *format,
-                const DataInfo *DI, const char *strip_path_prefix) {
-  buffer->append(kFormatData, DI->start);
-}
-
-// We don't support the stack_trace_format flag at all.
-void RenderFrame(InternalScopedString *buffer, const char *format, int frame_no,
-                 const AddressInfo &info, bool vs_style,
-                 const char *strip_path_prefix, const char *strip_func_prefix) {
-  buffer->append(kFormatFrame, frame_no, info.address);
-}
-
-Symbolizer *Symbolizer::PlatformInit() {
-  return new (symbolizer_allocator_) Symbolizer({});
-}
-
-void Symbolizer::LateInitialize() { Symbolizer::GetOrInit(); }
-
-void StartReportDeadlySignal() {}
-void ReportDeadlySignal(const SignalContext &sig, u32 tid,
-                        UnwindSignalStackCallbackType unwind,
-                        const void *unwind_context) {}
-
-struct UnwindTraceArg {
-  BufferedStackTrace *stack;
-  u32 max_depth;
-};
-
-_Unwind_Reason_Code Unwind_Trace(struct _Unwind_Context *ctx, void *param) {
-  UnwindTraceArg *arg = static_cast<UnwindTraceArg *>(param);
-  CHECK_LT(arg->stack->size, arg->max_depth);
-  uptr pc = _Unwind_GetIP(ctx);
-  if (pc < PAGE_SIZE) return _URC_NORMAL_STOP;
-  arg->stack->trace_buffer[arg->stack->size++] = pc;
-  return (arg->stack->size == arg->max_depth ? _URC_NORMAL_STOP
-                                             : _URC_NO_REASON);
-}
-
-void BufferedStackTrace::SlowUnwindStack(uptr pc, u32 max_depth) {
-  CHECK_GE(max_depth, 2);
-  size = 0;
-  UnwindTraceArg arg = {this, Min(max_depth + 1, kStackTraceMax)};
-  _Unwind_Backtrace(Unwind_Trace, &arg);
-  CHECK_GT(size, 0);
-  // We need to pop a few frames so that pc is on top.
-  uptr to_pop = LocatePcInTrace(pc);
-  // trace_buffer[0] belongs to the current function so we always pop it,
-  // unless there is only 1 frame in the stack trace (1 frame is always better
-  // than 0!).
-  PopStackFrames(Min(to_pop, static_cast<uptr>(1)));
-  trace_buffer[0] = pc;
-}
-
-void BufferedStackTrace::SlowUnwindStackWithContext(uptr pc, void *context,
-                                                    u32 max_depth) {
-  CHECK_NE(context, nullptr);
-  UNREACHABLE("signal context doesn't exist");
-}
-
-}  // namespace __sanitizer
-
-#endif  // SANITIZER_FUCHSIA

lib/sanitizer_common/sanitizer_symbolizer_markup.cc

+//===-- sanitizer_symbolizer_markup.cc ------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file is shared between various sanitizers' runtime libraries.
+//
+// Implementation of offline markup symbolizer.
+//===----------------------------------------------------------------------===//
+
+#include "sanitizer_platform.h"
+#if SANITIZER_SYMBOLIZER_MARKUP
+
+#if SANITIZER_FUCHSIA
+#include "sanitizer_symbolizer_fuchsia.h"
+#elif SANITIZER_RTEMS
+#include "sanitizer_symbolizer_rtems.h"
+#endif
+#include "sanitizer_stacktrace.h"
+#include "sanitizer_symbolizer.h"
+
+#include <limits.h>
+#include <unwind.h>
+
+namespace __sanitizer {
+
+// This generic support for offline symbolizing is based on the
+// Fuchsia port.  We don't do any actual symbolization per se.
+// Instead, we emit text containing raw addresses and raw linkage
+// symbol names, embedded in Fuchsia's symbolization markup format.
+// Fuchsia's logging infrastructure emits enough information about
+// process memory layout that a post-processing filter can do the
+// symbolization and pretty-print the markup.  See the spec at:
+// https://fuchsia.googlesource.com/zircon/+/master/docs/symbolizer_markup.md
+
+// This is used by UBSan for type names, and by ASan for global variable names.
+// It's expected to return a static buffer that will be reused on each call.
+const char *Symbolizer::Demangle(const char *name) {
+  static char buffer[kFormatDemangleMax];
+  internal_snprintf(buffer, sizeof(buffer), kFormatDemangle, name);
+  return buffer;
+}
+
+// This is used mostly for suppression matching.  Making it work
+// would enable "interceptor_via_lib" suppressions.  It's also used
+// once in UBSan to say "in module ..." in a message that also
+// includes an address in the module, so post-processing can already
+// pretty-print that so as to indicate the module.
+bool Symbolizer::GetModuleNameAndOffsetForPC(uptr pc, const char **module_name,
+                                             uptr *module_address) {
+  return false;
+}
+
+// This is used in some places for suppression checking, which we
+// don't really support for Fuchsia.  It's also used in UBSan to
+// identify a PC location to a function name, so we always fill in
+// the function member with a string containing markup around the PC
+// value.
+// TODO(mcgrathr): Under SANITIZER_GO, it's currently used by TSan
+// to render stack frames, but that should be changed to use
+// RenderStackFrame.
+SymbolizedStack *Symbolizer::SymbolizePC(uptr addr) {
+  SymbolizedStack *s = SymbolizedStack::New(addr);
+  char buffer[kFormatFunctionMax];
+  internal_snprintf(buffer, sizeof(buffer), kFormatFunction, addr);
+  s->info.function = internal_strdup(buffer);
+  return s;
+}
+
+// Always claim we succeeded, so that RenderDataInfo will be called.
+bool Symbolizer::SymbolizeData(uptr addr, DataInfo *info) {
+  info->Clear();
+  info->start = addr;
+  return true;
+}
+
+// We ignore the format argument to __sanitizer_symbolize_global.
+void RenderData(InternalScopedString *buffer, const char *format,
+                const DataInfo *DI, const char *strip_path_prefix) {
+  buffer->append(kFormatData, DI->start);
+}
+
+// We don't support the stack_trace_format flag at all.
+void RenderFrame(InternalScopedString *buffer, const char *format, int frame_no,
+                 const AddressInfo &info, bool vs_style,
+                 const char *strip_path_prefix, const char *strip_func_prefix) {
+  buffer->append(kFormatFrame, frame_no, info.address);
+}
+
+Symbolizer *Symbolizer::PlatformInit() {
+  return new (symbolizer_allocator_) Symbolizer({});
+}
+
+void Symbolizer::LateInitialize() { Symbolizer::GetOrInit(); }
+
+void StartReportDeadlySignal() {}
+void ReportDeadlySignal(const SignalContext &sig, u32 tid,
+                        UnwindSignalStackCallbackType unwind,
+                        const void *unwind_context) {}
+
+#if SANITIZER_CAN_SLOW_UNWIND
+struct UnwindTraceArg {
+  BufferedStackTrace *stack;
+  u32 max_depth;
+};
+
+_Unwind_Reason_Code Unwind_Trace(struct _Unwind_Context *ctx, void *param) {
+  UnwindTraceArg *arg = static_cast<UnwindTraceArg *>(param);
+  CHECK_LT(arg->stack->size, arg->max_depth);
+  uptr pc = _Unwind_GetIP(ctx);
+  if (pc < PAGE_SIZE) return _URC_NORMAL_STOP;
+  arg->stack->trace_buffer[arg->stack->size++] = pc;
+  return (arg->stack->size == arg->max_depth ? _URC_NORMAL_STOP
+                                             : _URC_NO_REASON);
+}
+
+void BufferedStackTrace::SlowUnwindStack(uptr pc, u32 max_depth) {
+  CHECK_GE(max_depth, 2);
+  size = 0;
+  UnwindTraceArg arg = {this, Min(max_depth + 1, kStackTraceMax)};
+  _Unwind_Backtrace(Unwind_Trace, &arg);
+  CHECK_GT(size, 0);
+  // We need to pop a few frames so that pc is on top.
+  uptr to_pop = LocatePcInTrace(pc);
+  // trace_buffer[0] belongs to the current function so we always pop it,
+  // unless there is only 1 frame in the stack trace (1 frame is always better
+  // than 0!).
+  PopStackFrames(Min(to_pop, static_cast<uptr>(1)));
+  trace_buffer[0] = pc;
+}
+
+void BufferedStackTrace::SlowUnwindStackWithContext(uptr pc, void *context,
+                                                    u32 max_depth) {
+  CHECK_NE(context, nullptr);
+  UNREACHABLE("signal context doesn't exist");
+}
+#endif  // SANITIZER_CAN_SLOW_UNWIND
+
+}  // namespace __sanitizer
+
+#endif  // SANITIZER_SYMBOLIZER_MARKUP

lib/sanitizer_common/sanitizer_symbolizer_rtems.h

+//===-- sanitizer_symbolizer_rtems.h -----------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file is shared between various sanitizers' runtime libraries.
+//
+// Define RTEMS's string formats and limits for the markup symbolizer.
+//===----------------------------------------------------------------------===//
+#ifndef SANITIZER_SYMBOLIZER_RTEMS_H
+#define SANITIZER_SYMBOLIZER_RTEMS_H
+
+#include "sanitizer_internal_defs.h"
+
+namespace __sanitizer {
+
+// The Myriad RTEMS symbolizer currently only parses backtrace lines,
+// so use a format that the symbolizer understands.  For other
+// markups, keep them the same as the Fuchsia's.
+
+// 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.
+
+// Function name or equivalent from PC location.
+constexpr const char *kFormatFunction = "{{{pc:%p}}}";
+constexpr uptr kFormatFunctionMax = 64;  // More than big enough for 64-bit hex.
+
+// Global variable name or equivalent from data memory address.
+constexpr const char *kFormatData = "{{{data:%p}}}";
+
+// One frame in a backtrace (printed on a line by itself).
+constexpr const char *kFormatFrame = "    [%u] IP: %p";
+
+}  // namespace __sanitizer
+
+#endif  // SANITIZER_SYMBOLIZER_RTEMS_H