[compiler-rt] a1e7f62 - Revert "[sanitizer] Run Stack compression in background thread"

Petr Hosek via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 9 09:57:04 PST 2021


Author: Petr Hosek
Date: 2021-12-09T09:56:48-08:00
New Revision: a1e7f6280104fd1a6a467cf3ac6d5081df1fcdd8

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

LOG: Revert "[sanitizer] Run Stack compression in background thread"

This reverts commit e5c2a46c5e8fc038b9f6c898df9628f9524dc10e as this
change introduced a linker error when building sanitizer runtimes:

  ld.lld: error: undefined symbol: __sanitizer::internal_start_thread(void* (*)(void*), void*)
  >>> referenced by sanitizer_stackdepot.cpp:133 (compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp:133)
  >>>               compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommonSymbolizer.x86_64.dir/sanitizer_stackdepot.cpp.obj:(__sanitizer::(anonymous namespace)::CompressThread::NewWorkNotify())

Added: 
    

Modified: 
    compiler-rt/lib/sanitizer_common/sanitizer_common_libcdep.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.h
    compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_common_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_common_libcdep.cpp
index b68392be3b3a1..13624a83865d7 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common_libcdep.cpp
@@ -15,14 +15,9 @@
 #include "sanitizer_common.h"
 #include "sanitizer_flags.h"
 #include "sanitizer_procmaps.h"
-#include "sanitizer_stackdepot.h"
 
-namespace __sanitizer {
 
-// Weak default implementation for when sanitizer_stackdepot is not linked in.
-#if !SANITIZER_GO
-SANITIZER_WEAK_ATTRIBUTE void StackDepotStopBackgroundThread() {}
-#endif
+namespace __sanitizer {
 
 #if (SANITIZER_LINUX || SANITIZER_NETBSD) && !SANITIZER_GO
 // Weak default implementation for when sanitizer_stackdepot is not linked in.
@@ -206,7 +201,6 @@ void ProtectGap(uptr addr, uptr size, uptr zero_base_shadow_start,
 
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_sandbox_on_notify,
                              __sanitizer_sandbox_arguments *args) {
-  __sanitizer::StackDepotStopBackgroundThread();
   __sanitizer::PlatformPrepareForSandboxing(args);
   if (__sanitizer::sandboxing_callback)
     __sanitizer::sandboxing_callback();

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp
index c755b1829d2a3..1d3ac5cc778a7 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp
@@ -12,10 +12,8 @@
 
 #include "sanitizer_stackdepot.h"
 
-#include "sanitizer_atomic.h"
 #include "sanitizer_common.h"
 #include "sanitizer_hash.h"
-#include "sanitizer_mutex.h"
 #include "sanitizer_stack_store.h"
 #include "sanitizer_stackdepotbase.h"
 
@@ -77,7 +75,7 @@ uptr StackDepotNode::allocated() {
 static void CompressStackStore() {
   u64 start = MonotonicNanoTime();
   uptr 
diff  = stackStore.Pack(static_cast<StackStore::Compression>(
-      Abs(common_flags()->compress_stack_depot)));
+      common_flags()->compress_stack_depot));
   if (!
diff )
     return;
   u64 finish = MonotonicNanoTime();
@@ -87,112 +85,12 @@ static void CompressStackStore() {
           (finish - start) / 1000000);
 }
 
-namespace {
-
-class CompressThread {
- public:
-  constexpr CompressThread() = default;
-  void NewWorkNotify();
-  void Stop();
-  void LockAndStop() NO_THREAD_SAFETY_ANALYSIS;
-  void Unlock() NO_THREAD_SAFETY_ANALYSIS;
-
- private:
-  enum class State {
-    NotStarted = 0,
-    Started,
-    Failed,
-    Stopped,
-  };
-
-  void Run();
-
-  bool WaitForWork() {
-    semaphore_.Wait();
-    return atomic_load(&run_, memory_order_acquire);
-  }
-
-  Semaphore semaphore_ = {};
-  StaticSpinMutex mutex_ = {};
-  State state_ GUARDED_BY(mutex_) = State::NotStarted;
-  void *thread_ GUARDED_BY(mutex_) = nullptr;
-  atomic_uint8_t run_ = {};
-};
-
-static CompressThread compress_thread;
-
-void CompressThread::NewWorkNotify() {
-  int compress = common_flags()->compress_stack_depot;
-  if (!compress)
-    return;
-  if (compress > 0 /* for testing or debugging */) {
-    SpinMutexLock l(&mutex_);
-    if (state_ == State::NotStarted) {
-      atomic_store(&run_, 1, memory_order_release);
-      CHECK_EQ(nullptr, thread_);
-      thread_ = internal_start_thread(
-          [](void *arg) -> void * {
-            reinterpret_cast<CompressThread *>(arg)->Run();
-            return nullptr;
-          },
-          this);
-      state_ = thread_ ? State::Started : State::Failed;
-    }
-    if (state_ == State::Started) {
-      semaphore_.Post();
-      return;
-    }
-  }
-  CompressStackStore();
-}
-
-void CompressThread::Run() {
-  VPrintf(1, "%s: StackDepot compression thread started\n", SanitizerToolName);
-  while (WaitForWork()) CompressStackStore();
-  VPrintf(1, "%s: StackDepot compression thread stopped\n", SanitizerToolName);
-}
-
-void CompressThread::Stop() {
-  void *t = nullptr;
-  {
-    SpinMutexLock l(&mutex_);
-    if (state_ != State::Started)
-      return;
-    state_ = State::Stopped;
-    CHECK_NE(nullptr, thread_);
-    t = thread_;
-    thread_ = nullptr;
-  }
-  atomic_store(&run_, 0, memory_order_release);
-  semaphore_.Post();
-  internal_join_thread(t);
-}
-
-void CompressThread::LockAndStop() {
-  mutex_.Lock();
-  if (state_ != State::Started)
-    return;
-  CHECK_NE(nullptr, thread_);
-
-  atomic_store(&run_, 0, memory_order_release);
-  semaphore_.Post();
-  internal_join_thread(thread_);
-  // Allow to restart after Unlock() if needed.
-  state_ = State::NotStarted;
-  thread_ = nullptr;
-}
-
-void CompressThread::Unlock() { mutex_.Unlock(); }
-
-}  // namespace
-
 void StackDepotNode::store(u32 id, const args_type &args, hash_type hash) {
   stack_hash = hash;
   uptr pack = 0;
   store_id = stackStore.Store(args, &pack);
-  if (LIKELY(!pack))
-    return;
-  compress_thread.NewWorkNotify();
+  if (pack && common_flags()->compress_stack_depot)
+    CompressStackStore();
 }
 
 StackDepotNode::args_type StackDepotNode::load(u32 id) const {
@@ -215,13 +113,11 @@ StackTrace StackDepotGet(u32 id) {
 
 void StackDepotLockAll() {
   theDepot.LockAll();
-  compress_thread.LockAndStop();
   stackStore.LockAll();
 }
 
 void StackDepotUnlockAll() {
   stackStore.UnlockAll();
-  compress_thread.Unlock();
   theDepot.UnlockAll();
 }
 
@@ -231,8 +127,6 @@ void StackDepotPrintAll() {
 #endif
 }
 
-void StackDepotStopBackgroundThread() { compress_thread.Stop(); }
-
 StackDepotHandle StackDepotNode::get_handle(u32 id) {
   return StackDepotHandle(&theDepot.nodes[id], id);
 }

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.h b/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.h
index cca6fd5346883..56d655d9404ca 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.h
@@ -42,7 +42,6 @@ StackTrace StackDepotGet(u32 id);
 void StackDepotLockAll();
 void StackDepotUnlockAll();
 void StackDepotPrintAll();
-void StackDepotStopBackgroundThread();
 
 void StackDepotTestOnlyUnmap();
 

diff  --git a/compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp b/compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp
index 6bf067618f950..c64c9392a107b 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/compress_stack_depot.cpp
@@ -1,9 +1,7 @@
 // RUN: %clangxx %s -fsanitize-memory-track-origins=1 -o %t
 // RUN: %env_tool_opts="compress_stack_depot=0:malloc_context_size=128:verbosity=1" %run %t 2>&1 | FileCheck %s --implicit-check-not="StackDepot released"
-// RUN: %env_tool_opts="compress_stack_depot=-1:malloc_context_size=128:verbosity=1" %run %t 2>&1 | FileCheck %s --check-prefixes=COMPRESS
-// RUN: %env_tool_opts="compress_stack_depot=-2:malloc_context_size=128:verbosity=1" %run %t 2>&1 | FileCheck %s --check-prefixes=COMPRESS
-// RUN: %env_tool_opts="compress_stack_depot=1:malloc_context_size=128:verbosity=1" %run %t 2>&1 | FileCheck %s --check-prefixes=COMPRESS,THREAD
-// RUN: %env_tool_opts="compress_stack_depot=2:malloc_context_size=128:verbosity=1" %run %t 2>&1 | FileCheck %s --check-prefixes=COMPRESS,THREAD
+// RUN: %env_tool_opts="compress_stack_depot=1:malloc_context_size=128:verbosity=1" %run %t 2>&1 | FileCheck %s --check-prefixes=COMPRESS
+// RUN: %env_tool_opts="compress_stack_depot=2:malloc_context_size=128:verbosity=1" %run %t 2>&1 | FileCheck %s --check-prefixes=COMPRESS
 
 // Ubsan does not store stacks.
 // UNSUPPORTED: ubsan
@@ -11,8 +9,6 @@
 // FIXME: Fails for unknown reason.
 // UNSUPPORTED: s390x
 
-#include <sanitizer/common_interface_defs.h>
-
 #include <memory>
 
 __attribute__((noinline)) void a(unsigned v);
@@ -39,13 +35,7 @@ __attribute__((noinline)) void b(unsigned v) { return a(v); }
 int main(int argc, char *argv[]) {
   for (unsigned i = 0; i < 100000; ++i)
     a(i + (i << 16));
-
-  __sanitizer_sandbox_arguments args = {0};
-  __sanitizer_sandbox_on_notify(&args);
-
   return 0;
 }
 
-// THREAD: StackDepot compression thread started
 // COMPRESS: StackDepot released {{[0-9]+}}
-// THREAD: StackDepot compression thread stopped


        


More information about the llvm-commits mailing list