Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFC][sanitizer] Move SymbolizedStackHolder into sanitizer_common #77152

Conversation

vitalybuka
Copy link
Collaborator

And replace most ClearAll() uses.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 5, 2024

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

Author: Vitaly Buka (vitalybuka)

Changes

And replace most ClearAll() uses.


Full diff: https://github.com/llvm/llvm-project/pull/77152.diff

7 Files Affected:

  • (modified) compiler-rt/lib/asan/asan_suppressions.cpp (+3-4)
  • (modified) compiler-rt/lib/hwasan/hwasan_report.cpp (+4-2)
  • (modified) compiler-rt/lib/lsan/lsan_common.cpp (+4-3)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp (+6-6)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h (+20)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp (+3-2)
  • (modified) compiler-rt/lib/ubsan/ubsan_diag.h (-20)
diff --git a/compiler-rt/lib/asan/asan_suppressions.cpp b/compiler-rt/lib/asan/asan_suppressions.cpp
index 8cb2c3e3b9b6a4..e71d231821866a 100644
--- a/compiler-rt/lib/asan/asan_suppressions.cpp
+++ b/compiler-rt/lib/asan/asan_suppressions.cpp
@@ -81,9 +81,10 @@ bool IsStackTraceSuppressed(const StackTrace *stack) {
     }
 
     if (suppression_ctx->HasSuppressionType(kInterceptorViaFunction)) {
-      SymbolizedStack *frames = symbolizer->SymbolizePC(addr);
+      SymbolizedStackHolder symbolized_stack(symbolizer->SymbolizePC(addr));
+      const SymbolizedStack *frames = symbolized_stack.get();
       CHECK(frames);
-      for (SymbolizedStack *cur = frames; cur; cur = cur->next) {
+      for (const SymbolizedStack *cur = frames; cur; cur = cur->next) {
         const char *function_name = cur->info.function;
         if (!function_name) {
           continue;
@@ -91,11 +92,9 @@ bool IsStackTraceSuppressed(const StackTrace *stack) {
         // Match "interceptor_via_fun" suppressions.
         if (suppression_ctx->Match(function_name, kInterceptorViaFunction,
                                    &s)) {
-          frames->ClearAll();
           return true;
         }
       }
-      frames->ClearAll();
     }
   }
   return false;
diff --git a/compiler-rt/lib/hwasan/hwasan_report.cpp b/compiler-rt/lib/hwasan/hwasan_report.cpp
index 1a018a891b56ee..784cfb904aa275 100644
--- a/compiler-rt/lib/hwasan/hwasan_report.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_report.cpp
@@ -292,12 +292,14 @@ static void PrintStackAllocations(const StackAllocationsRingBuffer *sa,
     uptr pc = record & pc_mask;
     frame_desc.AppendF("  record_addr:0x%zx record:0x%zx",
                        reinterpret_cast<uptr>(record_addr), record);
-    if (SymbolizedStack *frame = Symbolizer::GetOrInit()->SymbolizePC(pc)) {
+    SymbolizedStackHolder symbolized_stack(
+        Symbolizer::GetOrInit()->SymbolizePC(pc));
+    const SymbolizedStack *frame = symbolized_stack.get();
+    if (frame) {
       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());
     frame_desc.clear();
diff --git a/compiler-rt/lib/lsan/lsan_common.cpp b/compiler-rt/lib/lsan/lsan_common.cpp
index e24839c984b346..0ecded8b28cdb0 100644
--- a/compiler-rt/lib/lsan/lsan_common.cpp
+++ b/compiler-rt/lib/lsan/lsan_common.cpp
@@ -155,14 +155,15 @@ Suppression *LeakSuppressionContext::GetSuppressionForAddr(uptr addr) {
     return s;
 
   // Suppress by file or function name.
-  SymbolizedStack *frames = Symbolizer::GetOrInit()->SymbolizePC(addr);
-  for (SymbolizedStack *cur = frames; cur; cur = cur->next) {
+  SymbolizedStackHolder symbolized_stack(
+      Symbolizer::GetOrInit()->SymbolizePC(addr));
+  const SymbolizedStack *frames = symbolized_stack.get();
+  for (const SymbolizedStack *cur = frames; cur; cur = cur->next) {
     if (context.Match(cur->info.function, kSuppressionLeak, &s) ||
         context.Match(cur->info.file, kSuppressionLeak, &s)) {
       break;
     }
   }
-  frames->ClearAll();
   return s;
 }
 
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp
index 9a4c80fcfdd199..561eae9ab78065 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp
@@ -33,13 +33,14 @@ class StackTraceTextPrinter {
             stack_trace_fmt)) {}
 
   bool ProcessAddressFrames(uptr pc) {
-    SymbolizedStack *frames = symbolize_
-                                  ? Symbolizer::GetOrInit()->SymbolizePC(pc)
-                                  : SymbolizedStack::New(pc);
+    SymbolizedStackHolder symbolized_stack(
+        symbolize_ ? Symbolizer::GetOrInit()->SymbolizePC(pc)
+                   : SymbolizedStack::New(pc));
+    const SymbolizedStack *frames = symbolized_stack.get();
     if (!frames)
       return false;
 
-    for (SymbolizedStack *cur = frames; cur; cur = cur->next) {
+    for (const SymbolizedStack *cur = frames; cur; cur = cur->next) {
       uptr prev_len = output_->length();
       StackTracePrinter::GetOrInit()->RenderFrame(
           output_, stack_trace_fmt_, frame_num_++, cur->info.address,
@@ -51,13 +52,12 @@ class StackTraceTextPrinter {
 
       ExtendDedupToken(cur);
     }
-    frames->ClearAll();
     return true;
   }
 
  private:
   // Extend the dedup token by appending a new frame.
-  void ExtendDedupToken(SymbolizedStack *stack) {
+  void ExtendDedupToken(const SymbolizedStack *stack) {
     if (!dedup_token_)
       return;
 
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h
index 82cd9bc2279162..16ef2f2fd717b9 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h
@@ -64,6 +64,26 @@ struct SymbolizedStack {
   SymbolizedStack();
 };
 
+class SymbolizedStackHolder {
+  SymbolizedStack *Stack;
+
+  void clear() {
+    if (Stack)
+      Stack->ClearAll();
+  }
+
+ public:
+  explicit SymbolizedStackHolder(SymbolizedStack *Stack = nullptr)
+      : Stack(Stack) {}
+  ~SymbolizedStackHolder() { clear(); }
+  void reset(SymbolizedStack *S = nullptr) {
+    if (Stack != S)
+      clear();
+    Stack = S;
+  }
+  const SymbolizedStack *get() const { return Stack; }
+};
+
 // For now, DataInfo is used to describe global variable.
 struct DataInfo {
   // Owns all the string members. Storage for them is
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
index ec60dd3e0d6620..26f015e70d0405 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
@@ -102,9 +102,10 @@ void ReportErrorSummary(const char *error_type, const StackTrace *stack,
   // Currently, we include the first stack frame into the report summary.
   // Maybe sometimes we need to choose another frame (e.g. skip memcpy/etc).
   uptr pc = StackTrace::GetPreviousInstructionPc(stack->trace[0]);
-  SymbolizedStack *frame = Symbolizer::GetOrInit()->SymbolizePC(pc);
+  SymbolizedStackHolder symbolized_stack(
+      Symbolizer::GetOrInit()->SymbolizePC(pc));
+  const SymbolizedStack *frame = symbolized_stack.get();
   ReportErrorSummary(error_type, frame->info, alt_tool_name);
-  frame->ClearAll();
 #endif
 }
 
diff --git a/compiler-rt/lib/ubsan/ubsan_diag.h b/compiler-rt/lib/ubsan/ubsan_diag.h
index b444e971b22838..c836647c98f3c5 100644
--- a/compiler-rt/lib/ubsan/ubsan_diag.h
+++ b/compiler-rt/lib/ubsan/ubsan_diag.h
@@ -18,26 +18,6 @@
 
 namespace __ubsan {
 
-class SymbolizedStackHolder {
-  SymbolizedStack *Stack;
-
-  void clear() {
-    if (Stack)
-      Stack->ClearAll();
-  }
-
-public:
-  explicit SymbolizedStackHolder(SymbolizedStack *Stack = nullptr)
-      : Stack(Stack) {}
-  ~SymbolizedStackHolder() { clear(); }
-  void reset(SymbolizedStack *S) {
-    if (Stack != S)
-      clear();
-    Stack = S;
-  }
-  const SymbolizedStack *get() const { return Stack; }
-};
-
 SymbolizedStack *getSymbolizedLocation(uptr PC);
 
 inline SymbolizedStack *getCallerLocation(uptr CallerPC) {

Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@vitalybuka vitalybuka changed the base branch from users/vitalybuka/spr/main.nfcsanitizer-move-symbolizedstackholder-into-sanitizer_common to main January 6, 2024 00:57
Created using spr 1.3.4
@vitalybuka vitalybuka merged commit 23aabdd into main Jan 6, 2024
4 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/nfcsanitizer-move-symbolizedstackholder-into-sanitizer_common branch January 6, 2024 02:40
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants