[PATCH] D90351: [GWP-ASan] Add mutexes for Fuchsia

[Mitch Phillips via Phabricator via llvm-commits]

hctim accepted this revision.
hctim added a comment.

LGTM w/ nits.



================
Comment at: compiler-rt/lib/gwp_asan/platform_specific/mutex_fuchsia.cpp:11
+
+#if defined(__Fuchsia__)
+
----------------
Nit: Remove #ifdefs from platform-specific code. Should only be built on that platform, so this should be unnecessary.


================
Comment at: compiler-rt/lib/gwp_asan/platform_specific/mutex_fuchsia.cpp:14
+namespace gwp_asan {
+void Mutex::lock() __TA_NO_THREAD_SAFETY_ANALYSIS { sync_mutex_lock(&Mu); }
+
----------------
Nit: IWYU - please include `lib/sync/mutex.h` in the cpp as well.


================
Comment at: compiler-rt/lib/gwp_asan/platform_specific/mutex_posix.h:12
+
+#if defined(__unix__)
+#include <pthread.h>
----------------
Tiny nit just to be clear that "this whole file should be a nop if this isn't posix", can you move the `#if defined(__unix__)` into the same block as the header guards, move the `endif` down into the same block as the `endif` for the header guards, and do the same in the fuchsia header?

Like:

```
#if defined(__unix__)
#ifndef GWP_ASAN_MUTEX_POSIX_H_
#define GWP_ASAN_MUTEX_POSIX_H_

#include <pthread.h>

namespace ...

} // namespace ...

#endif // GWP_ASAN_MUTEX_POSIX_H_
#endif // defined(__unix__)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90351