This is a part of https://reviews.llvm.org/D95835. This change is to address two problems 1) When recording stacks in origin tracking, libunwind is not async signal safe. Inside signal callbacks, we need to use fast unwind. Fast unwind needs threads 2) StackDepot used by origin tracking is not async signal safe, we set a flag per thread inside a signal callback to prevent from using it. The thread registration is similar to ASan and MSan. Related MSan changes are * https://github.com/llvm/llvm-project/commit/98f5ea0dbae664a2e5f9381a64f2913fe1add208 * https://github.com/llvm/llvm-project/commit/f653cda2695ac7390fe5663f2c0895213938334d * https://github.com/llvm/llvm-project/commit/5a7c3643437c262137bd3dac7f6a0f5b9e8501be Some changes in the diff are used in the next diffs 1) The test case pthread.c is not very interesting for now. It will be extended to test origin tracking later. 2) DFsanThread::InSignalHandler will be used by origin tracking later.
Details
- Reviewers
morehouse - Commits
- rG0f3fd3b2810d: [dfsan] Add thread registration
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
compiler-rt/lib/dfsan/dfsan.cpp | ||
---|---|---|
434 | Isn't the else block dead code? Getting past the first if block implies WillUseFastUnwind is true. | |
compiler-rt/lib/dfsan/dfsan_custom.cpp | ||
486–488 | Since it isn't used by anything else, let's inline dfsan_pthread_create here. | |
compiler-rt/lib/dfsan/dfsan_thread.cpp | ||
27 | nit | |
70 | stack_switching_ is always false. | |
77 | next_stack_ is never set anywhere. | |
109 | Is the cast needed? | |
compiler-rt/lib/dfsan/dfsan_thread.h | ||
55 | If we use the proper function ptr type here and in the constructor, we can remove some unnecessary casting to void * and back. | |
compiler-rt/test/dfsan/pthread.c | ||
19 | Is the cast needed? | |
21 | Nit: let's use a const variable instead of magic number 24. |
addressed comments
compiler-rt/lib/dfsan/dfsan.cpp | ||
---|---|---|
434 | Yes. This branch cannot happen. Removed. | |
compiler-rt/lib/dfsan/dfsan_custom.cpp | ||
486–488 | We will use it after origin tracking is added. | |
compiler-rt/lib/dfsan/dfsan_thread.cpp | ||
70 | removed | |
77 | removed it with stack_switching_. In MSan/ASan threads, they expose APIs to allow users tell them when a fiber thread switch contexts. DFSan does not have the case yet. | |
109 | removed. | |
compiler-rt/lib/dfsan/dfsan_thread.h | ||
55 | This start_routine_trampoline_ will have a different type with and without origin tracking like this typedef void *(*thread_callback_trampoline_t)(void *, void *, dfsan_label, dfsan_label *); typedef void *(*thread_callback_origin_trampoline_t)( void *, void *, dfsan_label, dfsan_label *, dfsan_origin, dfsan_origin *); If we use different types here at the C code of dfsan_custom.c, we have to define two functions with similar code. | |
compiler-rt/test/dfsan/pthread.c | ||
19 | removed. | |
21 | added kNumThreads=4. |
compiler-rt/lib/dfsan/dfsan_thread.cpp | ||
---|---|---|
62 | Please address this lint. |
compiler-rt/lib/dfsan/dfsan_thread.cpp | ||
---|---|---|
62 | For some reason the local lint conflicts with the one used here, they propose two different styles... maybe they use different versions..? |
Isn't the else block dead code? Getting past the first if block implies WillUseFastUnwind is true.