[compiler-rt] e33446e - tsan: make obtaining current PC faster

Dmitry Vyukov via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 15 08:34:08 PDT 2021


Author: Dmitry Vyukov
Date: 2021-07-15T17:34:00+02:00
New Revision: e33446ea58b8357dd8b79eb39140a1de2baff1ae

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

LOG: tsan: make obtaining current PC faster

We obtain the current PC is all interceptors and collectively
common interceptor code contributes to overall slowdown
(in particular cheaper str/mem* functions).

The current way to obtain the current PC involves:

  4493e1:       e8 3a f3 fe ff          callq  438720 <_ZN11__sanitizer10StackTrace12GetCurrentPcEv>
  4493e9:       48 89 c6                mov    %rax,%rsi

and the called function is:

uptr StackTrace::GetCurrentPc() {
  438720:       48 8b 04 24             mov    (%rsp),%rax
  438724:       c3                      retq

The new way uses address of a local label and involves just:

  44a888:       48 8d 35 fa ff ff ff    lea    -0x6(%rip),%rsi

I am not switching all uses of StackTrace::GetCurrentPc to GET_CURRENT_PC
because it may lead some differences in produced reports and break tests.
The difference comes from the fact that currently we have PC pointing
to the CALL instruction, but the new way does not yield any code on its own
so the PC points to a random instruction in the function and symbolizing
that instruction can produce additional inlined frames (if the random
instruction happen to relate to some inlined function).

Reviewed By: melver

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

Added: 
    

Modified: 
    compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h
    compiler-rt/lib/tsan/rtl/tsan_interceptors.h
    compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cpp
index 07e4409f4a5d6..49672ebf830ea 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cpp
@@ -19,38 +19,6 @@
 
 namespace __sanitizer {
 
-uptr StackTrace::GetNextInstructionPc(uptr pc) {
-#if defined(__sparc__) || defined(__mips__)
-  return pc + 8;
-#elif defined(__powerpc__) || defined(__arm__) || defined(__aarch64__)
-  return pc + 4;
-#elif SANITIZER_RISCV64
-  // Current check order is 4 -> 2 -> 6 -> 8
-  u8 InsnByte = *(u8 *)(pc);
-  if (((InsnByte & 0x3) == 0x3) && ((InsnByte & 0x1c) != 0x1c)) {
-    // xxxxxxxxxxxbbb11 | 32 bit | bbb != 111
-    return pc + 4;
-  }
-  if ((InsnByte & 0x3) != 0x3) {
-    // xxxxxxxxxxxxxxaa | 16 bit | aa != 11
-    return pc + 2;
-  }
-  // RISC-V encoding allows instructions to be up to 8 bytes long
-  if ((InsnByte & 0x3f) == 0x1f) {
-    // xxxxxxxxxx011111 | 48 bit |
-    return pc + 6;
-  }
-  if ((InsnByte & 0x7f) == 0x3f) {
-    // xxxxxxxxx0111111 | 64 bit |
-    return pc + 8;
-  }
-  // bail-out if could not figure out the instruction size
-  return 0;
-#else
-  return pc + 1;
-#endif
-}
-
 uptr StackTrace::GetCurrentPc() {
   return GET_CALLER_PC();
 }

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h
index 6b2a687a990ca..03137f87ea629 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h
@@ -106,6 +106,39 @@ uptr StackTrace::GetPreviousInstructionPc(uptr pc) {
 #endif
 }
 
+ALWAYS_INLINE
+uptr StackTrace::GetNextInstructionPc(uptr pc) {
+#if defined(__sparc__) || defined(__mips__)
+  return pc + 8;
+#elif defined(__powerpc__) || defined(__arm__) || defined(__aarch64__)
+  return pc + 4;
+#elif SANITIZER_RISCV64
+  // Current check order is 4 -> 2 -> 6 -> 8
+  u8 InsnByte = *(u8 *)(pc);
+  if (((InsnByte & 0x3) == 0x3) && ((InsnByte & 0x1c) != 0x1c)) {
+    // xxxxxxxxxxxbbb11 | 32 bit | bbb != 111
+    return pc + 4;
+  }
+  if ((InsnByte & 0x3) != 0x3) {
+    // xxxxxxxxxxxxxxaa | 16 bit | aa != 11
+    return pc + 2;
+  }
+  // RISC-V encoding allows instructions to be up to 8 bytes long
+  if ((InsnByte & 0x3f) == 0x1f) {
+    // xxxxxxxxxx011111 | 48 bit |
+    return pc + 6;
+  }
+  if ((InsnByte & 0x7f) == 0x3f) {
+    // xxxxxxxxx0111111 | 64 bit |
+    return pc + 8;
+  }
+  // bail-out if could not figure out the instruction size
+  return 0;
+#else
+  return pc + 1;
+#endif
+}
+
 // StackTrace that owns the buffer used to store the addresses.
 struct BufferedStackTrace : public StackTrace {
   uptr trace_buffer[kStackTraceMax];
@@ -196,5 +229,10 @@ static inline bool IsValidFrame(uptr frame, uptr stack_top, uptr stack_bottom) {
   uptr local_stack;                           \
   uptr sp = (uptr)&local_stack
 
+#define GET_CURRENT_PC()                                   \
+  ({                                                       \
+    this_pc:                                               \
+      StackTrace::GetNextInstructionPc((uptr) && this_pc); \
+  })
 
 #endif  // SANITIZER_STACKTRACE_H

diff  --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors.h b/compiler-rt/lib/tsan/rtl/tsan_interceptors.h
index 29576ea2d49ad..c5716f53a323a 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors.h
@@ -30,14 +30,14 @@ inline bool in_symbolizer() {
 
 }  // namespace __tsan
 
-#define SCOPED_INTERCEPTOR_RAW(func, ...) \
-    cur_thread_init(); \
-    ThreadState *thr = cur_thread(); \
-    const uptr caller_pc = GET_CALLER_PC(); \
-    ScopedInterceptor si(thr, #func, caller_pc); \
-    const uptr pc = StackTrace::GetCurrentPc(); \
-    (void)pc; \
-/**/
+#define SCOPED_INTERCEPTOR_RAW(func, ...)      \
+  cur_thread_init();                           \
+  ThreadState *thr = cur_thread();             \
+  const uptr caller_pc = GET_CALLER_PC();      \
+  ScopedInterceptor si(thr, #func, caller_pc); \
+  const uptr pc = GET_CURRENT_PC();            \
+  (void)pc;                                    \
+  /**/
 
 #define SCOPED_TSAN_INTERCEPTOR(func, ...) \
     SCOPED_INTERCEPTOR_RAW(func, __VA_ARGS__); \

diff  --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index 6808f2e0e2d3d..86accfefc0c6e 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -2157,7 +2157,7 @@ void atfork_prepare() {
   if (in_symbolizer())
     return;
   ThreadState *thr = cur_thread();
-  const uptr pc = StackTrace::GetCurrentPc();
+  const uptr pc = GET_CURRENT_PC();
   ForkBefore(thr, pc);
 }
 
@@ -2165,7 +2165,7 @@ void atfork_parent() {
   if (in_symbolizer())
     return;
   ThreadState *thr = cur_thread();
-  const uptr pc = StackTrace::GetCurrentPc();
+  const uptr pc = GET_CURRENT_PC();
   ForkParentAfter(thr, pc);
 }
 
@@ -2173,7 +2173,7 @@ void atfork_child() {
   if (in_symbolizer())
     return;
   ThreadState *thr = cur_thread();
-  const uptr pc = StackTrace::GetCurrentPc();
+  const uptr pc = GET_CURRENT_PC();
   ForkChildAfter(thr, pc);
   FdOnFork(thr, pc);
 }


        


More information about the llvm-commits mailing list