From 90866301ad977489092ba462e2375dd93f614bd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Villegas?= 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 8db321051e1a0..69047c2e31eca 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 3e02333a8c8d5..9cf39013e78bc 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?= 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 69047c2e31eca..e0c3943e4a974 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 9cf39013e78bc..9de09bda0062d 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 6402cfd7c36e4..ef3866a4bd32a 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?= 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 e0c3943e4a974..88f186b9c20c1 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 9de09bda0062d..10361a3203445 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 ef3866a4bd32a..c7332af7d9efd 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(); }