[PATCH] D20913: [asan] add primitives that allow coroutine implementations

[Philippe Daouadi via llvm-commits]

blastrock added a comment.

In http://reviews.llvm.org/D20913#447864, @dvyukov wrote:

> Philippe, what coroutine implementation do you use?


I use boost::context as you seem to have guessed

> We should intercept and annotate at least ucontext API, so that applications using ucontext work out of the box. It will also simplify testing and make it more realistic, as we will just need to write a real program using ucontext.


So you mean redefining makecontext and swapcontext in compiler-rt, annotate them and call libc's implementation after that? Is there an example of such a code for another api?

> Unfortunately, boost::context uses own assembly to implement coroutines, so we still need to expose annotations. We can talk to boost maintainers to add annotations directly to boost::context.


I can try to make them a patch for that when I'm done with this. For the moment I added the annotations around boost::context calls.


================
Comment at: lib/asan/asan_thread.cc:365
@@ +364,3 @@
+
+void NOINLINE __asan_enter_fiber(const void *stack_top,
+                                 const void *stack_bottom) {
----------------
kcc wrote:
> Is this going to be public interface that we want to allow users to call? 
> If yes, it should be __sanitizer_enter_fiber and *also* declared in include/sanitizer/common_interface_defs.h
> (It's ok if we only implement it in asan for now, just explain it in comments in common_interface_defs.h)
Oh, you mean that __asan functions are meant to be called by the code generated by the compiler while __sanitizer functions are meant to be called by the user directly? Anyway, I'll change that.

================
Comment at: lib/asan/asan_thread.h:104
@@ +103,3 @@
+    if (bottom > top)
+      Report("WARNING: __asan_enter_fiber called with inverted stack\n");
+    if (fiber_stack_top_ || fiber_stack_bottom_ || fiber_stack_size_)
----------------
kcc wrote:
> should this be a hard failure? 
I wasn't sure. I can make this a hard failure.

Facebook's folly uses a slightly different interface which looks like enterfiber(uptr bottom, uint size), this kind of interface would remove the need for such a check. Would you prefer me to change it to that?

================
Comment at: lib/asan/asan_thread.h:159
@@ +158,3 @@
+  uptr fiber_stack_bottom_;
+  uptr fiber_stack_size_;
+
----------------
kcc wrote:
> Why do you need this as a variable? Isn't a function better? 
I agree, I was just copying what was done with the stack_* variables. I'll make this a function.


http://reviews.llvm.org/D20913