[compiler-rt] [NFC][sanitizer_symbolizer]Add StackTracePrinter class (PR #66530)

Andres Villegas via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 15 13:36:33 PDT 2023


=?utf-8?q?Andrés?= Villegas <andresvi at google.com>
Message-ID:
In-Reply-To: <llvm/llvm-project/pull/66530/compiler-rt at github.com>


https://github.com/avillega updated https://github.com/llvm/llvm-project/pull/66530

>From bbcea94a6c27186357680ea9d608bd153dfaadc9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9s=20Villegas?= <andresvi at google.com>
Date: Fri, 15 Sep 2023 16:46:24 +0000
Subject: [PATCH 1/2] Add StackTracePrinter class and impl

---
 compiler-rt/lib/hwasan/hwasan_report.cpp      |   7 +-
 compiler-rt/lib/msan/msan_report.cpp          |   2 +-
 .../sanitizer_stacktrace_libcdep.cpp          |  14 +-
 .../sanitizer_stacktrace_printer.cpp          |  46 ++++--
 .../sanitizer_stacktrace_printer.h            | 152 ++++++++++++------
 .../sanitizer_symbolizer_markup.cpp           |  19 ++-
 .../sanitizer_symbolizer_report.cpp           |   6 +-
 .../sanitizer_stacktrace_printer_test.cpp     |  87 +++++-----
 compiler-rt/lib/tsan/rtl/tsan_report.cpp      |   8 +-
 compiler-rt/lib/ubsan/ubsan_diag.cpp          |  19 ++-
 10 files changed, 225 insertions(+), 135 deletions(-)

diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index efe6f57704919a4..5b6920f7da30b7d 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -246,9 +246,10 @@ static void PrintStackAllocations(StackAllocationsRingBuffer *sa,
     frame_desc.append("  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 47983ee7ec713f2..2a0417ed96de1b9 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_->append("%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 45c480d225c7f5a..e211b5c20b4be1e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
@@ -12,13 +12,28 @@
 
 #include "sanitizer_stacktrace_printer.h"
 
+#include "sanitizer_common.h"
 #include "sanitizer_file.h"
 #include "sanitizer_flags.h"
 #include "sanitizer_fuchsia.h"
 
 namespace __sanitizer {
 
-const char *StripFunctionName(const char *function) {
+StackTracePrinter *StackTracePrinter::stacktrace_printer_;
+StaticSpinMutex StackTracePrinter::init_mu_;
+
+StackTracePrinter *StackTracePrinter::GetOrInit() {
+  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->append("%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->append("%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->append("(%s", StripPathPrefix(module, strip_path_prefix));
   if (arch != kModuleArchUnknown) {
     buffer->append(":%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..a58fd557b5cc17c 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
@@ -13,61 +13,111 @@
 #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) {
+    UNIMPLEMENTED();
+  }
+
+  virtual void RenderFrame(InternalScopedString *buffer, const char *format,
+                           int frame_no, uptr address, const AddressInfo *info,
+                           bool vs_style, const char *strip_path_prefix = "") {
+    UNIMPLEMENTED();
+  }
+
+  virtual bool RenderNeedsSymbolization(const char *format) { return false; }
+
+  virtual void RenderSourceLocation(InternalScopedString *buffer,
+                                    const char *file, int line, int column,
+                                    bool vs_style,
+                                    const char *strip_path_prefix) {}
+
+  virtual void RenderModuleLocation(InternalScopedString *buffer,
+                                    const char *module, uptr offset,
+                                    ModuleArch arch,
+                                    const char *strip_path_prefix) {}
+  virtual void RenderData(InternalScopedString *buffer, const char *format,
+                          const DataInfo *DI,
+                          const char *strip_path_prefix = "") {
+    UNIMPLEMENTED();
+  }
+
+ protected:
+  ~StackTracePrinter() {}
+
+ private:
+  static StackTracePrinter *stacktrace_printer_;
+  static StaticSpinMutex init_mu_;
+};
+
+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 c8c10de10d03a24..cf2309d0ed198b2 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
@@ -81,17 +81,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->append(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->append(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 73915715c5ba2c0..ef8cf55f0e997df 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.append("%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..9da9f1ded0e73b9 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,64 @@
 
 namespace __sanitizer {
 
-TEST(SanitizerStacktracePrinter, RenderSourceLocation) {
+TEST(FormattedStackTracePrinter, RenderSourceLocation) {
   InternalScopedString str;
-  RenderSourceLocation(&str, "/dir/file.cc", 10, 5, false, "");
+  FormattedStackTracePrinter *printer = new FormattedStackTracePrinter();
+
+  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());
 
   str.clear();
-  RenderSourceLocation(&str, "/dir/file.cc", 10, 5, true, "/dir/");
+  printer->RenderSourceLocation(&str, "/dir/file.cc", 10, 5, true, "/dir/");
   EXPECT_STREQ("file.cc(10,5)", str.data());
 }
 
-TEST(SanitizerStacktracePrinter, RenderModuleLocation) {
+TEST(FormattedStackTracePrinter, RenderModuleLocation) {
   InternalScopedString str;
-  RenderModuleLocation(&str, "/dir/exe", 0x123, kModuleArchUnknown, "");
+  FormattedStackTracePrinter *printer = new FormattedStackTracePrinter();
+  printer->RenderModuleLocation(&str, "/dir/exe", 0x123, kModuleArchUnknown,
+                                "");
   EXPECT_STREQ("(/dir/exe+0x123)", str.data());
 
   // Check that we strip file prefix if necessary.
   str.clear();
-  RenderModuleLocation(&str, "/dir/exe", 0x123, kModuleArchUnknown, "/dir/");
+  printer->RenderModuleLocation(&str, "/dir/exe", 0x123, kModuleArchUnknown,
+                                "/dir/");
   EXPECT_STREQ("(exe+0x123)", str.data());
 
   // Check that we render the arch.
   str.clear();
-  RenderModuleLocation(&str, "/dir/exe", 0x123, kModuleArchX86_64H, "/dir/");
+  printer->RenderModuleLocation(&str, "/dir/exe", 0x123, kModuleArchX86_64H,
+                                "/dir/");
   EXPECT_STREQ("(exe:x86_64h+0x123)", str.data());
 }
 
-TEST(SanitizerStacktracePrinter, RenderFrame) {
+TEST(FormattedStackTracePrinter, RenderFrame) {
+  FormattedStackTracePrinter *printer = new FormattedStackTracePrinter();
   int frame_no = 42;
   AddressInfo info;
   info.address = 0x400000;
@@ -80,11 +87,11 @@ TEST(SanitizerStacktracePrinter, RenderFrame) {
   InternalScopedString str;
 
   // Dump all the AddressInfo fields.
-  RenderFrame(&str,
-              "%% Frame:%n PC:%p Module:%m ModuleOffset:%o "
-              "Function:%f FunctionOffset:%q Source:%s Line:%l "
-              "Column:%c",
-              frame_no, info.address, &info, false, "/path/to/");
+  printer->RenderFrame(&str,
+                       "%% Frame:%n PC:%p Module:%m ModuleOffset:%o "
+                       "Function:%f FunctionOffset:%q Source:%s Line:%l "
+                       "Column:%c",
+                       frame_no, info.address, &info, false, "/path/to/");
   EXPECT_STREQ("% Frame:42 PC:0x400000 Module:my/module ModuleOffset:0x200 "
                "Function:foo FunctionOffset:0x100 Source:my/source Line:10 "
                "Column:5",
@@ -93,11 +100,11 @@ TEST(SanitizerStacktracePrinter, RenderFrame) {
   str.clear();
   // Check that RenderFrame() strips interceptor prefixes.
   info.function = internal_strdup(SANITIZER_STRINGIFY(WRAP(bar)));
-  RenderFrame(&str,
-              "%% Frame:%n PC:%p Module:%m ModuleOffset:%o "
-              "Function:%f FunctionOffset:%q Source:%s Line:%l "
-              "Column:%c",
-              frame_no, info.address, &info, false, "/path/to/");
+  printer->RenderFrame(&str,
+                       "%% Frame:%n PC:%p Module:%m ModuleOffset:%o "
+                       "Function:%f FunctionOffset:%q Source:%s Line:%l "
+                       "Column:%c",
+                       frame_no, info.address, &info, false, "/path/to/");
   EXPECT_STREQ("% Frame:42 PC:0x400000 Module:my/module ModuleOffset:0x200 "
                "Function:bar FunctionOffset:0x100 Source:my/source Line:10 "
                "Column:5",
@@ -107,26 +114,26 @@ TEST(SanitizerStacktracePrinter, RenderFrame) {
 
   // Test special format specifiers.
   info.address = 0x400000;
-  RenderFrame(&str, "%M", frame_no, info.address, &info, false);
+  printer->RenderFrame(&str, "%M", frame_no, info.address, &info, false);
   EXPECT_NE(nullptr, internal_strstr(str.data(), "400000"));
   str.clear();
 
-  RenderFrame(&str, "%L", frame_no, info.address, &info, false);
+  printer->RenderFrame(&str, "%L", frame_no, info.address, &info, false);
   EXPECT_STREQ("(<unknown module>)", str.data());
   str.clear();
 
   info.module = internal_strdup("/path/to/module");
   info.module_offset = 0x200;
-  RenderFrame(&str, "%M", frame_no, info.address, &info, false);
+  printer->RenderFrame(&str, "%M", frame_no, info.address, &info, false);
   EXPECT_NE(nullptr, internal_strstr(str.data(), "(module+0x"));
   EXPECT_NE(nullptr, internal_strstr(str.data(), "200"));
   str.clear();
 
-  RenderFrame(&str, "%L", frame_no, info.address, &info, false);
+  printer->RenderFrame(&str, "%L", frame_no, info.address, &info, false);
   EXPECT_STREQ("(/path/to/module+0x200)", str.data());
   str.clear();
 
-  RenderFrame(&str, "%b", frame_no, info.address, &info, false);
+  printer->RenderFrame(&str, "%b", frame_no, info.address, &info, false);
   EXPECT_STREQ("", str.data());
   str.clear();
 
@@ -134,7 +141,7 @@ TEST(SanitizerStacktracePrinter, RenderFrame) {
   info.uuid[0] = 0x55;
   info.uuid[1] = 0x66;
 
-  RenderFrame(&str, "%M", frame_no, info.address, &info, false);
+  printer->RenderFrame(&str, "%M", frame_no, info.address, &info, false);
   EXPECT_NE(nullptr, internal_strstr(str.data(), "(module+0x"));
   EXPECT_NE(nullptr, internal_strstr(str.data(), "200"));
 #if SANITIZER_APPLE
@@ -144,7 +151,7 @@ TEST(SanitizerStacktracePrinter, RenderFrame) {
 #endif
   str.clear();
 
-  RenderFrame(&str, "%L", frame_no, info.address, &info, false);
+  printer->RenderFrame(&str, "%L", frame_no, info.address, &info, false);
 #if SANITIZER_APPLE
   EXPECT_STREQ("(/path/to/module+0x200)", str.data());
 #else
@@ -152,46 +159,46 @@ TEST(SanitizerStacktracePrinter, RenderFrame) {
 #endif
   str.clear();
 
-  RenderFrame(&str, "%b", frame_no, info.address, &info, false);
+  printer->RenderFrame(&str, "%b", frame_no, info.address, &info, false);
   EXPECT_STREQ("(BuildId: 5566)", str.data());
   str.clear();
 
   info.function = internal_strdup("my_function");
-  RenderFrame(&str, "%F", frame_no, info.address, &info, false);
+  printer->RenderFrame(&str, "%F", frame_no, info.address, &info, false);
   EXPECT_STREQ("in my_function", str.data());
   str.clear();
 
   info.function_offset = 0x100;
-  RenderFrame(&str, "%F %S", frame_no, info.address, &info, false);
+  printer->RenderFrame(&str, "%F %S", frame_no, info.address, &info, false);
   EXPECT_STREQ("in my_function+0x100 <null>", str.data());
   str.clear();
 
   info.file = internal_strdup("my_file");
-  RenderFrame(&str, "%F %S", frame_no, info.address, &info, false);
+  printer->RenderFrame(&str, "%F %S", frame_no, info.address, &info, false);
   EXPECT_STREQ("in my_function my_file", str.data());
   str.clear();
 
   info.line = 10;
-  RenderFrame(&str, "%F %S", frame_no, info.address, &info, false);
+  printer->RenderFrame(&str, "%F %S", frame_no, info.address, &info, false);
   EXPECT_STREQ("in my_function my_file:10", str.data());
   str.clear();
 
   info.column = 5;
-  RenderFrame(&str, "%S %L", frame_no, info.address, &info, false);
+  printer->RenderFrame(&str, "%S %L", frame_no, info.address, &info, false);
   EXPECT_STREQ("my_file:10:5 my_file:10:5", str.data());
   str.clear();
 
-  RenderFrame(&str, "%S %L", frame_no, info.address, &info, true);
+  printer->RenderFrame(&str, "%S %L", frame_no, info.address, &info, true);
   EXPECT_STREQ("my_file(10,5) my_file(10,5)", str.data());
   str.clear();
 
   info.column = 0;
-  RenderFrame(&str, "%F %S", frame_no, info.address, &info, true);
+  printer->RenderFrame(&str, "%F %S", frame_no, info.address, &info, true);
   EXPECT_STREQ("in my_function my_file(10)", str.data());
   str.clear();
 
   info.line = 0;
-  RenderFrame(&str, "%F %S", frame_no, info.address, &info, true);
+  printer->RenderFrame(&str, "%F %S", frame_no, info.address, &info, true);
   EXPECT_STREQ("in my_function my_file", str.data());
   str.clear();
 
diff --git a/compiler-rt/lib/tsan/rtl/tsan_report.cpp b/compiler-rt/lib/tsan/rtl/tsan_report.cpp
index 3ae666e1212f72c..4028d1810784095 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_report.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_report.cpp
@@ -106,10 +106,10 @@ void PrintStack(const ReportStack *ent) {
   SymbolizedStack *frame = ent->frames;
   for (int i = 0; frame && frame->info.address; frame = frame->next, i++) {
     InternalScopedString res;
-    RenderFrame(&res, common_flags()->stack_trace_format, i,
-                frame->info.address, &frame->info,
-                common_flags()->symbolize_vs_style,
-                common_flags()->strip_path_prefix);
+    StackTracePrinter::GetOrInit()->RenderFrame(
+        &res, common_flags()->stack_trace_format, i, frame->info.address,
+        &frame->info, common_flags()->symbolize_vs_style,
+        common_flags()->strip_path_prefix);
     Printf("%s\n", res.data());
   }
   Printf("\n");
diff --git a/compiler-rt/lib/ubsan/ubsan_diag.cpp b/compiler-rt/lib/ubsan/ubsan_diag.cpp
index dd99613abbe3f17..d86b70350c18abf 100644
--- a/compiler-rt/lib/ubsan/ubsan_diag.cpp
+++ b/compiler-rt/lib/ubsan/ubsan_diag.cpp
@@ -149,9 +149,10 @@ static void RenderLocation(InternalScopedString *Buffer, Location Loc) {
     if (SLoc.isInvalid())
       Buffer->append("<unknown>");
     else
-      RenderSourceLocation(Buffer, SLoc.getFilename(), SLoc.getLine(),
-                           SLoc.getColumn(), common_flags()->symbolize_vs_style,
-                           common_flags()->strip_path_prefix);
+      StackTracePrinter::GetOrInit()->RenderSourceLocation(
+          Buffer, SLoc.getFilename(), SLoc.getLine(), SLoc.getColumn(),
+          common_flags()->symbolize_vs_style,
+          common_flags()->strip_path_prefix);
     return;
   }
   case Location::LK_Memory:
@@ -160,12 +161,14 @@ static void RenderLocation(InternalScopedString *Buffer, Location Loc) {
   case Location::LK_Symbolized: {
     const AddressInfo &Info = Loc.getSymbolizedStack()->info;
     if (Info.file)
-      RenderSourceLocation(Buffer, Info.file, Info.line, Info.column,
-                           common_flags()->symbolize_vs_style,
-                           common_flags()->strip_path_prefix);
+      StackTracePrinter::GetOrInit()->RenderSourceLocation(
+          Buffer, Info.file, Info.line, Info.column,
+          common_flags()->symbolize_vs_style,
+          common_flags()->strip_path_prefix);
     else if (Info.module)
-      RenderModuleLocation(Buffer, Info.module, Info.module_offset,
-                           Info.module_arch, common_flags()->strip_path_prefix);
+      StackTracePrinter::GetOrInit()->RenderModuleLocation(
+          Buffer, Info.module, Info.module_offset, Info.module_arch,
+          common_flags()->strip_path_prefix);
     else
       Buffer->append("%p", reinterpret_cast<void *>(Info.address));
     return;

>From c31f2a949c4d04d4832f23517b52c254a14eac7e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9s=20Villegas?= <andresvi at google.com>
Date: Fri, 15 Sep 2023 20:36:20 +0000
Subject: [PATCH 2/2] Fix review comments

---
 .../sanitizer_stacktrace_printer.cpp          | 18 ++---
 .../sanitizer_stacktrace_printer.h            |  4 --
 .../sanitizer_stacktrace_printer_test.cpp     | 69 ++++++++++---------
 3 files changed, 47 insertions(+), 44 deletions(-)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
index e211b5c20b4be1e..9d9b479f70b4d76 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
@@ -19,17 +19,19 @@
 
 namespace __sanitizer {
 
-StackTracePrinter *StackTracePrinter::stacktrace_printer_;
-StaticSpinMutex StackTracePrinter::init_mu_;
 
 StackTracePrinter *StackTracePrinter::GetOrInit() {
-  SpinMutexLock l(&init_mu_);
-  if (stacktrace_printer_)
-    return stacktrace_printer_;
-  stacktrace_printer_ =
+  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_;
+
+  CHECK(stacktrace_printer);
+  return stacktrace_printer;
 }
 
 const char *FormattedStackTracePrinter::StripFunctionName(
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
index a58fd557b5cc17c..bbdf70e7ab65a47 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
@@ -54,10 +54,6 @@ class StackTracePrinter {
 
  protected:
   ~StackTracePrinter() {}
-
- private:
-  static StackTracePrinter *stacktrace_printer_;
-  static StaticSpinMutex init_mu_;
 };
 
 class FormattedStackTracePrinter : public StackTracePrinter {
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 9da9f1ded0e73b9..c616a90279c86b2 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,64 +16,69 @@
 
 namespace __sanitizer {
 
+class TestFormattedStackTracePrinter final : public FormattedStackTracePrinter {
+ public:
+  ~TestFormattedStackTracePrinter() {}
+};
+
 TEST(FormattedStackTracePrinter, RenderSourceLocation) {
   InternalScopedString str;
-  FormattedStackTracePrinter *printer = new FormattedStackTracePrinter();
+  TestFormattedStackTracePrinter printer;
 
-  printer->RenderSourceLocation(&str, "/dir/file.cc", 10, 5, false, "");
+  printer.RenderSourceLocation(&str, "/dir/file.cc", 10, 5, false, "");
   EXPECT_STREQ("/dir/file.cc:10:5", str.data());
 
   str.clear();
-  printer->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();
-  printer->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();
-  printer->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();
-  printer->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();
-  printer->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();
-  printer->RenderSourceLocation(&str, "/dir/file.cc", 0, 0, true, "");
+  printer.RenderSourceLocation(&str, "/dir/file.cc", 0, 0, true, "");
   EXPECT_STREQ("/dir/file.cc", str.data());
 
   str.clear();
-  printer->RenderSourceLocation(&str, "/dir/file.cc", 10, 5, true, "/dir/");
+  printer.RenderSourceLocation(&str, "/dir/file.cc", 10, 5, true, "/dir/");
   EXPECT_STREQ("file.cc(10,5)", str.data());
 }
 
 TEST(FormattedStackTracePrinter, RenderModuleLocation) {
   InternalScopedString str;
-  FormattedStackTracePrinter *printer = new FormattedStackTracePrinter();
-  printer->RenderModuleLocation(&str, "/dir/exe", 0x123, kModuleArchUnknown,
+  TestFormattedStackTracePrinter printer;
+  printer.RenderModuleLocation(&str, "/dir/exe", 0x123, kModuleArchUnknown,
                                 "");
   EXPECT_STREQ("(/dir/exe+0x123)", str.data());
 
   // Check that we strip file prefix if necessary.
   str.clear();
-  printer->RenderModuleLocation(&str, "/dir/exe", 0x123, kModuleArchUnknown,
+  printer.RenderModuleLocation(&str, "/dir/exe", 0x123, kModuleArchUnknown,
                                 "/dir/");
   EXPECT_STREQ("(exe+0x123)", str.data());
 
   // Check that we render the arch.
   str.clear();
-  printer->RenderModuleLocation(&str, "/dir/exe", 0x123, kModuleArchX86_64H,
+  printer.RenderModuleLocation(&str, "/dir/exe", 0x123, kModuleArchX86_64H,
                                 "/dir/");
   EXPECT_STREQ("(exe:x86_64h+0x123)", str.data());
 }
 
 TEST(FormattedStackTracePrinter, RenderFrame) {
-  FormattedStackTracePrinter *printer = new FormattedStackTracePrinter();
+  TestFormattedStackTracePrinter printer;
   int frame_no = 42;
   AddressInfo info;
   info.address = 0x400000;
@@ -87,7 +92,7 @@ TEST(FormattedStackTracePrinter, RenderFrame) {
   InternalScopedString str;
 
   // Dump all the AddressInfo fields.
-  printer->RenderFrame(&str,
+  printer.RenderFrame(&str,
                        "%% Frame:%n PC:%p Module:%m ModuleOffset:%o "
                        "Function:%f FunctionOffset:%q Source:%s Line:%l "
                        "Column:%c",
@@ -100,7 +105,7 @@ TEST(FormattedStackTracePrinter, RenderFrame) {
   str.clear();
   // Check that RenderFrame() strips interceptor prefixes.
   info.function = internal_strdup(SANITIZER_STRINGIFY(WRAP(bar)));
-  printer->RenderFrame(&str,
+  printer.RenderFrame(&str,
                        "%% Frame:%n PC:%p Module:%m ModuleOffset:%o "
                        "Function:%f FunctionOffset:%q Source:%s Line:%l "
                        "Column:%c",
@@ -114,26 +119,26 @@ TEST(FormattedStackTracePrinter, RenderFrame) {
 
   // Test special format specifiers.
   info.address = 0x400000;
-  printer->RenderFrame(&str, "%M", frame_no, info.address, &info, false);
+  printer.RenderFrame(&str, "%M", frame_no, info.address, &info, false);
   EXPECT_NE(nullptr, internal_strstr(str.data(), "400000"));
   str.clear();
 
-  printer->RenderFrame(&str, "%L", frame_no, info.address, &info, false);
+  printer.RenderFrame(&str, "%L", frame_no, info.address, &info, false);
   EXPECT_STREQ("(<unknown module>)", str.data());
   str.clear();
 
   info.module = internal_strdup("/path/to/module");
   info.module_offset = 0x200;
-  printer->RenderFrame(&str, "%M", frame_no, info.address, &info, false);
+  printer.RenderFrame(&str, "%M", frame_no, info.address, &info, false);
   EXPECT_NE(nullptr, internal_strstr(str.data(), "(module+0x"));
   EXPECT_NE(nullptr, internal_strstr(str.data(), "200"));
   str.clear();
 
-  printer->RenderFrame(&str, "%L", frame_no, info.address, &info, false);
+  printer.RenderFrame(&str, "%L", frame_no, info.address, &info, false);
   EXPECT_STREQ("(/path/to/module+0x200)", str.data());
   str.clear();
 
-  printer->RenderFrame(&str, "%b", frame_no, info.address, &info, false);
+  printer.RenderFrame(&str, "%b", frame_no, info.address, &info, false);
   EXPECT_STREQ("", str.data());
   str.clear();
 
@@ -141,7 +146,7 @@ TEST(FormattedStackTracePrinter, RenderFrame) {
   info.uuid[0] = 0x55;
   info.uuid[1] = 0x66;
 
-  printer->RenderFrame(&str, "%M", frame_no, info.address, &info, false);
+  printer.RenderFrame(&str, "%M", frame_no, info.address, &info, false);
   EXPECT_NE(nullptr, internal_strstr(str.data(), "(module+0x"));
   EXPECT_NE(nullptr, internal_strstr(str.data(), "200"));
 #if SANITIZER_APPLE
@@ -151,7 +156,7 @@ TEST(FormattedStackTracePrinter, RenderFrame) {
 #endif
   str.clear();
 
-  printer->RenderFrame(&str, "%L", frame_no, info.address, &info, false);
+  printer.RenderFrame(&str, "%L", frame_no, info.address, &info, false);
 #if SANITIZER_APPLE
   EXPECT_STREQ("(/path/to/module+0x200)", str.data());
 #else
@@ -159,46 +164,46 @@ TEST(FormattedStackTracePrinter, RenderFrame) {
 #endif
   str.clear();
 
-  printer->RenderFrame(&str, "%b", frame_no, info.address, &info, false);
+  printer.RenderFrame(&str, "%b", frame_no, info.address, &info, false);
   EXPECT_STREQ("(BuildId: 5566)", str.data());
   str.clear();
 
   info.function = internal_strdup("my_function");
-  printer->RenderFrame(&str, "%F", frame_no, info.address, &info, false);
+  printer.RenderFrame(&str, "%F", frame_no, info.address, &info, false);
   EXPECT_STREQ("in my_function", str.data());
   str.clear();
 
   info.function_offset = 0x100;
-  printer->RenderFrame(&str, "%F %S", frame_no, info.address, &info, false);
+  printer.RenderFrame(&str, "%F %S", frame_no, info.address, &info, false);
   EXPECT_STREQ("in my_function+0x100 <null>", str.data());
   str.clear();
 
   info.file = internal_strdup("my_file");
-  printer->RenderFrame(&str, "%F %S", frame_no, info.address, &info, false);
+  printer.RenderFrame(&str, "%F %S", frame_no, info.address, &info, false);
   EXPECT_STREQ("in my_function my_file", str.data());
   str.clear();
 
   info.line = 10;
-  printer->RenderFrame(&str, "%F %S", frame_no, info.address, &info, false);
+  printer.RenderFrame(&str, "%F %S", frame_no, info.address, &info, false);
   EXPECT_STREQ("in my_function my_file:10", str.data());
   str.clear();
 
   info.column = 5;
-  printer->RenderFrame(&str, "%S %L", frame_no, info.address, &info, false);
+  printer.RenderFrame(&str, "%S %L", frame_no, info.address, &info, false);
   EXPECT_STREQ("my_file:10:5 my_file:10:5", str.data());
   str.clear();
 
-  printer->RenderFrame(&str, "%S %L", frame_no, info.address, &info, true);
+  printer.RenderFrame(&str, "%S %L", frame_no, info.address, &info, true);
   EXPECT_STREQ("my_file(10,5) my_file(10,5)", str.data());
   str.clear();
 
   info.column = 0;
-  printer->RenderFrame(&str, "%F %S", frame_no, info.address, &info, true);
+  printer.RenderFrame(&str, "%F %S", frame_no, info.address, &info, true);
   EXPECT_STREQ("in my_function my_file(10)", str.data());
   str.clear();
 
   info.line = 0;
-  printer->RenderFrame(&str, "%F %S", frame_no, info.address, &info, true);
+  printer.RenderFrame(&str, "%F %S", frame_no, info.address, &info, true);
   EXPECT_STREQ("in my_function my_file", str.data());
   str.clear();
 



More information about the llvm-commits mailing list