[PATCH] D61923: [GWP-ASan] Mutex implementation [2].

[Mitch Phillips via Phabricator via llvm-commits]

hctim added a comment.

In D61923#1502245 <https://reviews.llvm.org/D61923#1502245>, @jfb wrote:

> Seems a shame to duplicate mutex again... Why can't use use the STL's version again? It doesn't allocate.


We can't use `std::mutex` as we must be C-compatible (and can't make the strong guarantee that `std::mutex` is header-only), see https://reviews.llvm.org/D60593#inline-537276.
We also are trying to stay independent of `pthread` for platform-independentness.
We also can't use a `sanitizer_common` variant as we don't want to pull in the entirety `sanitizer_common` as a dependency.

Basically, the roadmap is to create a shared mutex library for `sanitizer_common`, `gwp_asan` and `scudo` to all use.

In D61923#1502245 <https://reviews.llvm.org/D61923#1502245>, @jfb wrote:

> Tests?


Added some unit tests.



================
Comment at: compiler-rt/lib/gwp_asan/mutex.h:32
+private:
+  void yieldProcessor(uint8_t Count);
+  void yieldPlatform();
----------------
jfb wrote:
> `uint8_t` seems easy to inadvertently truncate. If you want to restrict size, make it a template and refuse to compile above a certain size. It's always `10` anyways.
Seeming as this is an entirely-internal number, I've made it a global instead.


================
Comment at: compiler-rt/lib/gwp_asan/mutex.h:81
+    else
+      yieldPlatform();
+
----------------
jfb wrote:
> This won't compile if `__unix__` isn't defined.
Should be guarded by `if (COMPILER_RT_HAS_GWP_ASAN)` at `lib/gwp_asan/CMakeLists.txt:20`, but have added explicit error condition here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61923