[PATCH] D16844: [asan] Improved support for swapcontext()

Kostya Serebryany via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 3 18:30:49 PST 2016


kcc added a comment.

Thanks for this work, but, just curious, what is your motivation? 
What kind of software are you trying to apply asan to?
I haven't heard about swapcontext for a loooong time.

The change in AsanThread is quite intrusive and potentially risky. 
While I don't see what may go wrong, I'd like to have more tests, e.g. with threads, leaks, signals, etc.

Besides, I'd like to have another pair of eyes here. Anyone?


================
Comment at: lib/asan/asan_interceptors.cc:350
@@ +349,3 @@
+  AsanThread *curr_thread = GetCurrentThread();
+  uptr old_stack, old_ssize;
+  if (curr_thread) {
----------------
initialize these just in case (with =0); same below. 

================
Comment at: lib/asan/asan_interceptors.cc:351
@@ +350,3 @@
+  uptr old_stack, old_ssize;
+  if (curr_thread) {
+    old_stack = curr_thread->stack_bottom();
----------------
do you know a situation where curr_thread is nullptr? same below

================
Comment at: test/asan/TestCases/Linux/swapcontext_test.cc:32
@@ -25,1 +31,3 @@
 __attribute__((noinline))
+void Func() {
+  int buf[4 * 1024];
----------------
shouldn't we call Func from Throw() or some such? 

================
Comment at: test/lsan/TestCases/swapcontext.cc:5
@@ -4,3 +1,2 @@
 // RUN: %clangxx_lsan %s -o %t
-// RUN: %run %t 2>&1
 // RUN: not %run %t foo 2>&1 | FileCheck %s
----------------
don't we still want to test the case with heap memory? 


http://reviews.llvm.org/D16844





More information about the llvm-commits mailing list