[compiler-rt] r299630 - [lsan] Avoid segfaults during threads destruction under high load

Maxim Ostapenko via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 6 00:42:28 PDT 2017


Author: chefmax
Date: Thu Apr  6 02:42:27 2017
New Revision: 299630

URL: http://llvm.org/viewvc/llvm-project?rev=299630&view=rev
Log:
[lsan] Avoid segfaults during threads destruction under high load

This patch addresses two issues:

	* It turned out that suspended thread may have dtls->dtv_size == kDestroyedThread (-1)
	and LSan wrongly assumes that DTV is available. This leads to SEGV when LSan tries to
	iterate through DTV that is invalid.
	* In some rare cases GetRegistersAndSP can fail with errno 3 (ESRCH). In this case LSan
	assumes that the whole stack of a given thread is available. This is wrong because ESRCH
	can indicate that suspended thread was destroyed and its stack was unmapped. This patch
	properly handles ESRCH from GetRegistersAndSP in order to avoid invalid accesses to already
	unpapped threads stack.

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

Modified:
    compiler-rt/trunk/lib/lsan/lsan_common.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld.h
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_tls_get_addr.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_tls_get_addr.h

Modified: compiler-rt/trunk/lib/lsan/lsan_common.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/lsan/lsan_common.cc?rev=299630&r1=299629&r2=299630&view=diff
==============================================================================
--- compiler-rt/trunk/lib/lsan/lsan_common.cc (original)
+++ compiler-rt/trunk/lib/lsan/lsan_common.cc Thu Apr  6 02:42:27 2017
@@ -201,11 +201,13 @@ static void ProcessThreads(SuspendedThre
       continue;
     }
     uptr sp;
-    bool have_registers =
-        (suspended_threads.GetRegistersAndSP(i, registers.data(), &sp) == 0);
-    if (!have_registers) {
-      Report("Unable to get registers from thread %d.\n");
-      // If unable to get SP, consider the entire stack to be reachable.
+    PtraceRegistersStatus have_registers =
+        suspended_threads.GetRegistersAndSP(i, registers.data(), &sp);
+    if (have_registers != REGISTERS_AVAILABLE) {
+      Report("Unable to get registers from thread %d.\n", os_id);
+      // If unable to get SP, consider the entire stack to be reachable unless
+      // GetRegistersAndSP failed with ESRCH.
+      if (have_registers == REGISTERS_UNAVAILABLE_FATAL) continue;
       sp = stack_begin;
     }
 
@@ -253,7 +255,7 @@ static void ProcessThreads(SuspendedThre
         if (tls_end > cache_end)
           ScanRangeForPointers(cache_end, tls_end, frontier, "TLS", kReachable);
       }
-      if (dtls) {
+      if (dtls && !DTLSInDestruction(dtls)) {
         for (uptr j = 0; j < dtls->dtv_size; ++j) {
           uptr dtls_beg = dtls->dtv[j].beg;
           uptr dtls_end = dtls_beg + dtls->dtv[j].size;
@@ -263,6 +265,10 @@ static void ProcessThreads(SuspendedThre
                                  kReachable);
           }
         }
+      } else {
+        // We are handling a thread with DTLS under destruction. Log about
+        // this and continue.
+        LOG_THREADS("Thread %d has DTLS under destruction.\n", os_id);
       }
     }
   }

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld.h?rev=299630&r1=299629&r2=299630&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld.h Thu Apr  6 02:42:27 2017
@@ -20,6 +20,12 @@
 namespace __sanitizer {
 typedef int SuspendedThreadID;
 
+enum PtraceRegistersStatus {
+  REGISTERS_UNAVAILABLE_FATAL = -1,
+  REGISTERS_UNAVAILABLE = 0,
+  REGISTERS_AVAILABLE = 1
+};
+
 // Holds the list of suspended threads and provides an interface to dump their
 // register contexts.
 class SuspendedThreadsList {
@@ -30,7 +36,8 @@ class SuspendedThreadsList {
     CHECK_LT(index, thread_ids_.size());
     return thread_ids_[index];
   }
-  int GetRegistersAndSP(uptr index, uptr *buffer, uptr *sp) const;
+  PtraceRegistersStatus GetRegistersAndSP(uptr index, uptr *buffer,
+                                          uptr *sp) const;
   // The buffer in GetRegistersAndSP should be at least this big.
   static uptr RegisterCount();
   uptr thread_count() const { return thread_ids_.size(); }

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc?rev=299630&r1=299629&r2=299630&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc Thu Apr  6 02:42:27 2017
@@ -493,9 +493,9 @@ typedef _user_regs_struct regs_struct;
 #error "Unsupported architecture"
 #endif // SANITIZER_ANDROID && defined(__arm__)
 
-int SuspendedThreadsList::GetRegistersAndSP(uptr index,
-                                            uptr *buffer,
-                                            uptr *sp) const {
+PtraceRegistersStatus SuspendedThreadsList::GetRegistersAndSP(uptr index,
+                                                              uptr *buffer,
+                                                              uptr *sp) const {
   pid_t tid = GetThreadID(index);
   regs_struct regs;
   int pterrno;
@@ -513,12 +513,16 @@ int SuspendedThreadsList::GetRegistersAn
   if (isErr) {
     VReport(1, "Could not get registers from thread %d (errno %d).\n", tid,
             pterrno);
-    return -1;
+    // ESRCH means that the given thread is not suspended or already dead.
+    // Therefore it's unsafe to inspect its data (e.g. walk through stack) and
+    // we should notify caller about this.
+    return pterrno == ESRCH ? REGISTERS_UNAVAILABLE_FATAL
+                            : REGISTERS_UNAVAILABLE;
   }
 
   *sp = regs.REG_SP;
   internal_memcpy(buffer, &regs, sizeof(regs));
-  return 0;
+  return REGISTERS_AVAILABLE;
 }
 
 uptr SuspendedThreadsList::RegisterCount() {

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_tls_get_addr.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_tls_get_addr.cc?rev=299630&r1=299629&r2=299630&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_tls_get_addr.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_tls_get_addr.cc Thu Apr  6 02:42:27 2017
@@ -136,11 +136,17 @@ void DTLS_on_libc_memalign(void *ptr, up
 
 DTLS *DTLS_Get() { return &dtls; }
 
+bool DTLSInDestruction(DTLS *dtls) {
+  return dtls->dtv_size == kDestroyedThread;
+}
+
 #else
 void DTLS_on_libc_memalign(void *ptr, uptr size) {}
 DTLS::DTV *DTLS_on_tls_get_addr(void *arg, void *res) { return 0; }
 DTLS *DTLS_Get() { return 0; }
 void DTLS_Destroy() {}
+bool DTLSInDestruction(DTLS *dtls) { UNREACHABLE(); }
+
 #endif  // SANITIZER_INTERCEPT_TLS_GET_ADDR
 
 }  // namespace __sanitizer

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_tls_get_addr.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_tls_get_addr.h?rev=299630&r1=299629&r2=299630&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_tls_get_addr.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_tls_get_addr.h Thu Apr  6 02:42:27 2017
@@ -55,6 +55,8 @@ DTLS::DTV *DTLS_on_tls_get_addr(void *ar
 void DTLS_on_libc_memalign(void *ptr, uptr size);
 DTLS *DTLS_Get();
 void DTLS_Destroy();  // Make sure to call this before the thread is destroyed.
+// Returns true if DTLS of suspended thread is in destruction process.
+bool DTLSInDestruction(DTLS *dtls);
 
 }  // namespace __sanitizer
 




More information about the llvm-commits mailing list