[PATCH] Interceptors etc for standalone LSan.

Alexander Potapenko glider at google.com
Tue Apr 30 06:07:03 PDT 2013



================
Comment at: lib/lsan/lsan_allocator.cc:35
@@ +34,3 @@
+typedef CombinedAllocator<PrimaryAllocator, AllocatorCache,
+          SecondaryAllocator> Allocator;
+
----------------
The indentations here and above look random.

================
Comment at: lib/lsan/lsan_allocator.cc:160
@@ +159,3 @@
+template void ForEachChunk<ProcessSpecialAllocationsCb>(
+  ProcessSpecialAllocationsCb const &callback);
+template void ForEachChunk<PrintLeaksCb>(PrintLeaksCb const &callback);
----------------
Four spaces.

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

================
Comment at: lib/lsan/lsan_interceptors.cc:43
@@ +42,3 @@
+namespace std {
+  struct nothrow_t;
+}
----------------
No indentation here.

================
Comment at: lib/lsan/lsan_interceptors.cc:128
@@ +127,3 @@
+    SANITIZER_INTERFACE_ATTRIBUTE;
+extern "C" void *__libc_memalign(size_t alignment, size_t size)
+    ALIAS("memalign") SANITIZER_INTERFACE_ATTRIBUTE;
----------------
Please add a comment why you need to intercept a __libc_ function here.

================
Comment at: lib/lsan/lsan_thread.cc:170
@@ +169,3 @@
+  CHECK_NE(tid, kInvalidTid);
+  thread_registry->JoinThread(tid, 0 /*arg*/);
+}
----------------
/* arg */ 0

================
Comment at: lib/lsan/lsan_thread.cc:178
@@ +177,3 @@
+                           uptr *cache_begin, uptr *cache_end) {
+    ThreadContext *context = FindThreadByOsIDLocked(os_id);
+    if (!context)
----------------
2-space indentation here.

================
Comment at: lib/lsan/lsan_interceptors.cc:32
@@ +31,3 @@
+extern "C" {
+  int pthread_attr_init(void *attr);
+  int pthread_attr_destroy(void *attr);
----------------
No indentation within the "extern C" block

================
Comment at: lib/lsan/lsan_thread.cc:87
@@ +86,3 @@
+  return thread_registry->CreateThread(user_id, detached, parent_tid,
+                                       0 /*arg*/);
+}
----------------
/* arg */ 0

================
Comment at: lib/lsan/lsan_thread.cc:32
@@ +31,3 @@
+static ThreadRegistry *thread_registry;
+static THREADLOCAL u32 current_thread_tid = kInvalidTid;
+
----------------
Is THREADLOCAL available on all the platforms you're compiling this file on?

================
Comment at: lib/lsan/lsan_interceptors.cc:100
@@ +99,3 @@
+
+INTERCEPTOR(void, malloc_usable_size, void) {
+  // Not implemented.
----------------
I believe it's better to have these interceptors actually have the same prototypes as the intercepted functions do, otherwise you'll have problems if someone tries to call them.
If you want to forbid calling them, add UNIMPLEMENTED to their bodies.

================
Comment at: lib/lsan/lsan_interceptors.cc:160
@@ +159,3 @@
+  void *param = p->param;
+  if (pthread_setspecific(g_thread_finalize_key, (void*)4)) {
+    Report("LeakSanitizer: failed to set thread key.\n");
----------------
Please put a comment somewhere describing the magic 4 number.

================
Comment at: lib/lsan/lsan_interceptors.cc:127
@@ +126,3 @@
+extern "C" {
+  void cfree(void *p) ALIAS("free") SANITIZER_INTERFACE_ATTRIBUTE;
+  void *pvalloc(size_t size) ALIAS("valloc")
----------------
no indentation here

================
Comment at: lib/lsan/lsan_interceptors.cc:130
@@ +129,3 @@
+      SANITIZER_INTERFACE_ATTRIBUTE;
+  void *__libc_memalign(size_t alignment, size_t size)
+      ALIAS("memalign") SANITIZER_INTERFACE_ATTRIBUTE;
----------------
Please add a comment why are you intercepting a __libc_ function here, because you normally need not to.
I remember having asked about that already - are you keeping track of the comments that were/weren't fixed?


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



More information about the llvm-commits mailing list