llvm dev - Feb 2016 - [PATCH compiler-rt 0/5] Improved support for swapcontext()

Hi,

These patches improve ASan support for swapcontext() and friends mainly
focusing on eliminating false positives caused by throwing and catching
exceptions when on a custom user stack.
This is achieved by adding ability to AsanThread to change current
stack (in an async-safe way).

Some of the things that are still not fully supported:
 - swapcontext() and fake stacks
 - changing context from signal handlers
 - changing context using longjmp()
 - final context change via ucontext_t::uc_link

On x64 Linux 'make check-all' passes.

This series is also available at:

  https://github.com/pdziepak/compiler-rt.git asan-swapcontext-v1

Paweł Dziepak (5):
  asan: make getcontext() and swapcontext() write stack description
  asan: introduce AsanThread::StackDescriptor
  lsan: do not expect leaks in coroutines to be not reported
  asan: better support for swapcontext() and setcontext()
  asan: extend swapcontext_test

 lib/asan/asan_interceptors.cc                 | 42 +++++++++++++++++++++
 lib/asan/asan_internal.h                      |  1 +
 lib/asan/asan_linux.cc                        | 10 +++++
 lib/asan/asan_mac.cc                          |  4 ++
 lib/asan/asan_thread.cc                       | 21 +++++++----
 lib/asan/asan_thread.h                        | 54 ++++++++++++++++++++++-----
 lib/asan/asan_win.cc                          |  4 ++
 test/asan/TestCases/Linux/swapcontext_test.cc | 15 ++++++++
 test/lsan/TestCases/swapcontext.cc            |  4 --
 9 files changed, 134 insertions(+), 21 deletions(-)

-- 
2.5.0

Normally, ucontext_t::uc_stack is only used by makecontext() and not
written to or read from any other *context() functions. However, if
making sure that it contains proper stack description can make improving
support for swapcontext() much easier.
---
 lib/asan/asan_interceptors.cc | 16 ++++++++++++++++
 lib/asan/asan_internal.h      |  1 +
 lib/asan/asan_linux.cc        | 10 ++++++++++
 lib/asan/asan_mac.cc          |  4 ++++
 lib/asan/asan_win.cc          |  4 ++++
 5 files changed, 35 insertions(+)

diff --git a/lib/asan/asan_interceptors.cc b/lib/asan/asan_interceptors.cc
index faac15b..6b1aa00 100644
--- a/lib/asan/asan_interceptors.cc
+++ b/lib/asan/asan_interceptors.cc
@@ -346,6 +346,11 @@ INTERCEPTOR(int, swapcontext, struct ucontext_t *oucp,
   uptr stack, ssize;
   ReadContextStack(ucp, &stack, &ssize);
   ClearShadowMemoryForContextStack(stack, ssize);
+  AsanThread *curr_thread = GetCurrentThread();
+  if (curr_thread) {
+    WriteContextStack(oucp, curr_thread->stack_bottom(),
+                      curr_thread->stack_size());
+  }
   int res = REAL(swapcontext)(oucp, ucp);
   // swapcontext technically does not return, but program may swap context to
   // "oucp" later, that would look as if swapcontext() returned 0.
@@ -354,6 +359,16 @@ INTERCEPTOR(int, swapcontext, struct ucontext_t *oucp,
   ClearShadowMemoryForContextStack(stack, ssize);
   return res;
 }
+
+INTERCEPTOR(int, getcontext, struct ucontext_t *ucp) {
+  int res = REAL(getcontext)(ucp);
+  AsanThread *curr_thread = GetCurrentThread();
+  if (!res && curr_thread) {
+    WriteContextStack(ucp, curr_thread->stack_bottom(),
+                      curr_thread->stack_size());
+  }
+  return res;
+}
 #endif  // ASAN_INTERCEPT_SWAPCONTEXT
 
 INTERCEPTOR(void, longjmp, void *env, int val) {
@@ -796,6 +811,7 @@ void InitializeAsanInterceptors() {
 #endif
 #if ASAN_INTERCEPT_SWAPCONTEXT
   ASAN_INTERCEPT_FUNC(swapcontext);
+  ASAN_INTERCEPT_FUNC(getcontext);
 #endif
 #if ASAN_INTERCEPT__LONGJMP
   ASAN_INTERCEPT_FUNC(_longjmp);
diff --git a/lib/asan/asan_internal.h b/lib/asan/asan_internal.h
index 9391475..5d417eb 100644
--- a/lib/asan/asan_internal.h
+++ b/lib/asan/asan_internal.h
@@ -76,6 +76,7 @@ void AsanCheckIncompatibleRT();
 void AsanOnDeadlySignal(int, void *siginfo, void *context);
 
 void ReadContextStack(void *context, uptr *stack, uptr *ssize);
+void WriteContextStack(void *context, uptr stack, uptr ssize);
 void StopInitOrderChecking();
 
 // Wrapper for TLS/TSD.
diff --git a/lib/asan/asan_linux.cc b/lib/asan/asan_linux.cc
index e26b400..f157e48 100644
--- a/lib/asan/asan_linux.cc
+++ b/lib/asan/asan_linux.cc
@@ -156,10 +156,20 @@ void ReadContextStack(void *context, uptr *stack, uptr
*ssize) {
   *stack = (uptr)ucp->uc_stack.ss_sp;
   *ssize = ucp->uc_stack.ss_size;
 }
+
+void WriteContextStack(void *context, uptr stack, uptr ssize) {
+  ucontext_t *ucp = (ucontext_t*)context;
+  ucp->uc_stack.ss_sp = (void*)stack;
+  ucp->uc_stack.ss_size = ssize;
+}
 #else
 void ReadContextStack(void *context, uptr *stack, uptr *ssize) {
   UNIMPLEMENTED();
 }
+
+void WriteContextStack(void *context, uptr stack, uptr ssize) {
+  UNIMPLEMENTED();
+}
 #endif
 
 void *AsanDlSymNext(const char *sym) {
diff --git a/lib/asan/asan_mac.cc b/lib/asan/asan_mac.cc
index 6975b03..d6a4bc3 100644
--- a/lib/asan/asan_mac.cc
+++ b/lib/asan/asan_mac.cc
@@ -70,6 +70,10 @@ void ReadContextStack(void *context, uptr *stack, uptr
*ssize) {
   UNIMPLEMENTED();
 }
 
+void WriteContextStack(void *context, uptr stack, uptr ssize) {
+  UNIMPLEMENTED();
+}
+
 // Support for the following functions from libdispatch on Mac OS:
 //   dispatch_async_f()
 //   dispatch_async()
diff --git a/lib/asan/asan_win.cc b/lib/asan/asan_win.cc
index 92bd893..2cae8dd 100644
--- a/lib/asan/asan_win.cc
+++ b/lib/asan/asan_win.cc
@@ -190,6 +190,10 @@ void ReadContextStack(void *context, uptr *stack, uptr
*ssize) {
   UNIMPLEMENTED();
 }
 
+void WriteContextStack(void *context, uptr stack, uptr ssize) {
+  UNIMPLEMENTED();
+}
+
 void AsanOnDeadlySignal(int, void *siginfo, void *context) {
   UNIMPLEMENTED();
 }
-- 
2.5.0

StackDescriptor contains all stack information (bottom, top and size),
which will make code implementing better support for swapcontext()
simpler.
---
 lib/asan/asan_thread.cc | 17 +++++++++--------
 lib/asan/asan_thread.h  | 26 +++++++++++++++++++-------
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/lib/asan/asan_thread.cc b/lib/asan/asan_thread.cc
index 526ef3d..33fffec 100644
--- a/lib/asan/asan_thread.cc
+++ b/lib/asan/asan_thread.cc
@@ -154,13 +154,13 @@ void AsanThread::Init() {
   CHECK_EQ(this->stack_size(), 0U);
   SetThreadStackAndTls();
   CHECK_GT(this->stack_size(), 0U);
-  CHECK(AddrIsInMem(stack_bottom_));
-  CHECK(AddrIsInMem(stack_top_ - 1));
+  CHECK(AddrIsInMem(stack_bottom()));
+  CHECK(AddrIsInMem(stack_top() - 1));
   ClearShadowForThreadStackAndTLS();
   int local = 0;
   VReport(1, "T%d: stack [%p,%p) size 0x%zx; local=%p\n", tid(),
-          (void *)stack_bottom_, (void *)stack_top_, stack_top_ -
stack_bottom_,
-          &local);
+          (void *)stack_bottom(), (void *)stack_top(),
+          stack_top() - stack_bottom(), &local);
 }
 
 thread_return_t AsanThread::ThreadStart(
@@ -195,9 +195,10 @@ thread_return_t AsanThread::ThreadStart(
 
 void AsanThread::SetThreadStackAndTls() {
   uptr tls_size = 0;
-  GetThreadStackAndTls(tid() == 0, &stack_bottom_, &stack_size_,
&tls_begin_,
-                       &tls_size);
-  stack_top_ = stack_bottom_ + stack_size_;
+  GetThreadStackAndTls(tid() == 0, &current_stack_.stack_bottom,
+                       &current_stack_.stack_size, &tls_begin_,
&tls_size);
+  current_stack_.stack_top = current_stack_.stack_bottom
+                             + current_stack_.stack_size;
   tls_end_ = tls_begin_ + tls_size;
   dtls_ = DTLS_Get();
 
@@ -206,7 +207,7 @@ void AsanThread::SetThreadStackAndTls() {
 }
 
 void AsanThread::ClearShadowForThreadStackAndTLS() {
-  PoisonShadow(stack_bottom_, stack_top_ - stack_bottom_, 0);
+  PoisonShadow(stack_bottom(), stack_top() - stack_bottom(), 0);
   if (tls_begin_ != tls_end_)
     PoisonShadow(tls_begin_, tls_end_ - tls_begin_, 0);
 }
diff --git a/lib/asan/asan_thread.h b/lib/asan/asan_thread.h
index b05d720..c4670e1 100644
--- a/lib/asan/asan_thread.h
+++ b/lib/asan/asan_thread.h
@@ -66,9 +66,15 @@ class AsanThread {
   thread_return_t ThreadStart(uptr os_id,
                               atomic_uintptr_t *signal_thread_is_registered);
 
-  uptr stack_top() { return stack_top_; }
-  uptr stack_bottom() { return stack_bottom_; }
-  uptr stack_size() { return stack_size_; }
+  struct StackDescriptor {
+    uptr stack_top;
+    uptr stack_bottom;
+    uptr stack_size;
+  };
+
+  uptr stack_top() { return CurrentStack()->stack_top; }
+  uptr stack_bottom() { return CurrentStack()->stack_bottom; }
+  uptr stack_size() { return CurrentStack()->stack_size; }
   uptr tls_begin() { return tls_begin_; }
   uptr tls_end() { return tls_end_; }
   DTLS *dtls() { return dtls_; }
@@ -83,8 +89,16 @@ class AsanThread {
   };
   bool GetStackFrameAccessByAddr(uptr addr, StackFrameAccess *access);
 
+  bool AddrIsInStack(StackDescriptor *stack, uptr addr) {
+    return addr >= stack->stack_bottom && addr <
stack->stack_top;
+  }
+
+  StackDescriptor *CurrentStack() {
+    return &current_stack_;
+  }
+
   bool AddrIsInStack(uptr addr) {
-    return addr >= stack_bottom_ && addr < stack_top_;
+    return AddrIsInStack(CurrentStack(), addr);
   }
 
   void DeleteFakeStack(int tid) {
@@ -130,11 +144,9 @@ class AsanThread {
   AsanThreadContext *context_;
   thread_callback_t start_routine_;
   void *arg_;
-  uptr stack_top_;
-  uptr stack_bottom_;
   // stack_size_ == stack_top_ - stack_bottom_;
   // It needs to be set in a async-signal-safe manner.
-  uptr stack_size_;
+  StackDescriptor current_stack_;
   uptr tls_begin_;
   uptr tls_end_;
   DTLS *dtls_;
-- 
2.5.0

With improved support for swapcontext() and friends lsan may be able
to report some of the leaks that happened in coroutines.
---
 test/lsan/TestCases/swapcontext.cc | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/test/lsan/TestCases/swapcontext.cc
b/test/lsan/TestCases/swapcontext.cc
index f7e95ed..ece0db8 100644
--- a/test/lsan/TestCases/swapcontext.cc
+++ b/test/lsan/TestCases/swapcontext.cc
@@ -1,8 +1,4 @@
-// We can't unwind stack if we're running coroutines on heap-allocated
-// memory. Make sure we don't report these leaks.
-
 // RUN: %clangxx_lsan %s -o %t
-// RUN: %run %t 2>&1
 // RUN: not %run %t foo 2>&1 | FileCheck %s
 
 #include <stdio.h>
-- 
2.5.0

This patch makes swapcontext() and setcontext() update stack information
in AsanThread each time the stack is changed. It is still far from full
support of these functions but at least makes __asan_handle_no_return()
work correctly on a custom user stack.
---
 lib/asan/asan_interceptors.cc | 30 ++++++++++++++++++++++++++++--
 lib/asan/asan_thread.cc       | 12 ++++++++----
 lib/asan/asan_thread.h        | 32 ++++++++++++++++++++++++++++----
 3 files changed, 64 insertions(+), 10 deletions(-)

diff --git a/lib/asan/asan_interceptors.cc b/lib/asan/asan_interceptors.cc
index 6b1aa00..62ecc7c 100644
--- a/lib/asan/asan_interceptors.cc
+++ b/lib/asan/asan_interceptors.cc
@@ -347,11 +347,17 @@ INTERCEPTOR(int, swapcontext, struct ucontext_t *oucp,
   ReadContextStack(ucp, &stack, &ssize);
   ClearShadowMemoryForContextStack(stack, ssize);
   AsanThread *curr_thread = GetCurrentThread();
+  uptr old_stack, old_ssize;
   if (curr_thread) {
-    WriteContextStack(oucp, curr_thread->stack_bottom(),
-                      curr_thread->stack_size());
+    old_stack = curr_thread->stack_bottom();
+    old_ssize = curr_thread->stack_size();
+    WriteContextStack(oucp, old_stack, old_ssize);
+    curr_thread->SetUserStack(stack, ssize);
   }
   int res = REAL(swapcontext)(oucp, ucp);
+  if (curr_thread) {
+    curr_thread->SetUserStack(old_stack, old_ssize);
+  }
   // swapcontext technically does not return, but program may swap context to
   // "oucp" later, that would look as if swapcontext() returned 0.
   // We need to clear shadow for ucp once again, as it may be in arbitrary
@@ -360,6 +366,25 @@ INTERCEPTOR(int, swapcontext, struct ucontext_t *oucp,
   return res;
 }
 
+INTERCEPTOR(int, setcontext, struct ucontext_t *ucp) {
+  uptr stack, ssize;
+  ReadContextStack(ucp, &stack, &ssize);
+  ClearShadowMemoryForContextStack(stack, ssize);
+
+  AsanThread *curr_thread = GetCurrentThread();
+  uptr old_stack, old_ssize;
+  if (curr_thread) {
+      old_stack = curr_thread->stack_bottom();
+      old_ssize = curr_thread->stack_size();
+      curr_thread->SetUserStack(stack, ssize);
+  }
+  int res = REAL(setcontext)(ucp);
+  if (curr_thread) {
+    curr_thread->SetUserStack(old_stack, old_ssize);
+  }
+  return res;
+}
+
 INTERCEPTOR(int, getcontext, struct ucontext_t *ucp) {
   int res = REAL(getcontext)(ucp);
   AsanThread *curr_thread = GetCurrentThread();
@@ -811,6 +836,7 @@ void InitializeAsanInterceptors() {
 #endif
 #if ASAN_INTERCEPT_SWAPCONTEXT
   ASAN_INTERCEPT_FUNC(swapcontext);
+  ASAN_INTERCEPT_FUNC(setcontext);
   ASAN_INTERCEPT_FUNC(getcontext);
 #endif
 #if ASAN_INTERCEPT__LONGJMP
diff --git a/lib/asan/asan_thread.cc b/lib/asan/asan_thread.cc
index 33fffec..3fa3143 100644
--- a/lib/asan/asan_thread.cc
+++ b/lib/asan/asan_thread.cc
@@ -150,6 +150,9 @@ FakeStack *AsanThread::AsyncSignalSafeLazyInitFakeStack() {
 }
 
 void AsanThread::Init() {
+  temp_stack_ = &stacks_[0];
+  next_stack_ = &stacks_[1];
+  previous_stack_ = &stacks_[2];
   fake_stack_ = nullptr;  // Will be initialized lazily if needed.
   CHECK_EQ(this->stack_size(), 0U);
   SetThreadStackAndTls();
@@ -195,10 +198,11 @@ thread_return_t AsanThread::ThreadStart(
 
 void AsanThread::SetThreadStackAndTls() {
   uptr tls_size = 0;
-  GetThreadStackAndTls(tid() == 0, &current_stack_.stack_bottom,
-                       &current_stack_.stack_size, &tls_begin_,
&tls_size);
-  current_stack_.stack_top = current_stack_.stack_bottom
-                             + current_stack_.stack_size;
+  GetThreadStackAndTls(tid() == 0, &next_stack_->stack_bottom,
+                       &next_stack_->stack_size, &tls_begin_,
&tls_size);
+  next_stack_->stack_top = next_stack_->stack_bottom +
next_stack_->stack_size;
+  previous_stack_->stack_top = previous_stack_->stack_bottom = 0;
+  previous_stack_->stack_size = 0;
   tls_end_ = tls_begin_ + tls_size;
   dtls_ = DTLS_Get();
 
diff --git a/lib/asan/asan_thread.h b/lib/asan/asan_thread.h
index c4670e1..850b04c 100644
--- a/lib/asan/asan_thread.h
+++ b/lib/asan/asan_thread.h
@@ -82,6 +82,17 @@ class AsanThread {
   AsanThreadContext *context() { return context_; }
   void set_context(AsanThreadContext *context) { context_ = context; }
 
+  void SetUserStack(uptr base, uptr size) {
+    temp_stack_->stack_bottom = base;
+    temp_stack_->stack_top = base + size;
+    temp_stack_->stack_size = size;
+
+    StackDescriptor* oprev = previous_stack_;
+    previous_stack_ = next_stack_;
+    next_stack_ = temp_stack_;
+    temp_stack_ = oprev;
+  }
+
   struct StackFrameAccess {
     uptr offset;
     uptr frame_pc;
@@ -94,7 +105,12 @@ class AsanThread {
   }
 
   StackDescriptor *CurrentStack() {
-    return &current_stack_;
+    int local;
+    if (AddrIsInStack(previous_stack_, (uptr)&local)) {
+        return previous_stack_;
+    } else {
+        return next_stack_;
+    }
   }
 
   bool AddrIsInStack(uptr addr) {
@@ -144,9 +160,17 @@ class AsanThread {
   AsanThreadContext *context_;
   thread_callback_t start_routine_;
   void *arg_;
-  // stack_size_ == stack_top_ - stack_bottom_;
-  // It needs to be set in a async-signal-safe manner.
-  StackDescriptor current_stack_;
+
+  // We have three stack descriptors for async-safe stack change. New stack
+  // information is written to temp_stack_. Then previous_stack_ is made to
+  // point to the same descriptor that next_stack_ does. Finally, temp_stack_
+  // is assigned to next_stack_. The result is that at any time either
+  // previous_stack_ or next_stack_ contain the correct stack information.
+  StackDescriptor stacks_[3];
+  StackDescriptor* temp_stack_;
+  StackDescriptor* next_stack_;
+  StackDescriptor* previous_stack_;
+
   uptr tls_begin_;
   uptr tls_end_;
   DTLS *dtls_;
-- 
2.5.0

This patch extends swapcontext_test to verify that the shadow memory
of the stack is properly cleared after throw.
---
 test/asan/TestCases/Linux/swapcontext_test.cc | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/test/asan/TestCases/Linux/swapcontext_test.cc
b/test/asan/TestCases/Linux/swapcontext_test.cc
index 86ed593..279566c 100644
--- a/test/asan/TestCases/Linux/swapcontext_test.cc
+++ b/test/asan/TestCases/Linux/swapcontext_test.cc
@@ -17,16 +17,31 @@ ucontext_t child_context;
 
 const int kStackSize = 1 << 20;
 
+volatile int force_write;
+
 __attribute__((noinline))
 void Throw() {
+  char buf[1024];
+  for (int i = 0; i < 1024; i++) {
+    buf[i] = force_write;
+  }
   throw 1;
 }
 
 __attribute__((noinline))
+void Func() {
+  int buf[4 * 1024];
+  for (int i = 0; i < 4 * 1024; i++) {
+    buf[i] = force_write;
+  }
+}
+
+__attribute__((noinline))
 void ThrowAndCatch() {
   try {
     Throw();
   } catch(int a) {
+    Func();
     printf("ThrowAndCatch: %d\n", a);
   }
 }
-- 
2.5.0

Paweł,
thanks for the patches, but please use http://llvm.org/docs/Phabricator.html

On Tue, Feb 2, 2016 at 6:42 AM, Paweł Dziepak <llvm-dev at lists.llvm.org>
wrote:
> Hi,
>
> These patches improve ASan support for swapcontext() and friends mainly
> focusing on eliminating false positives caused by throwing and catching
> exceptions when on a custom user stack.
> This is achieved by adding ability to AsanThread to change current
> stack (in an async-safe way).
>
> Some of the things that are still not fully supported:
>  - swapcontext() and fake stacks
>  - changing context from signal handlers
>  - changing context using longjmp()
>  - final context change via ucontext_t::uc_link
>
> On x64 Linux 'make check-all' passes.
>
> This series is also available at:
>
>   https://github.com/pdziepak/compiler-rt.git asan-swapcontext-v1
>
> Paweł Dziepak (5):
>   asan: make getcontext() and swapcontext() write stack description
>   asan: introduce AsanThread::StackDescriptor
>   lsan: do not expect leaks in coroutines to be not reported
>   asan: better support for swapcontext() and setcontext()
>   asan: extend swapcontext_test
>
>  lib/asan/asan_interceptors.cc                 | 42 +++++++++++++++++++++
>  lib/asan/asan_internal.h                      |  1 +
>  lib/asan/asan_linux.cc                        | 10 +++++
>  lib/asan/asan_mac.cc                          |  4 ++
>  lib/asan/asan_thread.cc                       | 21 +++++++----
>  lib/asan/asan_thread.h                        | 54
> ++++++++++++++++++++++-----
>  lib/asan/asan_win.cc                          |  4 ++
>  test/asan/TestCases/Linux/swapcontext_test.cc | 15 ++++++++
>  test/lsan/TestCases/swapcontext.cc            |  4 --
>  9 files changed, 134 insertions(+), 21 deletions(-)
>
> --
> 2.5.0
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160202/296d9863/attachment.html>