[PATCH] D90195: [GWP-ASan] Abstract the thread local variables access

[Mitch Phillips via Phabricator via llvm-commits]

hctim added inline comments.


================
Comment at: compiler-rt/lib/gwp_asan/guarded_pool_allocator.h:246
+  ThreadLocalPackedVariables *getThreadLocals() {
+#if GWP_ASAN_HAS_PLATFORM_TLS_SLOT
+    return reinterpret_cast<ThreadLocalPackedVariables *>(
----------------
Prefer not to have #ifdefs in generic code.

Can you instead have:
```
>> here:
#include "gwp_asan/platform_specific/guarded_pool_allocator_tls.h"

>> gwp_asan/platform_specific/guarded_pool_allocator_tls.h:
struct ThreadLocalPackedVariables { ... }
#if defined(__Fuchsia__)
#include "gwp_asan/platform_specific/guarded_pool_allocator_tls_fuchsia.h"
#else if defined(__unix__)
#include "gwp_asan/platform_specific/guarded_pool_allocator_tls_posix.h"
#endif

>> gwp_asan/platform_specific/guarded_pool_allocator_tls_fuchsia.h
static ThreadLocalPackedVariables *getThreadLocals() {
  return reinterpret_cast<ThreadLocalPackedVariables *>(
        getPlatformGwpAsanTlsSlot());
}

>> gwp_asan/platform_specific/guarded_pool_allocator_tls_posix.h:
static ThreadLocalPackedVariables *getThreadLocals() {
    alignas(8) static GWP_ASAN_TLS_INITIAL_EXEC ThreadLocalPackedVariables Locals;
    return &Locals;
}
```

I think that if we could rely on LTO to inline things correctly then just the implementation would go in a platform-specific cpp file - but unfortunately we don't have that luxury everywhere, so it seems to me like the best solution to ensure inlining is platform-specific headers. In future, I'd like to give mutex.h the same treatment, as I'm really unhappy about the platform ifdefs there as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90195