[compiler-rt] [sanitizer_symbolizer] Add MarkupStackTracePrinter (PR #73032)

Andres Villegas via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 21 13:38:05 PST 2023


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


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

>From 90866301ad977489092ba462e2375dd93f614bd0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9s=20Villegas?= <andresvi at google.com>
Date: Tue, 21 Nov 2023 18:39:59 +0000
Subject: [PATCH 1/3] [NFC sanitizer_symbolizer] Make some functions non
 virtual

in StackTracePrinter.

Make some methods of StackTracPrinter that will have a common
implementation non virtual.
---
 .../sanitizer_stacktrace_printer.cpp          | 17 +++++------
 .../sanitizer_stacktrace_printer.h            | 28 ++++++-------------
 2 files changed, 17 insertions(+), 28 deletions(-)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
index 8db321051e1a0de..69047c2e31eca2a 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
@@ -32,8 +32,7 @@ StackTracePrinter *StackTracePrinter::GetOrInit() {
   return stacktrace_printer;
 }
 
-const char *FormattedStackTracePrinter::StripFunctionName(
-    const char *function) {
+const char *StackTracePrinter::StripFunctionName(const char *function) {
   if (!common_flags()->demangle)
     return function;
   if (!function)
@@ -324,9 +323,10 @@ void FormattedStackTracePrinter::RenderData(InternalScopedString *buffer,
 
 #endif  // !SANITIZER_SYMBOLIZER_MARKUP
 
-void FormattedStackTracePrinter::RenderSourceLocation(
-    InternalScopedString *buffer, const char *file, int line, int column,
-    bool vs_style, const char *strip_path_prefix) {
+void StackTracePrinter::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)
@@ -343,9 +343,10 @@ void FormattedStackTracePrinter::RenderSourceLocation(
   }
 }
 
-void FormattedStackTracePrinter::RenderModuleLocation(
-    InternalScopedString *buffer, const char *module, uptr offset,
-    ModuleArch arch, const char *strip_path_prefix) {
+void StackTracePrinter::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 3e02333a8c8d5cc..9cf39013e78bc17 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
@@ -25,7 +25,8 @@ class StackTracePrinter {
  public:
   static StackTracePrinter *GetOrInit();
 
-  virtual const char *StripFunctionName(const char *function) = 0;
+  // Strip interceptor prefixes from function name.
+  const char *StripFunctionName(const char *function);
 
   virtual void RenderFrame(InternalScopedString *buffer, const char *format,
                            int frame_no, uptr address, const AddressInfo *info,
@@ -34,15 +35,13 @@ class StackTracePrinter {
 
   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;
+  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) = 0;
+  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 = "") = 0;
@@ -53,9 +52,6 @@ class 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
@@ -90,14 +86,6 @@ class FormattedStackTracePrinter : public StackTracePrinter {
 
   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:

>From ef70d60ce8b52d8260e84db2aafaf55f5f548192 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9s=20Villegas?= <andresvi at google.com>
Date: Tue, 21 Nov 2023 20:10:08 +0000
Subject: [PATCH 2/3] [sanitizer_symbolizer] Add MarkupStackTracePrinter

This PR is part of a stack, #73029 should be merged
and reviewed first. Please only review the last commit
of this PR.

Adds a new Implementation of StackTracePrinter that only
emits symbolizer markup. Currently this change only
affects Fuchsia OS. Should be NFC.
---
 .../sanitizer_stacktrace_printer.cpp          |  8 +++-
 .../sanitizer_stacktrace_printer.h            |  4 ++
 .../sanitizer_symbolizer_markup.cpp           | 44 ++++++++++---------
 3 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
index 69047c2e31eca2a..e0c3943e4a9743e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
@@ -12,6 +12,7 @@
 
 #include "sanitizer_stacktrace_printer.h"
 
+#include "sanitizer_common.h"
 #include "sanitizer_file.h"
 #include "sanitizer_flags.h"
 #include "sanitizer_fuchsia.h"
@@ -25,8 +26,7 @@ StackTracePrinter *StackTracePrinter::GetOrInit() {
   if (stacktrace_printer)
     return stacktrace_printer;
 
-  stacktrace_printer =
-      new (GetGlobalLowLevelAllocator()) FormattedStackTracePrinter();
+  stacktrace_printer = StackTracePrinter::PlatformInit();
 
   CHECK(stacktrace_printer);
   return stacktrace_printer;
@@ -61,6 +61,10 @@ const char *StackTracePrinter::StripFunctionName(const char *function) {
 // sanitizer_symbolizer_markup.cpp implements these differently.
 #if !SANITIZER_SYMBOLIZER_MARKUP
 
+StackTracePrinter *StackTracePrinter::PlatformInit() {
+  return new (GetGlobalLowLevelAllocator()) FormattedStackTracePrinter();
+}
+
 static const char *DemangleFunctionName(const char *function) {
   if (!common_flags()->demangle)
     return function;
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
index 9cf39013e78bc17..9de09bda0062d45 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
@@ -46,6 +46,10 @@ class StackTracePrinter {
                           const DataInfo *DI,
                           const char *strip_path_prefix = "") = 0;
 
+ private:
+  // To be called from StackTracePrinter::GetOrInit
+  static StackTracePrinter *PlatformInit();
+
  protected:
   ~StackTracePrinter() {}
 };
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
index 6402cfd7c36e443..ef3866a4bd32ac8 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
@@ -76,27 +76,29 @@ bool Symbolizer::SymbolizeData(uptr addr, DataInfo *info) {
   return true;
 }
 
-// We ignore the format argument to __sanitizer_symbolize_global.
-void FormattedStackTracePrinter::RenderData(InternalScopedString *buffer,
-                                            const char *format,
-                                            const DataInfo *DI,
-                                            const char *strip_path_prefix) {
-  buffer->AppendF(kFormatData, DI->start);
-}
-
-bool FormattedStackTracePrinter::RenderNeedsSymbolization(const char *format) {
-  return false;
-}
-
-// We don't support the stack_trace_format flag at all.
-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);
+class MarkupStackTracePrinter : public StackTracePrinter {
+  // We ignore the format argument to __sanitizer_symbolize_global.
+  void RenderData(InternalScopedString *buffer, const char *format,
+                  const DataInfo *DI, const char *strip_path_prefix) override {
+    buffer->AppendF(kFormatData, DI->start);
+  }
+
+  bool RenderNeedsSymbolization(const char *format) override { 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) override {
+    CHECK(!RenderNeedsSymbolization(format));
+    buffer->AppendF(kFormatFrame, frame_no, address);
+  }
+
+ protected:
+  ~MarkupStackTracePrinter();
+};
+
+StackTracePrinter *StackTracePrinter::PlatformInit() {
+  return new (GetGlobalLowLevelAllocator()) MarkupStackTracePrinter();
 }
 
 Symbolizer *Symbolizer::PlatformInit() {

>From afa3e5a8c860a7643c729099af406b4a0f191536 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9s=20Villegas?= <andresvi at google.com>
Date: Tue, 21 Nov 2023 21:35:12 +0000
Subject: [PATCH 3/3] Review comments

---
 .../lib/sanitizer_common/sanitizer_stacktrace_printer.cpp     | 4 ++--
 .../lib/sanitizer_common/sanitizer_stacktrace_printer.h       | 2 +-
 .../lib/sanitizer_common/sanitizer_symbolizer_markup.cpp      | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
index e0c3943e4a9743e..88f186b9c20c105 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
@@ -26,7 +26,7 @@ StackTracePrinter *StackTracePrinter::GetOrInit() {
   if (stacktrace_printer)
     return stacktrace_printer;
 
-  stacktrace_printer = StackTracePrinter::PlatformInit();
+  stacktrace_printer = StackTracePrinter::NewStackTracePrinter();
 
   CHECK(stacktrace_printer);
   return stacktrace_printer;
@@ -61,7 +61,7 @@ const char *StackTracePrinter::StripFunctionName(const char *function) {
 // sanitizer_symbolizer_markup.cpp implements these differently.
 #if !SANITIZER_SYMBOLIZER_MARKUP
 
-StackTracePrinter *StackTracePrinter::PlatformInit() {
+StackTracePrinter *StackTracePrinter::NewStackTracePrinter() {
   return new (GetGlobalLowLevelAllocator()) FormattedStackTracePrinter();
 }
 
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
index 9de09bda0062d45..10361a32034452e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
@@ -48,7 +48,7 @@ class StackTracePrinter {
 
  private:
   // To be called from StackTracePrinter::GetOrInit
-  static StackTracePrinter *PlatformInit();
+  static StackTracePrinter *NewStackTracePrinter();
 
  protected:
   ~StackTracePrinter() {}
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
index ef3866a4bd32ac8..c7332af7d9efd5a 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
@@ -97,7 +97,7 @@ class MarkupStackTracePrinter : public StackTracePrinter {
   ~MarkupStackTracePrinter();
 };
 
-StackTracePrinter *StackTracePrinter::PlatformInit() {
+StackTracePrinter *StackTracePrinter::NewStackTracePrinter() {
   return new (GetGlobalLowLevelAllocator()) MarkupStackTracePrinter();
 }
 



More information about the llvm-commits mailing list