[compiler-rt] 4d5b1de - [sanitizer] Skip stack symbolization when not required for print format

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 7 15:39:07 PDT 2020


Author: Teresa Johnson
Date: 2020-10-07T15:38:52-07:00
New Revision: 4d5b1de40eccc7ffcfb859cef407e5f30bee77f8

URL: https://github.com/llvm/llvm-project/commit/4d5b1de40eccc7ffcfb859cef407e5f30bee77f8
DIFF: https://github.com/llvm/llvm-project/commit/4d5b1de40eccc7ffcfb859cef407e5f30bee77f8.diff

LOG: [sanitizer] Skip stack symbolization when not required for print format

Adds a check to avoid symbolization when printing stack traces if the
stack_trace_format flag does not need it. While there is a symbolize
flag that can be turned off to skip some of the symbolization,
SymbolizePC() still unconditionally looks up the module name and offset.
Avoid invoking SymbolizePC() at all if not needed.

This is an efficiency improvement when dumping all stack traces as part
of the memory profiler in D87120, for large stripped apps where we want
to symbolize as a post pass.

Differential Revision: https://reviews.llvm.org/D88361

Added: 
    

Modified: 
    compiler-rt/lib/hwasan/hwasan_report.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
    compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
    compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_printer_test.cpp
    compiler-rt/lib/tsan/rtl/tsan_report.cpp
    compiler-rt/test/sanitizer_common/TestCases/print-stack-trace.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index 206aa601903e..0be7deeaee1a 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -224,7 +224,7 @@ 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\n", 0, frame->info,
+      RenderFrame(&frame_desc, " %F %L\n", 0, frame->info.address, &frame->info,
                   common_flags()->symbolize_vs_style,
                   common_flags()->strip_path_prefix);
       frame->ClearAll();

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp
index 68bd0bb29629..7808ba9b0f57 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp
@@ -26,17 +26,23 @@ void StackTrace::Print() const {
   InternalScopedString frame_desc(GetPageSizeCached() * 2);
   InternalScopedString dedup_token(GetPageSizeCached());
   int dedup_frames = common_flags()->dedup_token_length;
+  bool symbolize = RenderNeedsSymbolization(common_flags()->stack_trace_format);
   uptr frame_num = 0;
   for (uptr i = 0; i < size && trace[i]; i++) {
     // PCs in stack traces are actually the return addresses, that is,
     // addresses of the next instructions after the call.
     uptr pc = GetPreviousInstructionPc(trace[i]);
-    SymbolizedStack *frames = Symbolizer::GetOrInit()->SymbolizePC(pc);
+    SymbolizedStack *frames;
+    if (symbolize)
+      frames = Symbolizer::GetOrInit()->SymbolizePC(pc);
+    else
+      frames = SymbolizedStack::New(pc);
     CHECK(frames);
     for (SymbolizedStack *cur = frames; cur; cur = cur->next) {
       frame_desc.clear();
       RenderFrame(&frame_desc, common_flags()->stack_trace_format, frame_num++,
-                  cur->info, common_flags()->symbolize_vs_style,
+                  cur->info.address, symbolize ? &cur->info : nullptr,
+                  common_flags()->symbolize_vs_style,
                   common_flags()->strip_path_prefix);
       Printf("%s\n", frame_desc.data());
       if (dedup_frames-- > 0) {
@@ -108,7 +114,12 @@ void __sanitizer_symbolize_pc(uptr pc, const char *fmt, char *out_buf,
                               uptr out_buf_size) {
   if (!out_buf_size) return;
   pc = StackTrace::GetPreviousInstructionPc(pc);
-  SymbolizedStack *frame = Symbolizer::GetOrInit()->SymbolizePC(pc);
+  SymbolizedStack *frame;
+  bool symbolize = RenderNeedsSymbolization(fmt);
+  if (symbolize)
+    frame = Symbolizer::GetOrInit()->SymbolizePC(pc);
+  else
+    frame = SymbolizedStack::New(pc);
   if (!frame) {
     internal_strncpy(out_buf, "<can't symbolize>", out_buf_size);
     out_buf[out_buf_size - 1] = 0;
@@ -121,7 +132,8 @@ void __sanitizer_symbolize_pc(uptr pc, const char *fmt, char *out_buf,
   for (SymbolizedStack *cur = frame; cur && out_buf < out_end;
        cur = cur->next) {
     frame_desc.clear();
-    RenderFrame(&frame_desc, fmt, frame_num++, cur->info,
+    RenderFrame(&frame_desc, fmt, frame_num++, cur->info.address,
+                symbolize ? &cur->info : nullptr,
                 common_flags()->symbolize_vs_style,
                 common_flags()->strip_path_prefix);
     if (!frame_desc.length())

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
index 150ff475316b..97755ca1c560 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp
@@ -107,8 +107,14 @@ static const char *DemangleFunctionName(const char *function) {
 static const char kDefaultFormat[] = "    #%n %p %F %L";
 
 void RenderFrame(InternalScopedString *buffer, const char *format, int frame_no,
-                 const AddressInfo &info, bool vs_style,
+                 uptr address, const AddressInfo *info, bool vs_style,
                  const char *strip_path_prefix, const char *strip_func_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
+  // ever ends up out of sync with this function. If non-null, the addresses
+  // should match.
+  CHECK(!info || address == info->address);
   if (0 == internal_strcmp(format, "DEFAULT"))
     format = kDefaultFormat;
   for (const char *p = format; *p != '\0'; p++) {
@@ -126,71 +132,69 @@ void RenderFrame(InternalScopedString *buffer, const char *format, int frame_no,
       buffer->append("%zu", frame_no);
       break;
     case 'p':
-      buffer->append("0x%zx", info.address);
+      buffer->append("0x%zx", address);
       break;
     case 'm':
-      buffer->append("%s", StripPathPrefix(info.module, strip_path_prefix));
+      buffer->append("%s", StripPathPrefix(info->module, strip_path_prefix));
       break;
     case 'o':
-      buffer->append("0x%zx", info.module_offset);
+      buffer->append("0x%zx", info->module_offset);
       break;
     case 'f':
-      buffer->append("%s",
-        DemangleFunctionName(
-          StripFunctionName(info.function, strip_func_prefix)));
+      buffer->append("%s", DemangleFunctionName(StripFunctionName(
+                               info->function, strip_func_prefix)));
       break;
     case 'q':
-      buffer->append("0x%zx", info.function_offset != AddressInfo::kUnknown
-                                  ? info.function_offset
+      buffer->append("0x%zx", info->function_offset != AddressInfo::kUnknown
+                                  ? info->function_offset
                                   : 0x0);
       break;
     case 's':
-      buffer->append("%s", StripPathPrefix(info.file, strip_path_prefix));
+      buffer->append("%s", StripPathPrefix(info->file, strip_path_prefix));
       break;
     case 'l':
-      buffer->append("%d", info.line);
+      buffer->append("%d", info->line);
       break;
     case 'c':
-      buffer->append("%d", info.column);
+      buffer->append("%d", info->column);
       break;
     // Smarter special cases.
     case 'F':
       // Function name and offset, if file is unknown.
-      if (info.function) {
-        buffer->append("in %s",
-                       DemangleFunctionName(
-                         StripFunctionName(info.function, strip_func_prefix)));
-        if (!info.file && info.function_offset != AddressInfo::kUnknown)
-          buffer->append("+0x%zx", info.function_offset);
+      if (info->function) {
+        buffer->append("in %s", DemangleFunctionName(StripFunctionName(
+                                    info->function, strip_func_prefix)));
+        if (!info->file && info->function_offset != AddressInfo::kUnknown)
+          buffer->append("+0x%zx", info->function_offset);
       }
       break;
     case 'S':
       // File/line information.
-      RenderSourceLocation(buffer, info.file, info.line, info.column, vs_style,
-                           strip_path_prefix);
+      RenderSourceLocation(buffer, info->file, info->line, info->column,
+                           vs_style, strip_path_prefix);
       break;
     case 'L':
       // Source location, or module location.
-      if (info.file) {
-        RenderSourceLocation(buffer, info.file, info.line, info.column,
+      if (info->file) {
+        RenderSourceLocation(buffer, info->file, info->line, info->column,
                              vs_style, strip_path_prefix);
-      } else if (info.module) {
-        RenderModuleLocation(buffer, info.module, info.module_offset,
-                             info.module_arch, strip_path_prefix);
+      } else if (info->module) {
+        RenderModuleLocation(buffer, info->module, info->module_offset,
+                             info->module_arch, strip_path_prefix);
       } else {
         buffer->append("(<unknown module>)");
       }
       break;
     case 'M':
       // Module basename and offset, or PC.
-      if (info.address & kExternalPCBit)
-        {} // There PCs are not meaningful.
-      else if (info.module)
+      if (address & kExternalPCBit) {
+      }  // There PCs are not meaningful.
+      else if (info->module)
         // Always strip the module name for %M.
-        RenderModuleLocation(buffer, StripModuleName(info.module),
-                             info.module_offset, info.module_arch, "");
+        RenderModuleLocation(buffer, StripModuleName(info->module),
+                             info->module_offset, info->module_arch, "");
       else
-        buffer->append("(%p)", (void *)info.address);
+        buffer->append("(%p)", (void *)address);
       break;
     default:
       Report("Unsupported specifier in stack frame format: %c (0x%zx)!\n", *p,
@@ -200,6 +204,29 @@ void RenderFrame(InternalScopedString *buffer, const char *format, int frame_no,
   }
 }
 
+bool RenderNeedsSymbolization(const char *format) {
+  if (0 == internal_strcmp(format, "DEFAULT"))
+    format = kDefaultFormat;
+  for (const char *p = format; *p != '\0'; p++) {
+    if (*p != '%')
+      continue;
+    p++;
+    switch (*p) {
+      case '%':
+        break;
+      case 'n':
+        // frame_no
+        break;
+      case 'p':
+        // address
+        break;
+      default:
+        return true;
+    }
+  }
+  return false;
+}
+
 void RenderData(InternalScopedString *buffer, const char *format,
                 const DataInfo *DI, const char *strip_path_prefix) {
   for (const char *p = format; *p != '\0'; p++) {

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
index f7f7629f773f..96119b2ee9e9 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.h
@@ -47,10 +47,12 @@ namespace __sanitizer {
 //        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,
-                 const AddressInfo &info, bool vs_style,
+                 uptr address, const AddressInfo *info, bool vs_style,
                  const char *strip_path_prefix = "",
                  const char *strip_func_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);

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
index c8eb781dfc84..06301b83ea1f 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
@@ -33,7 +33,8 @@ void ReportErrorSummary(const char *error_type, const AddressInfo &info,
   if (!common_flags()->print_summary) return;
   InternalScopedString buff(kMaxSummaryLength);
   buff.append("%s ", error_type);
-  RenderFrame(&buff, "%L %F", 0, info, common_flags()->symbolize_vs_style,
+  RenderFrame(&buff, "%L %F", 0, info.address, &info,
+              common_flags()->symbolize_vs_style,
               common_flags()->strip_path_prefix);
   ReportErrorSummary(buff.data(), alt_tool_name);
 }

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 1ce89a30cf69..a98e47ab6c53 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
@@ -79,10 +79,11 @@ TEST(SanitizerStacktracePrinter, RenderFrame) {
   InternalScopedString str(256);
 
   // 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, false, "/path/to/", "function_");
+  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/", "function_");
   EXPECT_STREQ("% Frame:42 PC:0x400000 Module:my/module ModuleOffset:0x200 "
                "Function:foo FunctionOffset:0x100 Source:my/source Line:10 "
                "Column:5",
@@ -92,61 +93,61 @@ TEST(SanitizerStacktracePrinter, RenderFrame) {
 
   // Test special format specifiers.
   info.address = 0x400000;
-  RenderFrame(&str, "%M", frame_no, info, false);
+  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, false);
+  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, false);
+  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, false);
+  RenderFrame(&str, "%L", frame_no, info.address, &info, false);
   EXPECT_STREQ("(/path/to/module+0x200)", str.data());
   str.clear();
 
   info.function = internal_strdup("my_function");
-  RenderFrame(&str, "%F", frame_no, info, false);
+  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, false);
+  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, false);
+  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, false);
+  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, false);
+  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, true);
+  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, true);
+  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, true);
+  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 368f1ca8adf2..4892c446c104 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_report.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_report.cpp
@@ -128,7 +128,8 @@ void PrintStack(const ReportStack *ent) {
   SymbolizedStack *frame = ent->frames;
   for (int i = 0; frame && frame->info.address; frame = frame->next, i++) {
     InternalScopedString res(2 * GetPageSizeCached());
-    RenderFrame(&res, common_flags()->stack_trace_format, i, frame->info,
+    RenderFrame(&res, common_flags()->stack_trace_format, i,
+                frame->info.address, &frame->info,
                 common_flags()->symbolize_vs_style,
                 common_flags()->strip_path_prefix, kInterposedFunctionPrefix);
     Printf("%s\n", res.data());

diff  --git a/compiler-rt/test/sanitizer_common/TestCases/print-stack-trace.cpp b/compiler-rt/test/sanitizer_common/TestCases/print-stack-trace.cpp
index 0c0bdc03dac2..9d7d03d81b53 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/print-stack-trace.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/print-stack-trace.cpp
@@ -2,6 +2,7 @@
 // RUN: %clangxx -O3 %s -o %t && %env_tool_opts=stack_trace_format=DEFAULT %run %t 2>&1 | FileCheck %s
 // RUN: %env_tool_opts=stack_trace_format=frame%n_lineno%l %run %t 2>&1 | FileCheck %s --check-prefix=CUSTOM
 // RUN: %env_tool_opts=symbolize_inline_frames=false:stack_trace_format=DEFAULT %run %t 2>&1 | FileCheck %s --check-prefix=NOINLINE
+// RUN: %env_tool_opts=stack_trace_format='"frame:%n address:%%p"' %run %t 2>&1 | FileCheck %s --check-prefix=NOSYMBOLIZE
 
 // UNSUPPORTED: darwin
 
@@ -27,3 +28,7 @@ int main() {
 
 // NOINLINE: #0 0x{{.*}} in __sanitizer_print_stack_trace
 // NOINLINE: #1 0x{{.*}} in main{{.*}}print-stack-trace.cpp:[[@LINE-15]]
+
+// NOSYMBOLIZE: frame:0 address:{{0x.*}}
+// NOSYMBOLIZE: frame:1 address:{{0x.*}}
+// NOSYMBOLIZE: frame:2 address:{{0x.*}}


        


More information about the llvm-commits mailing list