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

Filipe Cabecinhas via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 20 10:08:53 PDT 2016


filcab accepted this revision.
filcab added a reviewer: filcab.
filcab added a comment.

In http://reviews.llvm.org/D20913#461772, @blastrock wrote:

> I think I got how fake stacks work. I implemented your idea @filcab, and with your test code, the FakeFrame is not collected anymore.


Yay!

> Now we must provide a void* to store the current FakeStack before a switch occurs. During a fiber switch, the FakeStack mechanism is disabled (I understand that it's ok to have __asan_stack_malloc() return null), I couldn't find a solution without disabling it. One consequence is that the fake frame that should be allocated when entering the fiber is not, only the functions called from there have one.

>  When leaving a fiber definitely, null must be passed as first argument so that instead of saving the fake stack, it gets destroyed.


As I commented above, I think adding a way to kill a fake stack without being in the fiber (so: from outside the fiber that "has" that fake stack) would be nice. But I'm ok with doing this internally if there's no interest in it from open source.
In your fiber library, is a use case where the fiber returns to the thread and then re-enters possible? And is it possible to have the fiber go back to the thread and the thread decide to either switch back to the fiber or kill the fiber?

> I also removed the test I wrote which doesn't seem to test anything.


Fair enough. I don't see a way of looking at this bug without adding those printfs.

I think this patch looks good. Just ping back with the documentation before committing. If I don't reply after one day, assume I have no complaints or will address them later.

Thanks a lot!


================
Comment at: include/sanitizer/common_interface_defs.h:145
@@ -142,1 +144,3 @@
+                                      const void *bottom, size_t size);
+  void __sanitizer_finish_switch_fiber(void* fake_stack_save);
 #ifdef __cplusplus
----------------
These still need documentation.
We probably want a way to collect fake stacks that we saved in a fiber (`__sanitizer_fake_stack_kill(void *)` or something), instead of just the "current" fake stack (when we're leaving the fiber).

Our fiber library doesn't really have a "I'm now leaving the fiber and won't come back" function. Execution flow can do something like thread->fiberA->fiberB->thread->fiberC->fiberA->thread->fiberB->kill(fiberA)->fiberC->...
And users can terminate a fiber at any time, as long as it's not running.
In order to support this kind of use-case, we need to be able to kill the fake stack without being inside the fiber. I can implement this internally if you think this is not very general. But depending on your fiber library, I'd guess this is likely to appear elsewhere too.


http://reviews.llvm.org/D20913





More information about the llvm-commits mailing list