[PATCH] D86471: [MSAN] Add fiber switching APIs

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: compiler-rt/lib/msan/msan_thread.cpp:99
+  if (cur_stack >= next_stack_bottom_ && cur_stack < next_stack_top_) {
+    return {next_stack_bottom_, next_stack_top_};
+  }
----------------
Please drop {} for single line if/for for consistency


================
Comment at: compiler-rt/lib/msan/msan_thread.cpp:114
+void MsanThread::StartSwitchFiber(uptr bottom, uptr size) {
+  if (atomic_load(&stack_switching_, memory_order_relaxed)) {
+    Report("ERROR: starting fiber switch while in fiber switch\n");
----------------
if we don't expect multiple threads access this field, then we don't need atomic
if we do expect that, this atomic does not solve data race.

>From what I see stack_top() stack_bottom()  only used from BufferedStackTrace::UnwindImpl for current thread.

So please remove remove stack_switching_ and simplify GetStackBounds()  (next_stack_top_ == next_stack_bottom_ should be enought? )



================
Comment at: compiler-rt/lib/msan/msan_thread.h:64-65
+
   uptr stack_top_;
   uptr stack_bottom_;
+
----------------



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86471/new/

https://reviews.llvm.org/D86471