[compiler-rt] Reland: [sanitizer_symbolizer] Add StackTracePrinter virtual class (PR #66689)

via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 18 12:26:04 PDT 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-compiler-rt-sanitizer

<details>
<summary>Changes</summary>

Introduce a new virtual class StackTracePrinter and an implementation
FormattedStackTracePrinter in preparation of enabling symbolizer markup
for linux.
This change allows us to implement other behaviour under the same api
for StackTracePrinter, for example, MarkupStackTracePrinter.

Reason for revert: A missing header file for the sanitizer_symbolizer_markup.cpp files. 
This was not caught in local builds or pre-merge checks given that to trigger the error, the code
has to be compiled for Fuchsia.
For this reland I've build for the fuchsia targets as well as linux.

---

Patch is 30.19 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/66689.diff


10 Files Affected:

- (modified) compiler-rt/lib/hwasan/hwasan_report.cpp (+4-3) 
- (modified) compiler-rt/lib/msan/msan_report.cpp (+1-1) 
- (modified) compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp (+8-6) 
- (modified) compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp (+33-13) 
- (modified) compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h (+92-51) 
- (modified) compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp (+14-6) 
- (modified) compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp (+3-3) 
- (modified) compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_printer_test.cpp (+51-40) 
- (modified) compiler-rt/lib/tsan/rtl/tsan_report.cpp (+4-4) 
- (modified) compiler-rt/lib/ubsan/ubsan_diag.cpp (+11-8) 


``````````diff
diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index d318e26c6a86849..85702366d22202e 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -242,9 +242,10 @@ static void PrintStackAllocations(StackAllocationsRingBuffer *sa,
     frame_desc.AppendF("  record_addr:0x%zx record:0x%zx",
                        reinterpret_cast<uptr>(record_addr), record);
     if (SymbolizedStack *frame = Symbolizer::GetOrInit()->SymbolizePC(pc)) {
-      RenderFrame(&frame_desc, " %F %L", 0, frame->info.address, &frame->info,
-                  common_flags()->symbolize_vs_style,
-                  common_flags()->strip_path_prefix);
+      StackTracePrinter::GetOrInit()->RenderFrame(
+          &frame_desc, " %F %L", 0, frame->info.address, &frame->info,
+          common_flags()->symbolize_vs_style,
+          common_flags()->strip_path_prefix);
       frame->ClearAll();
     }
     Printf("%s\n", frame_desc.data());
diff --git a/compiler-rt/lib/msan/msan_report.cpp b/compiler-rt/lib/msan/msan_report.cpp
index 90164e50ca3acca..99bf81f66dc9e2c 100644
--- a/compiler-rt/lib/msan/msan_report.cpp
+++ b/compiler-rt/lib/msan/msan_report.cpp
@@ -269,7 +269,7 @@ void DescribeMemoryRange(const void *x, uptr size) {
 
 void ReportUMRInsideAddressRange(const char *function, const void *start,
                                  uptr size, uptr offset) {
-  function = StripFunctionName(function);
+  function = StackTracePrinter::GetOrInit()->StripFunctionName(function);
   Decorator d;
   Printf("%s", d.Warning());
   Printf("%sUninitialized bytes in %s%s%s at offset %zu inside [%p, %zu)%s\n",
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp
index efbf7d9e450c63f..9a4c80fcfdd199c 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp
@@ -29,7 +29,8 @@ class StackTraceTextPrinter {
         frame_delimiter_(frame_delimiter),
         output_(output),
         dedup_token_(dedup_token),
-        symbolize_(RenderNeedsSymbolization(stack_trace_fmt)) {}
+        symbolize_(StackTracePrinter::GetOrInit()->RenderNeedsSymbolization(
+            stack_trace_fmt)) {}
 
   bool ProcessAddressFrames(uptr pc) {
     SymbolizedStack *frames = symbolize_
@@ -40,10 +41,10 @@ class StackTraceTextPrinter {
 
     for (SymbolizedStack *cur = frames; cur; cur = cur->next) {
       uptr prev_len = output_->length();
-      RenderFrame(output_, stack_trace_fmt_, frame_num_++, cur->info.address,
-                  symbolize_ ? &cur->info : nullptr,
-                  common_flags()->symbolize_vs_style,
-                  common_flags()->strip_path_prefix);
+      StackTracePrinter::GetOrInit()->RenderFrame(
+          output_, stack_trace_fmt_, frame_num_++, cur->info.address,
+          symbolize_ ? &cur->info : nullptr, common_flags()->symbolize_vs_style,
+          common_flags()->strip_path_prefix);
 
       if (prev_len != output_->length())
         output_->AppendF("%c", frame_delimiter_);
@@ -210,7 +211,8 @@ void __sanitizer_symbolize_global(uptr data_addr, const char *fmt,
   DataInfo DI;
   if (!Symbolizer::GetOrInit()->SymbolizeData(data_addr, &DI)) return;
   InternalScopedString data_desc;
-  RenderData(&data_desc, fmt, &DI, common_flags()->strip_path_prefix);
+  StackTracePrinter::GetOrInit()->RenderData(&data_desc, fmt, &DI,
+                                             common_flags()->strip_path_prefix);
   internal_strncpy(out_buf, data_desc.data(), out_buf_size);
   out_buf[out_buf_size - 1] = 0;
 }
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
index 656f78a6186f44e..8db321051e1a0de 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
@@ -18,7 +18,22 @@
 
 namespace __sanitizer {
 
-const char *StripFunctionName(const char *function) {
+StackTracePrinter *StackTracePrinter::GetOrInit() {
+  static StackTracePrinter *stacktrace_printer;
+  static StaticSpinMutex init_mu;
+  SpinMutexLock l(&init_mu);
+  if (stacktrace_printer)
+    return stacktrace_printer;
+
+  stacktrace_printer =
+      new (GetGlobalLowLevelAllocator()) FormattedStackTracePrinter();
+
+  CHECK(stacktrace_printer);
+  return stacktrace_printer;
+}
+
+const char *FormattedStackTracePrinter::StripFunctionName(
+    const char *function) {
   if (!common_flags()->demangle)
     return function;
   if (!function)
@@ -141,9 +156,12 @@ static void MaybeBuildIdToBuffer(const AddressInfo &info, bool PrefixSpace,
 
 static const char kDefaultFormat[] = "    #%n %p %F %L";
 
-void RenderFrame(InternalScopedString *buffer, const char *format, int frame_no,
-                 uptr address, const AddressInfo *info, bool vs_style,
-                 const char *strip_path_prefix) {
+void FormattedStackTracePrinter::RenderFrame(InternalScopedString *buffer,
+                                             const char *format, int frame_no,
+                                             uptr address,
+                                             const AddressInfo *info,
+                                             bool vs_style,
+                                             const char *strip_path_prefix) {
   // info will be null in the case where symbolization is not needed for the
   // given format. This ensures that the code below will get a hard failure
   // rather than print incorrect information in case RenderNeedsSymbolization
@@ -250,7 +268,7 @@ void RenderFrame(InternalScopedString *buffer, const char *format, int frame_no,
   }
 }
 
-bool RenderNeedsSymbolization(const char *format) {
+bool FormattedStackTracePrinter::RenderNeedsSymbolization(const char *format) {
   if (0 == internal_strcmp(format, "DEFAULT"))
     format = kDefaultFormat;
   for (const char *p = format; *p != '\0'; p++) {
@@ -273,8 +291,10 @@ bool RenderNeedsSymbolization(const char *format) {
   return false;
 }
 
-void RenderData(InternalScopedString *buffer, const char *format,
-                const DataInfo *DI, const char *strip_path_prefix) {
+void FormattedStackTracePrinter::RenderData(InternalScopedString *buffer,
+                                            const char *format,
+                                            const DataInfo *DI,
+                                            const char *strip_path_prefix) {
   for (const char *p = format; *p != '\0'; p++) {
     if (*p != '%') {
       buffer->AppendF("%c", *p);
@@ -304,9 +324,9 @@ void RenderData(InternalScopedString *buffer, const char *format,
 
 #endif  // !SANITIZER_SYMBOLIZER_MARKUP
 
-void RenderSourceLocation(InternalScopedString *buffer, const char *file,
-                          int line, int column, bool vs_style,
-                          const char *strip_path_prefix) {
+void FormattedStackTracePrinter::RenderSourceLocation(
+    InternalScopedString *buffer, const char *file, int line, int column,
+    bool vs_style, const char *strip_path_prefix) {
   if (vs_style && line > 0) {
     buffer->AppendF("%s(%d", StripPathPrefix(file, strip_path_prefix), line);
     if (column > 0)
@@ -323,9 +343,9 @@ void RenderSourceLocation(InternalScopedString *buffer, const char *file,
   }
 }
 
-void RenderModuleLocation(InternalScopedString *buffer, const char *module,
-                          uptr offset, ModuleArch arch,
-                          const char *strip_path_prefix) {
+void FormattedStackTracePrinter::RenderModuleLocation(
+    InternalScopedString *buffer, const char *module, uptr offset,
+    ModuleArch arch, const char *strip_path_prefix) {
   buffer->AppendF("(%s", StripPathPrefix(module, strip_path_prefix));
   if (arch != kModuleArchUnknown) {
     buffer->AppendF(":%s", ModuleArchToString(arch));
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
index bf2755a2e8f458d..3e02333a8c8d5cc 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
@@ -13,61 +13,102 @@
 #define SANITIZER_STACKTRACE_PRINTER_H
 
 #include "sanitizer_common.h"
+#include "sanitizer_internal_defs.h"
 #include "sanitizer_symbolizer.h"
 
 namespace __sanitizer {
 
-// Strip interceptor prefixes from function name.
-const char *StripFunctionName(const char *function);
-
-// Render the contents of "info" structure, which represents the contents of
-// stack frame "frame_no" and appends it to the "buffer". "format" is a
-// string with placeholders, which is copied to the output with
-// placeholders substituted with the contents of "info". For example,
-// format string
-//   "  frame %n: function %F at %S"
-// will be turned into
-//   "  frame 10: function foo::bar() at my/file.cc:10"
-// You may additionally pass "strip_path_prefix" to strip prefixes of paths to
-// source files and modules.
-// Here's the full list of available placeholders:
-//   %% - represents a '%' character;
-//   %n - frame number (copy of frame_no);
-//   %p - PC in hex format;
-//   %m - path to module (binary or shared object);
-//   %o - offset in the module in hex format;
-//   %f - function name;
-//   %q - offset in the function in hex format (*if available*);
-//   %s - path to source file;
-//   %l - line in the source file;
-//   %c - column in the source file;
-//   %F - if function is known to be <foo>, prints "in <foo>", possibly
-//        followed by the offset in this function, but only if source file
-//        is unknown;
-//   %S - prints file/line/column information;
-//   %L - prints location information: file/line/column, if it is known, or
-//        module+offset if it is known, or (<unknown module>) string.
-//   %M - prints module basename and offset, if it is known, or PC.
-void RenderFrame(InternalScopedString *buffer, const char *format, int frame_no,
-                 uptr address, const AddressInfo *info, bool vs_style,
-                 const char *strip_path_prefix = "");
-
-bool RenderNeedsSymbolization(const char *format);
-
-void RenderSourceLocation(InternalScopedString *buffer, const char *file,
-                          int line, int column, bool vs_style,
-                          const char *strip_path_prefix);
-
-void RenderModuleLocation(InternalScopedString *buffer, const char *module,
-                          uptr offset, ModuleArch arch,
-                          const char *strip_path_prefix);
-
-// Same as RenderFrame, but for data section (global variables).
-// Accepts %s, %l from above.
-// Also accepts:
-//   %g - name of the global variable.
-void RenderData(InternalScopedString *buffer, const char *format,
-                const DataInfo *DI, const char *strip_path_prefix = "");
+// StacktracePrinter is an interface that is implemented by
+// classes that can perform rendering of the different parts
+// of a stacktrace.
+class StackTracePrinter {
+ public:
+  static StackTracePrinter *GetOrInit();
+
+  virtual const char *StripFunctionName(const char *function) = 0;
+
+  virtual void RenderFrame(InternalScopedString *buffer, const char *format,
+                           int frame_no, uptr address, const AddressInfo *info,
+                           bool vs_style,
+                           const char *strip_path_prefix = "") = 0;
+
+  virtual bool RenderNeedsSymbolization(const char *format) = 0;
+
+  virtual void RenderSourceLocation(InternalScopedString *buffer,
+                                    const char *file, int line, int column,
+                                    bool vs_style,
+                                    const char *strip_path_prefix) = 0;
+
+  virtual void RenderModuleLocation(InternalScopedString *buffer,
+                                    const char *module, uptr offset,
+                                    ModuleArch arch,
+                                    const char *strip_path_prefix) = 0;
+  virtual void RenderData(InternalScopedString *buffer, const char *format,
+                          const DataInfo *DI,
+                          const char *strip_path_prefix = "") = 0;
+
+ protected:
+  ~StackTracePrinter() {}
+};
+
+class FormattedStackTracePrinter : public StackTracePrinter {
+ public:
+  // Strip interceptor prefixes from function name.
+  const char *StripFunctionName(const char *function) override;
+
+  // Render the contents of "info" structure, which represents the contents of
+  // stack frame "frame_no" and appends it to the "buffer". "format" is a
+  // string with placeholders, which is copied to the output with
+  // placeholders substituted with the contents of "info". For example,
+  // format string
+  //   "  frame %n: function %F at %S"
+  // will be turned into
+  //   "  frame 10: function foo::bar() at my/file.cc:10"
+  // You may additionally pass "strip_path_prefix" to strip prefixes of paths to
+  // source files and modules.
+  // Here's the full list of available placeholders:
+  //   %% - represents a '%' character;
+  //   %n - frame number (copy of frame_no);
+  //   %p - PC in hex format;
+  //   %m - path to module (binary or shared object);
+  //   %o - offset in the module in hex format;
+  //   %f - function name;
+  //   %q - offset in the function in hex format (*if available*);
+  //   %s - path to source file;
+  //   %l - line in the source file;
+  //   %c - column in the source file;
+  //   %F - if function is known to be <foo>, prints "in <foo>", possibly
+  //        followed by the offset in this function, but only if source file
+  //        is unknown;
+  //   %S - prints file/line/column information;
+  //   %L - prints location information: file/line/column, if it is known, or
+  //        module+offset if it is known, or (<unknown module>) string.
+  //   %M - prints module basename and offset, if it is known, or PC.
+  void RenderFrame(InternalScopedString *buffer, const char *format,
+                   int frame_no, uptr address, const AddressInfo *info,
+                   bool vs_style, const char *strip_path_prefix = "") override;
+
+  bool RenderNeedsSymbolization(const char *format) override;
+
+  void RenderSourceLocation(InternalScopedString *buffer, const char *file,
+                            int line, int column, bool vs_style,
+                            const char *strip_path_prefix) override;
+
+  void RenderModuleLocation(InternalScopedString *buffer, const char *module,
+                            uptr offset, ModuleArch arch,
+                            const char *strip_path_prefix) override;
+
+  // Same as RenderFrame, but for data section (global variables).
+  // Accepts %s, %l from above.
+  // Also accepts:
+  //   %g - name of the global variable.
+  void RenderData(InternalScopedString *buffer, const char *format,
+                  const DataInfo *DI,
+                  const char *strip_path_prefix = "") override;
+
+ protected:
+  ~FormattedStackTracePrinter() {}
+};
 
 }  // namespace __sanitizer
 
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
index e56af63a4397056..d1b0f46004efaff 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
@@ -22,6 +22,7 @@
 #  include <unwind.h>
 
 #  include "sanitizer_stacktrace.h"
+#  include "sanitizer_stacktrace_printer.h"
 #  include "sanitizer_symbolizer.h"
 
 namespace __sanitizer {
@@ -81,17 +82,24 @@ bool Symbolizer::SymbolizeData(uptr addr, DataInfo *info) {
 }
 
 // We ignore the format argument to __sanitizer_symbolize_global.
-void RenderData(InternalScopedString *buffer, const char *format,
-                const DataInfo *DI, const char *strip_path_prefix) {
+void FormattedStackTracePrinter::RenderData(InternalScopedString *buffer,
+                                            const char *format,
+                                            const DataInfo *DI,
+                                            const char *strip_path_prefix) {
   buffer->AppendF(kFormatData, DI->start);
 }
 
-bool RenderNeedsSymbolization(const char *format) { return false; }
+bool FormattedStackTracePrinter::RenderNeedsSymbolization(const char *format) {
+  return false;
+}
 
 // We don't support the stack_trace_format flag at all.
-void RenderFrame(InternalScopedString *buffer, const char *format, int frame_no,
-                 uptr address, const AddressInfo *info, bool vs_style,
-                 const char *strip_path_prefix) {
+void FormattedStackTracePrinter::RenderFrame(InternalScopedString *buffer,
+                                             const char *format, int frame_no,
+                                             uptr address,
+                                             const AddressInfo *info,
+                                             bool vs_style,
+                                             const char *strip_path_prefix) {
   CHECK(!RenderNeedsSymbolization(format));
   buffer->AppendF(kFormatFrame, frame_no, address);
 }
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
index d2196ecf8f261ab..3e4417ae3f57e57 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
@@ -33,9 +33,9 @@ void ReportErrorSummary(const char *error_type, const AddressInfo &info,
   if (!common_flags()->print_summary) return;
   InternalScopedString buff;
   buff.AppendF("%s ", error_type);
-  RenderFrame(&buff, "%L %F", 0, info.address, &info,
-              common_flags()->symbolize_vs_style,
-              common_flags()->strip_path_prefix);
+  StackTracePrinter::GetOrInit()->RenderFrame(
+      &buff, "%L %F", 0, info.address, &info,
+      common_flags()->symbolize_vs_style, common_flags()->strip_path_prefix);
   ReportErrorSummary(buff.data(), alt_tool_name);
 }
 #endif
diff --git a/compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_printer_test.cpp b/compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_printer_test.cpp
index 489ef4dbb5b7d4c..9602ba31621e647 100644
--- a/compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_printer_test.cpp
+++ b/compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_printer_test.cpp
@@ -16,57 +16,68 @@
 
 namespace __sanitizer {
 
-TEST(SanitizerStacktracePrinter, RenderSourceLocation) {
+class TestFormattedStackTracePrinter final : public FormattedStackTracePrinter {
+ public:
+  ~TestFormattedStackTracePrinter() {}
+};
+
+TEST(FormattedStackTracePrinter, RenderSourceLocation) {
   InternalScopedString str;
-  RenderSourceLocation(&str, "/dir/file.cc", 10, 5, false, "");
+  TestFormattedStackTracePrinter printer;
+
+  printer.RenderSourceLocation(&str, "/dir/file.cc", 10, 5, false, "");
   EXPECT_STREQ("/dir/file.cc:10:5", str.data());
 
   str.clear();
-  RenderSourceLocation(&str, "/dir/file.cc", 11, 0, false, "");
+  printer.RenderSourceLocation(&str, "/dir/file.cc", 11, 0, false, "");
   EXPECT_STREQ("/dir/file.cc:11", str.data());
 
   str.clear();
-  RenderSourceLocation(&str, "/dir/file.cc", 0, 0, false, "");
+  printer.RenderSourceLocation(&str, "/dir/file.cc", 0, 0, false, "");
   EXPECT_STREQ("/dir/file.cc", str.data());
 
   str.clear();
-  RenderSourceLocation(&str, "/dir/file.cc", 10, 5, false, "/dir/");
+  printer.RenderSourceLocation(&str, "/dir/file.cc", 10, 5, false, "/dir/");
   EXPECT_STREQ("file.cc:10:5", str.data());
 
   str.clear();
-  RenderSourceLocation(&str, "/dir/file.cc", 10, 5, true, "");
+  printer.RenderSourceLocation(&str, "/dir/file.cc", 10, 5, true, "");
   EXPECT_STREQ("/dir/file.cc(10,5)", str.data());
 
   str.clear();
-  RenderSourceLocation(&str, "/dir/file.cc", 11, 0, true, "");
+  printer.RenderSourceLocation(&str, "/dir/file.cc", 11, 0, true, "");
   EXPECT_STREQ("/dir/file.cc(11)", str.data());
 
   str.clear();
-  RenderSourceLocation(&str, "/dir/file.cc", 0, 0, true, "");
+  printer.RenderSourceLocation(&str, "/dir/file.cc", 0, 0, true, "");
   EXPECT_STREQ("/dir/file.cc", str.data());
 
   ...
[truncated]

``````````

</details>


https://github.com/llvm/llvm-project/pull/66689


More information about the llvm-commits mailing list