Asan, fix FreeBSD support

[devnexen]

Summary

• the pthread_key_specific approach does not work well on FreeBSD, at AsanTSDInit it gets stuck.
• Instead storing the data in a thread local storage variable.

[devnexen]

Confirmed with this bug report https://bugs.llvm.org/show_bug.cgi?id=37238

[emaste]

Do we know why pthread_{get,set}specific fails now? And would it make sense to use tls more broadly (not just on FreeBSD)?

[krytarowski]

Please fix the hang in AsanTSDInit

[dim]

In D45652#1078277, @emaste wrote:

Do we know why pthread_{get,set}specific fails now? And would it make sense to use tls more broadly (not just on FreeBSD)?

I seem to remember that it sometimes (but not always) returns ENOSYS and fails. This is when libthr is not completely initialized yet, and it appears to depend on the order in which the DT_NEEDED entries are loaded. I know that I could never figure it out completely, that's why D39254 ("add -pthread to the flags for dynamic ASan tests") stalled. That was a workaround to make the tests succeed more often...

[devnexen]

In fact I ve noticed it stopped to be workable since around end of February/beginning March but things got in the way and I lost track but just to say it s not a so new issue. I m half surprised by @dim comment then. But in fact this PR it s more a way to trigger thoughts I knew it would not be accepted just like that ... Ideally I would go more with a common solution indeed.

[devnexen]

• Avoiding a bit of leakage, registering the destructor.

[krytarowski]

This is now needed on NetBSD too. Do we know what change caused this regression?

In the NetBSD case we are now configuring main thread on the first interceptor, that is premature to get pthread specific functions.

[krytarowski]

@vitalybuka do you agree with this patch?

[vitalybuka]

"context->destructor_iterations > 1" trick is needed to make sure that this destructor is called after other thread_local destructors
E.g. This descructor will remove thread form thread registry making it invisible to leak sanitizer causing false leak reports on thread_locals not yet destroyed.

[vitalybuka]

lib/asan/asan_posix.cc

67

it's not the same
atexit is called on process termination, we need thread termination

[krytarowski]

lib/asan/asan_posix.cc

67

There might be need to rework the code, and add support for calling a destructor in this fashion:

• intercept pthread_create()
• call an intermediate function (wrapper)
• within this wrapper call real function
• on exit of the wrapped function, call destructor
• add special case for the main thread

Alternatively follow TSan for NetBSD, extend it for FreeBSD. In TSan we are intercepting _lwp_exit() on NetBSD as the thread termination routine. FreeBSD shall have something similar.

There is also an option for: __cxa_thread_atexit but perhaps the worst one.

Some hybrid solution.. get/set specific for !main thread.

[vitalybuka]

lib/asan/asan_posix.cc

65

pthread_setspecific() works together with destructor_iterations.
pthreads will continue to call the destructor if we call pthread_setspecific from it up to GetPthreadDestructorIterations() times. And we hope that all other thread_locals are gone after few first iteration and we are the last when destructor_iterations is 0.

With TsdKey Tsd[4] pthread does not guaranty that it not going to destroy all 4 of them before the rest. btw it's not always 4.

On FreeBSD, is the bug is the problem of the main thread?
Could we just store tsd of the main thread in the static and other threads keep as is with pthread_getspecific?
Not sure, but also maybe we don't need to call TSDDtor of the main thread.

[krytarowski]

BTW. Does FreeBSD/i386 work? The NetBSD 32-bit code broke due to shadow granularity assert in:

void PoisonShadow(uptr addr, uptr size, u8 value) {
  if (!CanPoisonMemory()) return;
  CHECK(AddrIsAlignedByGranularity(addr)); // <- here, addr=0xxxxxxxx4
  CHECK(AddrIsInMem(addr));
  CHECK(AddrIsAlignedByGranularity(addr + size));
  CHECK(AddrIsInMem(addr + size - SHADOW_GRANULARITY));
  CHECK(REAL(memset));
  FastPoisonShadow(addr, size, value);
}

I don't see related changes in the code base.. debugging.

lib/asan/asan_posix.cc

65

On FreeBSD, is the bug is the problem of the main thread?
Could we just store tsd of the main thread in the static and other threads keep as is with pthread_getspecific?
Not sure, but also maybe we don't need to call TSDDtor of the main thread.

This is the case for NetBSD. Alternatively we could defer of setup of this main thread later.

In TSan we are starting extra g/c thread for the first pthread_create(3) interceptor.

[krytarowski]

BTW. Does FreeBSD/i386 work? The NetBSD 32-bit code broke due to shadow granularity assert in:

void PoisonShadow(uptr addr, uptr size, u8 value) {

if (!CanPoisonMemory()) return;
CHECK(AddrIsAlignedByGranularity(addr)); // <- here, addr=0xxxxxxxx4
CHECK(AddrIsInMem(addr));
CHECK(AddrIsAlignedByGranularity(addr + size));
CHECK(AddrIsInMem(addr + size - SHADOW_GRANULARITY));
CHECK(REAL(memset));
FastPoisonShadow(addr, size, value);

}

I've fixed this and I will submit a patch in a separate review.

[vitalybuka]

Nothing new, just to remove from phabricator dashboard.

[devnexen]

Will try to come up with newer solution later on if no one had fixed that meanwhile.

lib/asan/asan_posix.cc

 void AsanOnDeadlySignal(int signo, void *siginfo, void *context) {
   StartReportDeadlySignal();
   SignalContext sig(siginfo, context);
   ReportDeadlySignal(sig);
 }
 
 // ---------------------- TSD ---------------- {{{1
 
-static pthread_key_t tsd_key;
+struct TsdKey {
+  void *data;
+  void (*dst)(void *value);
+  TsdKey() : data(nullptr), dst(nullptr) {}
+  ~TsdKey() {
+    AsanThreadContext *context = (AsanThreadContext*)data;
+    if (context->destructor_iterations > 1) {
+      context->destructor_iterations--;
+      return;
+    }
+
+    if (dst)
+      dst(data);
+  }
+};
+
+static const size_t dit = GetPthreadDestructorIterations();
+static thread_local TsdKey Tsd[4];
 static bool tsd_key_inited = false;
 void AsanTSDInit(void (*destructor)(void *tsd)) {
   CHECK(!tsd_key_inited);
   tsd_key_inited = true;
-  CHECK_EQ(0, pthread_key_create(&tsd_key, destructor));
+  for (size_t i = 0; i < dit; i ++)
+    Tsd[i].dst = destructor;
 }
 
 void *AsanTSDGet() {
   CHECK(tsd_key_inited);
-  return pthread_getspecific(tsd_key);
+  return Tsd[0].data;
 }
 
 void AsanTSDSet(void *tsd) {
   CHECK(tsd_key_inited);
-  pthread_setspecific(tsd_key, tsd);
+  for (size_t i = 0; i < dit; i ++)
+    Tsd[i].data = tsd;
 }
 
 void PlatformTSDDtor(void *tsd) {
-  AsanThreadContext *context = (AsanThreadContext*)tsd;
-  if (context->destructor_iterations > 1) {
-    context->destructor_iterations--;
-    CHECK_EQ(0, pthread_setspecific(tsd_key, tsd));
-    return;
-  }
   AsanThread::TSDDtor(tsd);
 }
 }  // namespace __asan
 
 #endif  // SANITIZER_POSIX