[PATCH] D38854: [scudo] Allow for non-Android Shared TSD platforms, part 2

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad added inline comments.


================
Comment at: lib/scudo/scudo_platform.h:26
 // Android and Fuchsia use a pool of TSDs shared between threads.
-# define SCUDO_TSD_EXCLUSIVE 0
-#elif SANITIZER_LINUX && !SANITIZER_ANDROID
+#  define SCUDO_TSD_EXCLUSIVE 0
+# elif SANITIZER_LINUX && !SANITIZER_ANDROID
----------------
alekseyshl wrote:
> What's going to happen if we define SCUDO_TSD_EXCLUSIVE at compile time on these platforms?
It will blow up at runtime (see main comment of the patch).
Android doesn't have ELF TLS but it's planned for some point. They do have emutls but it doesn't work for us.
Fuchsia I think supports a non emulated TLS, I'd have to double check.
I can put some safeguards?


================
Comment at: lib/scudo/scudo_tsd_shared.cpp:21
 static pthread_once_t GlobalInitialized = PTHREAD_ONCE_INIT;
-static pthread_key_t PThreadKey;
+pthread_key_t PThreadKey;
 
----------------
alekseyshl wrote:
> Why is it not static anymore?
It is used from scudo_tsd_shared.inc (defined as extern there).


================
Comment at: lib/scudo/scudo_tsd_shared.cpp:35
 static void initOnce() {
-  // Hack: TLS_SLOT_TSAN was introduced in N. To be able to use it on M for
-  // testing, we create an unused key. Since the key_data array follows the tls
-  // array, it basically gives us the extra entry we need.
-  // TODO(kostyak): remove and restrict to N and above.
   CHECK_EQ(pthread_key_create(&PThreadKey, NULL), 0);
   initScudo();
----------------
alekseyshl wrote:
> You do not need this PThreadKey on Android, right? Maybe it does make sense to separate implementations along these lines? Move PThreadKey and Android into separate cpp files and link what you need?
Correct, it's not needed, it's useful on Android to makes things work on older platforms (the comment that I removed explains it).
I guess the issue with the separation is that it will end up affecting the .inc as well, which would have to be split or have further #ifs (the pthread key is used there). If you see a way to make this work, I can accommodate that.


https://reviews.llvm.org/D38854