Skip to content

[compiler-rt][rtsan] Record pc and bp higher up in the stack #107014

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

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

cjappl
Copy link
Contributor

@cjappl cjappl commented Sep 2, 2024

As we solidify our external facing bits, I wanted to clean up the call stack. This is also necessary when we de-duplicate the "continue mode", which will come later down the road - we need to record the program counter as high up as feasible.

Functionally, this change affects only our printed stack traces. New version:

Real-time violation: intercepted call to real-time unsafe function `malloc` in real-time context! Stack trace:
    #0 0xaaaaab737be0 in malloc rtsan_interceptors.cpp:310:3
    #1 0xffffbe804b48 in operator new(unsigned long) 
   ...                                             

Previous call stack had internal bits:

Real-time violation: intercepted call to real-time unsafe function 
`malloc` in real-time context! Stack trace:
    #0 0x103184630 in __rtsan::printStackTrace() rtsan_stack.cpp:36
    #1 0x1031842f4 in __rtsan::Context::ExpectNotRealtime(char const*)
    #2 0x103184cfc in malloc rtsan_interceptors.cpp:225

@cjappl
Copy link
Contributor Author

cjappl commented Sep 2, 2024

CC @davidtrevelyan for review

@llvmbot
Copy link
Member

llvmbot commented Sep 2, 2024

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

Author: Chris Apple (cjappl)

Changes

As we solidify our external facing bits, I wanted to clean up the call stack. This is also necessary when we de-duplicate the "continue mode", which will come later down the road - we need to record the program counter as high up as feasible.

Functionally, this change affects only our printed stack traces. New version:

Real-time violation: intercepted call to real-time unsafe function `malloc` in real-time context! Stack trace:
    #<!-- -->0 0xaaaaab737be0 in malloc rtsan_interceptors.cpp:310:3
    #<!-- -->1 0xffffbe804b48 in operator new(unsigned long) 
   ...                                             

Previous call stack had internal bits:

Real-time violation: intercepted call to real-time unsafe function 
`malloc` in real-time context! Stack trace:
    #<!-- -->0 0x103184630 in __rtsan::printStackTrace() rtsan_stack.cpp:36
    #<!-- -->1 0x1031842f4 in __rtsan::Context::ExpectNotRealtime(char const*)
    #<!-- -->2 0x103184cfc in malloc rtsan_interceptors.cpp:225

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

5 Files Affected:

  • (modified) compiler-rt/lib/rtsan/rtsan_context.cpp (+8-3)
  • (modified) compiler-rt/lib/rtsan/rtsan_context.h (+4-1)
  • (modified) compiler-rt/lib/rtsan/rtsan_stack.cpp (+1-2)
  • (modified) compiler-rt/lib/rtsan/rtsan_stack.h (+3-1)
  • (modified) compiler-rt/test/rtsan/basic.cpp (+2-2)
diff --git a/compiler-rt/lib/rtsan/rtsan_context.cpp b/compiler-rt/lib/rtsan/rtsan_context.cpp
index 81ee44670ff3e8..abeaa1e8483493 100644
--- a/compiler-rt/lib/rtsan/rtsan_context.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_context.cpp
@@ -20,6 +20,8 @@
 #include <stdio.h>
 #include <stdlib.h>
 
+using namespace __sanitizer;
+
 static pthread_key_t context_key;
 static pthread_once_t key_once = PTHREAD_ONCE_INIT;
 
@@ -75,7 +77,9 @@ void __rtsan::ExpectNotRealtime(Context &context,
                                 const char *intercepted_function_name) {
   if (context.InRealtimeContext() && !context.IsBypassed()) {
     context.BypassPush();
-    PrintDiagnostics(intercepted_function_name);
+
+    GET_CALLER_PC_BP;
+    PrintDiagnostics(intercepted_function_name, pc, bp);
     InvokeViolationDetectedAction();
     context.BypassPop();
   }
@@ -85,12 +89,13 @@ bool __rtsan::Context::InRealtimeContext() const { return realtime_depth_ > 0; }
 
 bool __rtsan::Context::IsBypassed() const { return bypass_depth_ > 0; }
 
-void __rtsan::PrintDiagnostics(const char *intercepted_function_name) {
+void __rtsan::PrintDiagnostics(const char *intercepted_function_name, uptr pc,
+                               uptr bp) {
   fprintf(stderr,
           "Real-time violation: intercepted call to real-time unsafe function "
           "`%s` in real-time context! Stack trace:\n",
           intercepted_function_name);
-  __rtsan::PrintStackTrace();
+  __rtsan::PrintStackTrace(pc, bp);
 }
 
 __rtsan::Context &__rtsan::GetContextForThisThread() {
diff --git a/compiler-rt/lib/rtsan/rtsan_context.h b/compiler-rt/lib/rtsan/rtsan_context.h
index c8f0bf20e75a78..4196137e70b265 100644
--- a/compiler-rt/lib/rtsan/rtsan_context.h
+++ b/compiler-rt/lib/rtsan/rtsan_context.h
@@ -10,6 +10,8 @@
 
 #pragma once
 
+#include <sanitizer_common/sanitizer_internal_defs.h>
+
 namespace __rtsan {
 
 class Context {
@@ -38,6 +40,7 @@ class Context {
 Context &GetContextForThisThread();
 
 void ExpectNotRealtime(Context &context, const char *intercepted_function_name);
-void PrintDiagnostics(const char *intercepted_function_name);
+void PrintDiagnostics(const char *intercepted_function_name,
+                      __sanitizer::uptr pc, __sanitizer::uptr bp);
 
 } // namespace __rtsan
diff --git a/compiler-rt/lib/rtsan/rtsan_stack.cpp b/compiler-rt/lib/rtsan/rtsan_stack.cpp
index 43fd5fbf05509d..0598af2337ed18 100644
--- a/compiler-rt/lib/rtsan/rtsan_stack.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_stack.cpp
@@ -38,11 +38,10 @@ static void SetGlobalStackTraceFormat() {
   OverrideCommonFlags(cf);
 }
 
-void __rtsan::PrintStackTrace() {
+void __rtsan::PrintStackTrace(uptr pc, uptr bp) {
 
   BufferedStackTrace stack{};
 
-  GET_CURRENT_PC_BP;
   stack.Unwind(pc, bp, nullptr, common_flags()->fast_unwind_on_fatal);
 
   SetGlobalStackTraceFormat();
diff --git a/compiler-rt/lib/rtsan/rtsan_stack.h b/compiler-rt/lib/rtsan/rtsan_stack.h
index cecdd43045db50..38f811fa8643df 100644
--- a/compiler-rt/lib/rtsan/rtsan_stack.h
+++ b/compiler-rt/lib/rtsan/rtsan_stack.h
@@ -10,6 +10,8 @@
 
 #pragma once
 
+#include <sanitizer_common/sanitizer_internal_defs.h>
+
 namespace __rtsan {
-void PrintStackTrace();
+void PrintStackTrace(__sanitizer::uptr pc, __sanitizer::uptr bp);
 } // namespace __rtsan
diff --git a/compiler-rt/test/rtsan/basic.cpp b/compiler-rt/test/rtsan/basic.cpp
index ec7382cb0ecaff..c7cbfcda31562e 100644
--- a/compiler-rt/test/rtsan/basic.cpp
+++ b/compiler-rt/test/rtsan/basic.cpp
@@ -16,6 +16,6 @@ void violation() [[clang::nonblocking]] {
 int main() {
   violation();
   return 0;
-  // CHECK: {{.*Real-time violation.*}}
-  // CHECK: {{.*malloc*}}
+  // CHECK: Real-time violation: intercepted call to real-time unsafe function `malloc` in real-time context! Stack trace:
+  // CHECK-NEXT: {{.*malloc*}}
 }

@cjappl cjappl merged commit a424b79 into llvm:main Sep 3, 2024
10 checks passed
@cjappl cjappl deleted the pc_up_higher branch September 3, 2024 13:34
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.

3 participants