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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to