Skip to content

[sanitizer] Select non-internal frames in ReportErrorSummary #77406

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

Conversation

vitalybuka
Copy link
Collaborator

Summary contains one line and should point to user code instead of
internal compiler-rt location. TSAN already does that.

Created using spr 1.3.4
@llvmbot
Copy link
Member

llvmbot commented Jan 9, 2024

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

Author: Vitaly Buka (vitalybuka)

Changes

Summary contains one line and should point to user code instead of
internal compiler-rt location. TSAN already does that.


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

10 Files Affected:

  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp (+21-7)
  • (modified) compiler-rt/test/hwasan/TestCases/Linux/aligned_alloc-alignment.cpp (+1-1)
  • (modified) compiler-rt/test/hwasan/TestCases/Linux/pvalloc-overflow.cpp (+1-1)
  • (modified) compiler-rt/test/hwasan/TestCases/Posix/posix_memalign-alignment.cpp (+1-1)
  • (modified) compiler-rt/test/hwasan/TestCases/allocator_returns_null.cpp (+8-8)
  • (modified) compiler-rt/test/hwasan/TestCases/halt-on-error.cpp (+3-3)
  • (modified) compiler-rt/test/hwasan/TestCases/report-unmapped.cpp (+1-1)
  • (modified) compiler-rt/test/hwasan/TestCases/use-after-free.c (+1-1)
  • (modified) compiler-rt/test/sanitizer_common/TestCases/allocator_returns_null.cpp (+8-8)
  • (modified) compiler-rt/test/sanitizer_common/TestCases/max_allocation_size.cpp (+8-8)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
index 0cf250f7212943..4e5c55a8f85b22 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
@@ -95,17 +95,31 @@ void ReportErrorSummary(const char *error_type, const StackTrace *stack,
 #if !SANITIZER_GO
   if (!common_flags()->print_summary)
     return;
-  if (stack->size == 0) {
-    ReportErrorSummary(error_type);
-    return;
+
+  // Find first non-internal stack frame.
+  for (uptr i = 0; i < stack->size; ++i) {
+    uptr pc = StackTrace::GetPreviousInstructionPc(stack->trace[i]);
+    SymbolizedStackHolder symbolized_stack(
+        Symbolizer::GetOrInit()->SymbolizePC(pc));
+    if (const SymbolizedStack *frame = symbolized_stack.get()) {
+      if (const SymbolizedStack *summary_frame = SkipInternalFrames(frame)) {
+        ReportErrorSummary(error_type, summary_frame->info, alt_tool_name);
+        return;
+      }
+    }
   }
-  // Currently, we include the first stack frame into the report summary.
-  // Maybe sometimes we need to choose another frame (e.g. skip memcpy/etc).
+
+  // Fallback to the top one.
   uptr pc = StackTrace::GetPreviousInstructionPc(stack->trace[0]);
   SymbolizedStackHolder symbolized_stack(
       Symbolizer::GetOrInit()->SymbolizePC(pc));
-  const SymbolizedStack *frame = symbolized_stack.get();
-  ReportErrorSummary(error_type, frame->info, alt_tool_name);
+  if (const SymbolizedStack *frame = symbolized_stack.get()) {
+    ReportErrorSummary(error_type, frame->info, alt_tool_name);
+    return;
+  }
+
+  // Fallback to a summary without location.
+  ReportErrorSummary(error_type);
 #endif
 }
 
diff --git a/compiler-rt/test/hwasan/TestCases/Linux/aligned_alloc-alignment.cpp b/compiler-rt/test/hwasan/TestCases/Linux/aligned_alloc-alignment.cpp
index ad5b7616e8a7fb..35e29e8cc83435 100644
--- a/compiler-rt/test/hwasan/TestCases/Linux/aligned_alloc-alignment.cpp
+++ b/compiler-rt/test/hwasan/TestCases/Linux/aligned_alloc-alignment.cpp
@@ -14,7 +14,7 @@ int main() {
   // CHECK: ERROR: HWAddressSanitizer: invalid alignment requested in aligned_alloc: 17
   // CHECK: {{#0 0x.* in .*}}{{aligned_alloc|memalign}}
   // CHECK: {{#1 0x.* in main .*aligned_alloc-alignment.cpp:}}[[@LINE-3]]
-  // CHECK: SUMMARY: HWAddressSanitizer: invalid-aligned-alloc-alignment {{.*}} in aligned_alloc
+  // CHECK: SUMMARY: HWAddressSanitizer: invalid-aligned-alloc-alignment {{.*}} in main
 
   printf("pointer after failed aligned_alloc: %zd\n", (size_t)p);
   // CHECK-NULL: pointer after failed aligned_alloc: 0
diff --git a/compiler-rt/test/hwasan/TestCases/Linux/pvalloc-overflow.cpp b/compiler-rt/test/hwasan/TestCases/Linux/pvalloc-overflow.cpp
index bd9f34a0dac921..6b4410449a838a 100644
--- a/compiler-rt/test/hwasan/TestCases/Linux/pvalloc-overflow.cpp
+++ b/compiler-rt/test/hwasan/TestCases/Linux/pvalloc-overflow.cpp
@@ -39,6 +39,6 @@ int main(int argc, char *argv[]) {
 // CHECK: {{ERROR: HWAddressSanitizer: pvalloc parameters overflow: size .* rounded up to system page size .* cannot be represented in type size_t}}
 // CHECK: {{#0 0x.* in .*pvalloc}}
 // CHECK: {{#1 0x.* in main .*pvalloc-overflow.cpp:}}
-// CHECK: SUMMARY: HWAddressSanitizer: pvalloc-overflow {{.*}} in pvalloc
+// CHECK: SUMMARY: HWAddressSanitizer: pvalloc-overflow {{.*}} in main
 
 // CHECK-NULL: errno: 12
diff --git a/compiler-rt/test/hwasan/TestCases/Posix/posix_memalign-alignment.cpp b/compiler-rt/test/hwasan/TestCases/Posix/posix_memalign-alignment.cpp
index 029e086f99ada2..5841ca42ceb033 100644
--- a/compiler-rt/test/hwasan/TestCases/Posix/posix_memalign-alignment.cpp
+++ b/compiler-rt/test/hwasan/TestCases/Posix/posix_memalign-alignment.cpp
@@ -11,7 +11,7 @@ int main() {
   // CHECK: ERROR: HWAddressSanitizer: invalid alignment requested in posix_memalign: 17
   // CHECK: {{#0 0x.* in .*posix_memalign}}
   // CHECK: {{#1 0x.* in main .*posix_memalign-alignment.cpp:}}[[@LINE-3]]
-  // CHECK: SUMMARY: HWAddressSanitizer: invalid-posix-memalign-alignment {{.*}} in posix_memalign
+  // CHECK: SUMMARY: HWAddressSanitizer: invalid-posix-memalign-alignment {{.*}} in main
 
   printf("pointer after failed posix_memalign: %zd\n", (size_t)p);
   // CHECK-NULL: pointer after failed posix_memalign: 42
diff --git a/compiler-rt/test/hwasan/TestCases/allocator_returns_null.cpp b/compiler-rt/test/hwasan/TestCases/allocator_returns_null.cpp
index 18ee9406d146fb..2db28984e94988 100644
--- a/compiler-rt/test/hwasan/TestCases/allocator_returns_null.cpp
+++ b/compiler-rt/test/hwasan/TestCases/allocator_returns_null.cpp
@@ -87,21 +87,21 @@ int main(int argc, char **argv) {
 }
 
 // CHECK-mCRASH: malloc:
-// CHECK-mCRASH: SUMMARY: HWAddressSanitizer: allocation-size-too-big {{.*}} in malloc
+// CHECK-mCRASH: SUMMARY: HWAddressSanitizer: allocation-size-too-big {{.*}} in main
 // CHECK-cCRASH: calloc:
-// CHECK-cCRASH: SUMMARY: HWAddressSanitizer: allocation-size-too-big {{.*}} in calloc
+// CHECK-cCRASH: SUMMARY: HWAddressSanitizer: allocation-size-too-big {{.*}} in main
 // CHECK-coCRASH: calloc-overflow:
-// CHECK-coCRASH: SUMMARY: HWAddressSanitizer: calloc-overflow {{.*}} in calloc
+// CHECK-coCRASH: SUMMARY: HWAddressSanitizer: calloc-overflow {{.*}} in main
 // CHECK-rCRASH: realloc:
-// CHECK-rCRASH: SUMMARY: HWAddressSanitizer: allocation-size-too-big {{.*}} in realloc
+// CHECK-rCRASH: SUMMARY: HWAddressSanitizer: allocation-size-too-big {{.*}} in main
 // CHECK-mrCRASH: realloc-after-malloc:
-// CHECK-mrCRASH: SUMMARY: HWAddressSanitizer: allocation-size-too-big {{.*}} in realloc
+// CHECK-mrCRASH: SUMMARY: HWAddressSanitizer: allocation-size-too-big {{.*}} in main
 // CHECK-nCRASH: new:
-// CHECK-nCRASH: SUMMARY: HWAddressSanitizer: allocation-size-too-big {{.*}} in operator new
+// CHECK-nCRASH: SUMMARY: HWAddressSanitizer: allocation-size-too-big {{.*}} in main
 // CHECK-nCRASH-OOM: new:
-// CHECK-nCRASH-OOM: SUMMARY: HWAddressSanitizer: out-of-memory {{.*}} in operator new
+// CHECK-nCRASH-OOM: SUMMARY: HWAddressSanitizer: out-of-memory {{.*}} in main
 // CHECK-nnCRASH: new-nothrow:
-// CHECK-nnCRASH: SUMMARY: HWAddressSanitizer: allocation-size-too-big {{.*}} in operator new
+// CHECK-nnCRASH: SUMMARY: HWAddressSanitizer: allocation-size-too-big {{.*}} in main
 
 // CHECK-mNULL: malloc:
 // CHECK-mNULL: errno: 12
diff --git a/compiler-rt/test/hwasan/TestCases/halt-on-error.cpp b/compiler-rt/test/hwasan/TestCases/halt-on-error.cpp
index 1a32e4bf4cc4b5..b27ee34ff7cc70 100644
--- a/compiler-rt/test/hwasan/TestCases/halt-on-error.cpp
+++ b/compiler-rt/test/hwasan/TestCases/halt-on-error.cpp
@@ -26,15 +26,15 @@ int main() {
   // COMMON: READ of size 4 at
   // When instrumenting with callbacks, main is actually #1, and #0 is __hwasan_load4.
   // COMMON: #{{.*}} in main {{.*}}halt-on-error.cpp:[[@LINE-3]]
-  // COMMON: SUMMARY: HWAddressSanitizer: tag-mismatch {{.*}} in
+  // COMMON: SUMMARY: HWAddressSanitizer: tag-mismatch {{.*}} in main
 
   // RECOVER: READ of size 1 at
   // RECOVER: #{{.*}} in main {{.*}}halt-on-error.cpp:[[@LINE-7]]
-  // RECOVER: SUMMARY: HWAddressSanitizer: tag-mismatch {{.*}} in
+  // RECOVER: SUMMARY: HWAddressSanitizer: tag-mismatch {{.*}} in main
 
   // RECOVER: READ of size 1 at
   // RECOVER: #{{.*}} in main {{.*}}halt-on-error.cpp:[[@LINE-11]]
-  // RECOVER: SUMMARY: HWAddressSanitizer: tag-mismatch {{.*}} in
+  // RECOVER: SUMMARY: HWAddressSanitizer: tag-mismatch {{.*}} in main
 
   // COMMON-NOT: tag-mismatch
 }
diff --git a/compiler-rt/test/hwasan/TestCases/report-unmapped.cpp b/compiler-rt/test/hwasan/TestCases/report-unmapped.cpp
index a58e50a78d8750..c00a615f7d5254 100644
--- a/compiler-rt/test/hwasan/TestCases/report-unmapped.cpp
+++ b/compiler-rt/test/hwasan/TestCases/report-unmapped.cpp
@@ -36,4 +36,4 @@ int main(int argc, char **argv) {
 // CHECK: Tags for short granules around
 
 // Check that report is complete.
-// CHECK: SUMMARY: HWAddressSanitizer
+// CHECK: SUMMARY: HWAddressSanitizer: tag-mismatch {{.*}} in main
diff --git a/compiler-rt/test/hwasan/TestCases/use-after-free.c b/compiler-rt/test/hwasan/TestCases/use-after-free.c
index b3eed88600726c..070622f560a225 100644
--- a/compiler-rt/test/hwasan/TestCases/use-after-free.c
+++ b/compiler-rt/test/hwasan/TestCases/use-after-free.c
@@ -38,6 +38,6 @@ int main() {
   // CHECK: #1 {{.*}} in main {{.*}}use-after-free.c:[[@LINE-24]]
   // CHECK: Memory tags around the buggy address (one tag corresponds to 16 bytes):
   // CHECK: =>{{.*}}[[MEM_TAG]]
-  // CHECK: SUMMARY: HWAddressSanitizer: tag-mismatch
+  // CHECK: SUMMARY: HWAddressSanitizer: tag-mismatch {{.*}} in main
   return r;
 }
diff --git a/compiler-rt/test/sanitizer_common/TestCases/allocator_returns_null.cpp b/compiler-rt/test/sanitizer_common/TestCases/allocator_returns_null.cpp
index 9f8e12ff6aa060..ca6f637b9a3f50 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/allocator_returns_null.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/allocator_returns_null.cpp
@@ -95,28 +95,28 @@ int main(int argc, char **argv) {
 
 // CHECK-mCRASH: malloc:
 // CHECK-mCRASH: #{{[0-9]+.*}}allocator_returns_null.cpp
-// CHECK-mCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.*}} in {{.*}}lloc
+// CHECK-mCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.*allocator_returns_null.cpp.*}} in main
 // CHECK-cCRASH: calloc:
 // CHECK-cCRASH: #{{[0-9]+.*}}allocator_returns_null.cpp
-// CHECK-cCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.*}} in {{.*}}lloc
+// CHECK-cCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.*allocator_returns_null.cpp.*}} in main
 // CHECK-coCRASH: calloc-overflow:
 // CHECK-coCRASH: #{{[0-9]+.*}}allocator_returns_null.cpp
-// CHECK-coCRASH: {{SUMMARY: .*Sanitizer: calloc-overflow.*}} in {{.*}}lloc
+// CHECK-coCRASH: {{SUMMARY: .*Sanitizer: calloc-overflow.*allocator_returns_null.cpp.*}} in main
 // CHECK-rCRASH: realloc:
 // CHECK-rCRASH: #{{[0-9]+.*}}allocator_returns_null.cpp
-// CHECK-rCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.*}} in {{.*}}lloc
+// CHECK-rCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.*allocator_returns_null.cpp.*}} in main
 // CHECK-mrCRASH: realloc-after-malloc:
 // CHECK-mrCRASH: #{{[0-9]+.*}}allocator_returns_null.cpp
-// CHECK-mrCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.*}} in {{.*}}lloc
+// CHECK-mrCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.*allocator_returns_null.cpp.*}} in main
 // CHECK-nCRASH: new:
 // CHECK-nCRASH: #{{[0-9]+.*}}allocator_returns_null.cpp
-// CHECK-nCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.*}} in {{operator new|.*lloc}}
+// CHECK-nCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.*allocator_returns_null.cpp.*}} in main
 // CHECK-nCRASH-OOM: new:
 // CHECK-nCRASH-O#{{[0-9]+.*}}allocator_returns_null.cpp
-// CHECK-nCRASH-OOM: {{SUMMARY: .*Sanitizer: out-of-memory.*}} in {{operator new|.*lloc}}
+// CHECK-nCRASH-OOM: {{SUMMARY: .*Sanitizer: out-of-memory.*allocator_returns_null.cpp.*}} in main
 // CHECK-nnCRASH: new-nothrow:
 // CHECK-nnCRASH: #{{[0-9]+.*}}allocator_returns_null.cpp
-// CHECK-nnCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.*}} in {{operator new|.*lloc}}
+// CHECK-nnCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.*allocator_returns_null.cpp.*}} in main
 
 // CHECK-NULL: {{malloc|calloc|calloc-overflow|realloc|realloc-after-malloc|new-nothrow}}
 // CHECK-NULL: errno: 12, x: 0
diff --git a/compiler-rt/test/sanitizer_common/TestCases/max_allocation_size.cpp b/compiler-rt/test/sanitizer_common/TestCases/max_allocation_size.cpp
index c74f241c32b754..2fde16fbed3d26 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/max_allocation_size.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/max_allocation_size.cpp
@@ -124,28 +124,28 @@ int main(int Argc, char **Argv) {
 
 // CHECK-mCRASH: malloc:
 // CHECK-mCRASH: #{{[0-9]+.*}}max_allocation_size.cpp
-// CHECK-mCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big}}
+// CHECK-mCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.* in allocate}}
 // CHECK-cCRASH: calloc:
 // CHECK-cCRASH: #{{[0-9]+.*}}max_allocation_size.cpp
-// CHECK-cCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big}}
+// CHECK-cCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.* in allocate}}
 // CHECK-rCRASH: realloc:
 // CHECK-rCRASH: #{{[0-9]+.*}}max_allocation_size.cpp
-// CHECK-rCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big}}
+// CHECK-rCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.* in allocate}}
 // CHECK-mrCRASH: realloc-after-malloc:
 // CHECK-mrCRASH: #{{[0-9]+.*}}max_allocation_size.cpp
-// CHECK-mrCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big}}
+// CHECK-mrCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.* in allocate}}
 // CHECK-nCRASH: new:
 // CHECK-nCRASH: #{{[0-9]+.*}}max_allocation_size.cpp
-// CHECK-nCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big}}
+// CHECK-nCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.* in allocate}}
 // CHECK-nCRASH-OOM: new:
 // CHECK-nCRASH-OOM: #{{[0-9]+.*}}max_allocation_size.cpp
-// CHECK-nCRASH-OOM: {{SUMMARY: .*Sanitizer: out-of-memory}}
+// CHECK-nCRASH-OOM: {{SUMMARY: .*Sanitizer: out-of-memory.* in allocate}}
 // CHECK-nnCRASH: new-nothrow:
 // CHECK-nnCRASH: #{{[0-9]+.*}}max_allocation_size.cpp
-// CHECK-nnCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big}}
+// CHECK-nnCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.* in allocate}}
 // CHECK-sCRASH: strndup:
 // CHECK-sCRASH: #{{[0-9]+.*}}max_allocation_size.cpp
-// CHECK-sCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big}}
+// CHECK-sCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.*}}
 
 // CHECK-NULL: {{malloc|calloc|calloc-overflow|realloc|realloc-after-malloc|new-nothrow|strndup}}
 // CHECK-NULL: errno: 12, P: 0

Created using spr 1.3.4
Created using spr 1.3.4
@vitalybuka vitalybuka merged commit 71e5652 into main Jan 9, 2024
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/sanitizer-select-non-internal-frames-in-reporterrorsummary branch January 9, 2024 22:03
@MaskRay
Copy link
Member

MaskRay commented Jan 10, 2024

Nice filtering!

justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
)

Summary contains one line and should point to user code instead of
internal compiler-rt location. TSAN already does that.
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.

4 participants