[PATCH] D29786: Use pthreads to manage thread-local storage on darwin for leak sanitizer

[Kuba (Brecka) Mracek via Phabricator via llvm-commits]

kubamracek added inline comments.


================
Comment at: lib/lsan/lsan_common_mac.cc:28
+
+static int make_tls_key() { CHECK_EQ(pthread_key_create(&key, NULL), 0); }
+
----------------
The declaration says the function returns "int", but the function body doesn't return anything.


================
Comment at: lib/lsan/lsan_common_mac.cc:35
+  if (ptr == NULL && allocate) {
+    ptr = (int *)malloc(sizeof(0));
+    *ptr = 0;
----------------
sizeof(int)


================
Comment at: lib/lsan/lsan_common_mac.cc:55-58
+  if (!*disable_counter && common_flags()->detect_leaks) {
+    Report("Unmatched call to __lsan_enable().\n");
+    Die();
+  }
----------------
I would prefer to replace the `!` with an explicit `== 0` (and change the lsan_common_linux.cc file as well) to improve readability.

Also, can we de-duplicate these 4 lines?  Perhaps having just `if (*disable_counter > 0) DisableCounterUnderflow();` and defining the new function in lsan_common.cc. 


================
Comment at: lib/lsan/lsan_common_mac.cc:60
+
+  if (common_flags()->detect_leaks) {
+    --*disable_counter;
----------------
Why not decrement unconditionally here?  The Linux implementation does it unconditionally.


https://reviews.llvm.org/D29786