[PATCH] Interceptors etc for standalone LSan.

[Alexander Potapenko]

================
Comment at: lib/lsan/lsan_allocator.h:20
@@ +19,3 @@
+#include "sanitizer_common/sanitizer_internal_defs.h"
+#include "lsan.h"
+
----------------
Pls fix the order of includes here and below.

================
Comment at: lib/lsan/lsan_allocator.h:34
@@ +33,3 @@
+void *Allocate(StackTrace const &stack, uptr size, uptr alignment,
+               bool cleared = false);
+void Deallocate(void *p);
----------------
I think it's better to avoid the default parameter here.

================
Comment at: lib/lsan/lsan_allocator.cc:85
@@ +84,3 @@
+class MaybeScopedLock {
+  BlockingMutex *m_;
+
----------------
Please keep the data members after the methods.

================
Comment at: lib/lsan/lsan_interceptors.cc:31
@@ +30,3 @@
+
+extern "C" int pthread_attr_init(void *attr);
+extern "C" int pthread_attr_destroy(void *attr);
----------------
extern "C" {
...
}

================
Comment at: lib/lsan/lsan_interceptors.cc:167
@@ +166,3 @@
+  pthread_attr_t myattr;
+  if (attr == 0) {
+    pthread_attr_init(&myattr);
----------------
Why do you need myattr at all?

================
Comment at: lib/lsan/lsan_interceptors.cc:188
@@ +187,3 @@
+      atomic_store(&p.tid, tid, memory_order_release);
+      while (atomic_load(&p.tid, memory_order_acquire) != 0)
+        pthread_yield();
----------------
Blocking in pthread_create until the pthread callback starts the execution sounds like an overkill. Do we need this?

================
Comment at: lib/lsan/lsan_interceptors.cc:229
@@ +228,3 @@
+// TODO(smatveev): find a home for this
+void Init() {
+  static bool lsan_inited;
----------------
How about making this a constructor? You won't need to call Init() from each function then.

================
Comment at: lib/lsan/lsan_thread.cc:145
@@ +144,3 @@
+  return (ThreadContext *)thread_registry->
+    GetThreadLocked(GetCurrentThread());
+}
----------------
Unless the two spaces have been suggested by clang-format, please use four-space indentation here.
I also think it's more readable to avoid line wrapping here.

================
Comment at: lib/lsan/lsan_thread.cc:211
@@ +210,3 @@
+  thread_registry->RunCallbackForEachThreadLocked(LockAllocatorsCallback,
+                                                       NULL);
+}
----------------
Please indent NULL properly and add a comment describing it. ("/*arg*/" ?)

================
Comment at: lib/lsan/lsan_thread.cc:86
@@ +85,3 @@
+  return thread_registry->CreateThread(user_id, detached,
+                                            parent_tid, NULL);
+}
----------------
Please fix the indentation.

================
Comment at: lib/lsan/lsan_thread.cc:25
@@ +24,3 @@
+
+extern "C" int arch_prctl(int code, __sanitizer::uptr *addr);
+
----------------
Isn't arch_prctl declared in asm/prctl.h?

================
Comment at: lib/lsan/lsan_thread.cc:47
@@ +46,3 @@
+  thread_registry = new(thread_registry_placeholder)
+    ThreadRegistry(CreateThreadContext, 1 << 13, 64);
+}
----------------
What are 1<<13 and 64?

================
Comment at: lib/lsan/lsan_stack.cc:47
@@ +46,3 @@
+  static char current_path[256];
+  static bool getcwd_attempted;
+
----------------
s/getcwd_attempted/current_path_initialized/

================
Comment at: lib/lsan/lsan_interceptors.cc:150
@@ +149,3 @@
+  void *param = p->param;
+  if (pthread_setspecific(g_thread_finalize_key, (void*)4)) {
+    Report("LeakSanitizer: failed to set thread key.\n");
----------------
Why exactly 4?

================
Comment at: lib/lsan/lsan_interceptors.cc:107
@@ +106,3 @@
+void *operator new(size_t size, std::nothrow_t const&) ALIAS("malloc")
+  SANITIZER_INTERFACE_ATTRIBUTE;
+void *operator new[](size_t size, std::nothrow_t const&) ALIAS("malloc")
----------------
Four spaces here and below, unless you're formatting everything with clang-format.

================
Comment at: lib/lsan/lsan_allocator.cc:42
@@ +41,3 @@
+
+void InitializeAllocator() {
+  static bool inited;
----------------
Why not initialize the allocator from Init() instead?


http://llvm-reviews.chandlerc.com/D702