[compiler-rt] 9be8892 - [asan] Fix GC of FakeFrames

[Vitaly Buka via llvm-commits]

Author: Vitaly Buka
Date: 2023-08-09T20:47:23-07:00
New Revision: 9be8892908d49c19fd6c9fc930d0f41276c3e345

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

LOG: [asan] Fix GC of FakeFrames

When FakeStack GC from altstack, it may see default stack on lower
addressed and incorectly disard all frames.

Fixes bug exposed by D153536.

Reviewed By: kstoimenov

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

Added: 
    compiler-rt/test/asan/TestCases/Posix/fake_stack_gc.cpp

Modified: 
    compiler-rt/lib/asan/asan_fake_stack.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/asan/asan_fake_stack.cpp b/compiler-rt/lib/asan/asan_fake_stack.cpp
index 72d1920579e997..8680bc6d6630e2 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.cpp
+++ b/compiler-rt/lib/asan/asan_fake_stack.cpp
@@ -133,6 +133,12 @@ void FakeStack::HandleNoReturn() {
   needs_gc_ = true;
 }
 
+// Hack: The statement below is not true if we take into account sigaltstack or
+// makecontext. It should be possible to make GC to discard wrong stack frame if
+// we use these tools. For now, let's support the simplest case and allow GC to
+// discard only frames from the default stack, assuming there is no buffer on
+// the stack which is used for makecontext or sigaltstack.
+//
 // When throw, longjmp or some such happens we don't call OnFree() and
 // as the result may leak one or more fake frames, but the good news is that
 // we are notified about all such events by HandleNoReturn().
@@ -140,6 +146,14 @@ void FakeStack::HandleNoReturn() {
 // We do it based on their 'real_stack' values -- everything that is lower
 // than the current real_stack is garbage.
 NOINLINE void FakeStack::GC(uptr real_stack) {
+  AsanThread *curr_thread = GetCurrentThread();
+  if (!curr_thread)
+    return;  // Try again when we have a thread.
+  auto top = curr_thread->stack_top();
+  auto bottom = curr_thread->stack_bottom();
+  if (real_stack < top || real_stack > bottom)
+    return;  // Not the default stack.
+
   for (uptr class_id = 0; class_id < kNumberOfSizeClasses; class_id++) {
     u8 *flags = GetFlags(stack_size_log(), class_id);
     for (uptr i = 0, n = NumberOfFrames(stack_size_log(), class_id); i < n;
@@ -147,8 +161,12 @@ NOINLINE void FakeStack::GC(uptr real_stack) {
       if (flags[i] == 0) continue;  // not allocated.
       FakeFrame *ff = reinterpret_cast<FakeFrame *>(
           GetFrame(stack_size_log(), class_id, i));
-      if (ff->real_stack < real_stack) {
+      // GC only on the default stack.
+      if (ff->real_stack < real_stack && ff->real_stack >= top) {
         flags[i] = 0;
+        // Poison the frame, so the any access will be reported as UAR.
+        SetShadow(reinterpret_cast<uptr>(ff), BytesInSizeClass(class_id),
+                  class_id, kMagic8);
       }
     }
   }

diff  --git a/compiler-rt/test/asan/TestCases/Posix/fake_stack_gc.cpp b/compiler-rt/test/asan/TestCases/Posix/fake_stack_gc.cpp
new file mode 100644
index 00000000000000..57c07187b1b138
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/Posix/fake_stack_gc.cpp
@@ -0,0 +1,92 @@
+// RUN: %clangxx_asan -O0 -pthread %s -o %t && %env_asan_opts=use_sigaltstack=0 %run not --crash %t 2>&1 | FileCheck %s
+
+// Check that fake stack does not discard frames on the main stack, when GC is
+// triggered from high alt stack.
+
+#include <algorithm>
+#include <assert.h>
+#include <csignal>
+#include <cstdint>
+#include <pthread.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+const size_t kStackSize = 0x100000;
+
+int *on_thread;
+int *p;
+
+template <size_t N> void Fn() {
+  int t[N];
+  p = t;
+  if constexpr (N > 1)
+    Fn<N - 1>();
+}
+
+static void Handler(int signo) {
+  fprintf(stderr, "Handler Frame:%p\n", __builtin_frame_address(0));
+
+  // Trigger GC and create a lot of frame to reuse "Thread" frame if it was
+  // discarded.
+  for (int i = 0; i < 1000; ++i)
+    Fn<1000>();
+  // If we discarder and reused "Thread" frame, the next line will crash with
+  // false report.
+  *on_thread = 10;
+  fprintf(stderr, "SUCCESS\n");
+  // CHECK: SUCCESS
+}
+
+void *Thread(void *arg) {
+  fprintf(stderr, "Thread Frame:%p\n", __builtin_frame_address(0));
+  stack_t stack = {
+      .ss_sp = arg,
+      .ss_flags = 0,
+      .ss_size = kStackSize,
+  };
+  assert(sigaltstack(&stack, nullptr) == 0);
+
+  struct sigaction sa = {};
+  sa.sa_handler = Handler;
+  sa.sa_flags = SA_ONSTACK;
+  sigaction(SIGABRT, &sa, nullptr);
+
+  // Store pointer to the local var, so we can access this frame from the signal
+  // handler when the frame is still alive.
+  int n;
+  on_thread = &n;
+
+  // Abort should schedule FakeStack GC and call handler on alt stack.
+  abort();
+}
+
+int main(void) {
+  // Allocate main and alt stack for future thread.
+  void *main_stack = malloc(kStackSize);
+  void *alt_stack = malloc(kStackSize);
+
+  // Pick the lower stack as the main stack, as we want to trigger GC in
+  // FakeStack from alt stack in a such way that main stack is allocated below.
+  if ((uintptr_t)main_stack > (uintptr_t)alt_stack)
+    std::swap(alt_stack, main_stack);
+
+  pthread_attr_t attr;
+  assert(pthread_attr_init(&attr) == 0);
+  assert(pthread_attr_setstack(&attr, main_stack, kStackSize) == 0);
+
+  fprintf(stderr, "main_stack: %p-%p\n", main_stack,
+          (char *)main_stack + kStackSize);
+  fprintf(stderr, "alt_stack: %p-%p\n", alt_stack,
+          (char *)alt_stack + kStackSize);
+
+  pthread_t tid;
+  assert(pthread_create(&tid, &attr, Thread, alt_stack) == 0);
+
+  pthread_join(tid, nullptr);
+
+  free(main_stack);
+  free(alt_stack);
+
+  return 0;
+}