[NFC][sanitizer_symbolizer]Add StackTracePrinter class

[avillega]

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.

[llvmbot]

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

Changes

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.

Patch is 30.37 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/66530.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 (+101-51)
• (modified) compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp (+13-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 (+47-40)
• (modified) compiler-rt/lib/tsan/rtl/tsan_report.cpp (+4-4)
• (modified) compiler-rt/lib/ubsan/ubsan_diag.cpp (+11-8)

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, ""); ... [truncated] 

[vitalybuka]

With 2 suggestions

[petrhosek]

Can you make these methods pure virtual?