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

[Alexey Samsonov via llvm-commits]

samsonov added a subscriber: samsonov.

================
Comment at: lib/asan/asan_interceptors.cc:351
@@ +350,3 @@
+  uptr old_stack, old_ssize;
+  if (curr_thread) {
+    old_stack = curr_thread->stack_bottom();
----------------
kcc wrote:
> do you know a situation where curr_thread is nullptr? same below
Yeah, I believe this happens during thread destruction.

================
Comment at: lib/asan/asan_interceptors.cc:354
@@ +353,3 @@
+    old_ssize = curr_thread->stack_size();
+    WriteContextStack(oucp, old_stack, old_ssize);
+    curr_thread->SetUserStack(stack, ssize);
----------------
Won't these be overwritten anyway by real swapcontext(), when it will be copying ucp to oucp?

================
Comment at: lib/asan/asan_interceptors.cc:392
@@ +391,3 @@
+  if (!res && curr_thread) {
+    WriteContextStack(ucp, curr_thread->stack_bottom(),
+                      curr_thread->stack_size());
----------------
Why do you need this?

================
Comment at: lib/asan/asan_thread.cc:204
@@ +203,3 @@
+  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;
----------------
Maybe, just add a constructor for StackDescriptor that would fill everything with zeroes?

================
Comment at: lib/asan/asan_thread.h:170
@@ +169,3 @@
+  StackDescriptor stacks_[3];
+  StackDescriptor* temp_stack_;
+  StackDescriptor* next_stack_;
----------------
Note: you can replace three pointers with a single "int current_stack_;", where
next_stack_ = &stacks_[current_stack_];
previous_stack_ = &stacks_[(current_stack_ + 1) % 3];
temp_stack_ = &stacks_[(current_stack_ + 2) % 3];

================
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
----------------
kcc wrote:
> don't we still want to test the case with heap memory? 
We definitely still want to run it. Can we predict if LSan will or won't report leak in this case?


http://reviews.llvm.org/D16844