[PATCH] D39175: [XRay][compiler-rt] Remove C++ STL from the buffer queue implementation

Martin Pelikán via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Oct 22 23:17:14 PDT 2017


pelikan accepted this revision.
pelikan added inline comments.
This revision is now accepted and ready to land.


================
Comment at: compiler-rt/lib/xray/xray_buffer_queue.cc:68
+    if (*I == Buf.Buffer) {
+      Found = true;
+      break;
----------------
I'm slightly worried about what happens if we mark this "true", then a signal happens, the handler will be instrumented and this thing will be reentered and succeeds past the mutex-guarded thing at the bottom.  We will resume afterwards and continue with incrementing First yet again, possibly wasting a buffer.

SpinMutex is clever enough to yield in case there is a different thread waiting (which isn't here) but not to be aware of signal processing.  I think this should be addressed in a different diff though, using a lock-less mechanism.  Shouldn't be too tricky for this simple case.


================
Comment at: compiler-rt/lib/xray/xray_buffer_queue.h:39
+    Buffer Buffer;
+    bool Used = false;
+  };
----------------
I was having a bit of a problem realizing whether this means "is being used" or "has been used".  Maybe you can think of a small comment to improve this, maybe not.


https://reviews.llvm.org/D39175





More information about the llvm-commits mailing list