[HWSAN] Use ThreadArgRetval in HWSAN

[vitalybuka]

Summary

Fixes false leaks on thread arg, retval.

[Enna1]

compiler-rt/lib/hwasan/hwasan_interceptors.cpp

91

Not related to this patch.
Just wondering why we intercept pthread_join first argument as void *, the actual type is pthread_t, is there any context about this?

[vitalybuka]

compiler-rt/lib/hwasan/hwasan_interceptors.cpp

91

typedef unsigned long int pthread_t; which is size of pointer on posix compilers
internally, for glibc, pthread_t is just a pointer to internal data structure.

we cannot include pthread.h here, but it would be cleaner to have some our typedef for that.

[Enna1]

compiler-rt/lib/hwasan/hwasan_interceptors.cpp

91

Got it, thanks!

[vitalybuka]

update

compiler-rt/lib/hwasan/hwasan_interceptors.cpp

 //
 // FIXME: move as many interceptors as possible into
 // sanitizer_common/sanitizer_common_interceptors.h
 //===----------------------------------------------------------------------===//
 
 #include "hwasan.h"
 #include "hwasan_checks.h"
 #include "hwasan_thread.h"
+#include "hwasan_thread_list.h"
 #include "interception/interception.h"
 #include "sanitizer_common/sanitizer_linux.h"
 #include "sanitizer_common/sanitizer_stackdepot.h"
 
 #if !SANITIZER_FUCHSIA
 
 using namespace __hwasan;
 
#    define COMMON_SYSCALL_POST_WRITE_RANGE(p, s) \
       do {                                        \
         (void)(p);                                \
         (void)(s);                                \
       } while (false)
 #    include "sanitizer_common/sanitizer_common_syscalls.inc"
 #    include "sanitizer_common/sanitizer_syscalls_netbsd.inc"
 
 struct ThreadStartArg {
-  thread_callback_t callback;
-  void *param;
   __sanitizer_sigset_t starting_sigset_;
 };
 
 static void *HwasanThreadStartFunc(void *arg) {
   __hwasan_thread_enter();
   ThreadStartArg A = *reinterpret_cast<ThreadStartArg *>(arg);
   SetSigProcMask(&A.starting_sigset_, nullptr);
   InternalFree(arg);
-  return A.callback(A.param);
+  auto self = GetThreadSelf();
+  auto args = hwasanThreadArgRetval().GetArgs(self);
+  void *retval = (*args.routine)(args.arg_retval);
+  hwasanThreadArgRetval().Finish(self, retval);
+  return retval;
 }
 
-INTERCEPTOR(int, pthread_create, void *th, void *attr,
+extern "C" {
+int pthread_attr_getdetachstate(void *attr, int *v);
+}
+
+INTERCEPTOR(int, pthread_create, void *thread, void *attr,
             void *(*callback)(void *), void *param) {
   EnsureMainThreadIDIsCorrect();
   ScopedTaggingDisabler tagging_disabler;
+  int detached = 0;
+  if (attr)
+    pthread_attr_getdetachstate(attr, &detached);
   ThreadStartArg *A = (ThreadStartArg *)InternalAlloc(sizeof(ThreadStartArg));
-  A->callback = callback;
-  A->param = param;
   ScopedBlockSignals block(&A->starting_sigset_);
   // ASAN uses the same approach to disable leaks from pthread_create.
 #    if CAN_SANITIZE_LEAKS
   __lsan::ScopedInterceptorDisabler lsan_disabler;
 #    endif
-  int result = REAL(pthread_create)(th, attr, &HwasanThreadStartFunc, A);
+
+  int result;
+  hwasanThreadArgRetval().Create(detached, {callback, param}, [&]() -> uptr {
+    result = REAL(pthread_create)(thread, attr, &HwasanThreadStartFunc, A);
+    return result ? 0 : *(uptr *)(thread);
+  });
   if (result != 0)
     InternalFree(A);
   return result;
 }
 
-INTERCEPTOR(int, pthread_join, void *t, void **arg) {
-  return REAL(pthread_join)(t, arg);
+INTERCEPTOR(int, pthread_join, void *thread, void **retval) {
+  int result;
+  hwasanThreadArgRetval().Join((uptr)thread, [&]() {
+    result = REAL(pthread_join)(thread, retval);
+    return !result;
+  });
+  return result;
 }
 
 INTERCEPTOR(int, pthread_detach, void *thread) {
-  return REAL(pthread_detach)(thread);
+  int result;
+  hwasanThreadArgRetval().Detach((uptr)thread, [&]() {
+    result = REAL(pthread_detach)(thread);
+    return !result;
+  });
+  return result;
 }
 
 INTERCEPTOR(int, pthread_exit, void *retval) {
+  hwasanThreadArgRetval().Finish(GetThreadSelf(), retval);
   return REAL(pthread_exit)(retval);
 }
 
 #    if SANITIZER_GLIBC
 INTERCEPTOR(int, pthread_tryjoin_np, void *thread, void **ret) {
-  return REAL(pthread_tryjoin_np)(thread, ret);
+  int result;
+  hwasanThreadArgRetval().Join((uptr)thread, [&]() {
+    result = REAL(pthread_tryjoin_np)(thread, ret);
+    return !result;
+  });
+  return result;
 }
 
 INTERCEPTOR(int, pthread_timedjoin_np, void *thread, void **ret,
             const struct timespec *abstime) {
-  return REAL(pthread_timedjoin_np)(thread, ret, abstime);
+  int result;
+  hwasanThreadArgRetval().Join((uptr)thread, [&]() {
+    result = REAL(pthread_timedjoin_np)(thread, ret, abstime);
+    return !result;
+  });
+  return result;
 }
 #    endif
 
 DEFINE_REAL_PTHREAD_FUNCTIONS
 
 DEFINE_REAL(int, vfork)
 DECLARE_EXTERN_INTERCEPTOR_AND_WRAPPER(int, vfork)
 

compiler-rt/lib/hwasan/hwasan_thread.cpp

static __hwasan::HwasanThreadList *GetHwasanThreadListLocked() {
   return &tl;
 }
 
 static __hwasan::Thread *GetThreadByOsIDLocked(tid_t os_id) {
   return GetHwasanThreadListLocked()->FindThreadLocked(
       [os_id](__hwasan::Thread *t) { return t->os_id() == os_id; });
 }
 
-void LockThreadRegistry() { __hwasan::hwasanThreadList().Lock(); }
+void LockThreadRegistry() {
+  __hwasan::hwasanThreadList().Lock();
+  __hwasan::hwasanThreadArgRetval().Lock();
+}
 
-void UnlockThreadRegistry() { __hwasan::hwasanThreadList().Unlock(); }
+void UnlockThreadRegistry() {
+  __hwasan::hwasanThreadArgRetval().Unlock();
+  __hwasan::hwasanThreadList().Unlock();
+}
 
 void EnsureMainThreadIDIsCorrect() { __hwasan::EnsureMainThreadIDIsCorrect(); }
 
 bool GetThreadRangesLocked(tid_t os_id, uptr *stack_begin, uptr *stack_end,
                            uptr *tls_begin, uptr *tls_end, uptr *cache_begin,
                            uptr *cache_end, DTLS **dtls) {
   auto *t = GetThreadByOsIDLocked(os_id);
   if (!t)

 }
 
 void GetAllThreadAllocatorCachesLocked(InternalMmapVector<uptr> *caches) {}
 
 void GetThreadExtraStackRangesLocked(tid_t os_id,
                                      InternalMmapVector<Range> *ranges) {}
 void GetThreadExtraStackRangesLocked(InternalMmapVector<Range> *ranges) {}
 
-void GetAdditionalThreadContextPtrsLocked(InternalMmapVector<uptr> *ptrs) {}
+void GetAdditionalThreadContextPtrsLocked(InternalMmapVector<uptr> *ptrs) {
+  __hwasan::hwasanThreadArgRetval().GetAllPtrsLocked(ptrs);
+}
+
 void GetRunningThreadsLocked(InternalMmapVector<tid_t> *threads) {}
 
 }  // namespace __lsan

compiler-rt/lib/hwasan/hwasan_thread_list.h

 //   (A | ((1 << K) - 1)) + 1
 // It is only correct if A is not already equal to the shadow base address, but
 // it saves 2 instructions on AArch64.
 
 #include "hwasan.h"
 #include "hwasan_allocator.h"
 #include "hwasan_flags.h"
 #include "hwasan_thread.h"
-
 #include "sanitizer_common/sanitizer_placement_new.h"
+#include "sanitizer_common/sanitizer_thread_arg_retval.h"
 
 namespace __hwasan {
 
 static uptr RingBufferSize() {
   uptr desired_bytes = flags()->stack_history_size * sizeof(uptr);
   // FIXME: increase the limit to 8 once this bug is fixed:
   // https://bugs.llvm.org/show_bug.cgi?id=39030
   for (int shift = 1; shift < 7; ++shift) {
InternalMmapVector<Thread *> live_list_
       SANITIZER_GUARDED_BY(live_list_mutex_);
 
   SpinMutex stats_mutex_;
   ThreadStats stats_ SANITIZER_GUARDED_BY(stats_mutex_);
 };
 
 void InitThreadList(uptr storage, uptr size);
 HwasanThreadList &hwasanThreadList();
+ThreadArgRetval &hwasanThreadArgRetval();
 
 } // namespace __hwasan

compiler-rt/lib/hwasan/hwasan_thread_list.cpp

 #include "hwasan_thread_list.h"
 
+#include "sanitizer_common/sanitizer_thread_arg_retval.h"
+
 namespace __hwasan {
 
 static HwasanThreadList *hwasan_thread_list;
+static ThreadArgRetval *thread_data;
 
 HwasanThreadList &hwasanThreadList() { return *hwasan_thread_list; }
+ThreadArgRetval &hwasanThreadArgRetval() { return *thread_data; }
 
 void InitThreadList(uptr storage, uptr size) {
   CHECK_EQ(hwasan_thread_list, nullptr);
 
   static ALIGNED(alignof(
       HwasanThreadList)) char thread_list_placeholder[sizeof(HwasanThreadList)];
   hwasan_thread_list =
       new (thread_list_placeholder) HwasanThreadList(storage, size);
+
+  CHECK_EQ(thread_data, nullptr);
+
+  static ALIGNED(alignof(
+      ThreadArgRetval)) char thread_data_placeholder[sizeof(ThreadArgRetval)];
+  thread_data = new (thread_data_placeholder) ThreadArgRetval();
 }
 
-} // namespace __hwasan
+}  // namespace __hwasan

compiler-rt/test/lsan/TestCases/create_thread_leak.cpp

 // Test handling of arg and retval of child thread.
 
 // RUN: %clangxx_lsan -pthread %s -o %t
 // RUN: %run not %t 10 1 0 0 2>&1 | FileCheck %s --check-prefixes=LEAK,LEAK123
 // RUN: %run not %t 10 0 1 0 2>&1 | FileCheck %s --check-prefixes=LEAK,LEAK234
 // RUN: %run not %t 10 0 0 1 2>&1 | FileCheck %s --check-prefixes=LEAK,LEAK234
-
-// FIXME: Remove "not". There is no leak.
-// False LEAK123 is broken for HWASAN.
-// False LEAK234 is broken for HWASAN.
-// RUN: %run %if hwasan %{ not %} %t 10 0 0 0
+// RUN: %run %t 10 0 0 0
 
 #include <pthread.h>
 #include <stdlib.h>
 
 #include <sanitizer/lsan_interface.h>
 
 int detach;
 int leak_arg;