[msan] Don't delete MSanAtExitRecord

[vitalybuka]

Summary

Pre 2.27 libc can run same atexit handler twice
We will keep MSanAtExitRecord and reset fun to mark it as executed.

Fix PR40162

[eugenis]

compiler-rt/test/msan/cxa_atexit_race.cc

24 ↗

(On Diff #186781)

I'm not sure about this .The other thread can register _a lot_ of handlers in one second.

25 ↗

(On Diff #186781)

return 0 and remove "not" before "%run" ?

[vitalybuka]

limit loop

[eugenis]

LGTM

compiler-rt/test/msan/cxa_atexit_race.cc

14 ↗

(On Diff #186785)

Please use an atomic!

[vitalybuka]

atomic

[peter.smith]

The test added here is intermittently hanging on our AArch64 buildbot.

This can either manifest itself as a the whole step timing out, or the test failing:

• http://lab.llvm.org:8011/builders/clang-cmake-aarch64-lld/builds/6060/steps/ninja%20check%202/logs/stdio (ninja check-all failing).
• http://lab.llvm.org:8011/builders/clang-cmake-aarch64-lld/builds/6057 (failing)

By running just the test continuously on an Ubuntu 16.04 machine I can get it to hang. The machine seems to need to be heavily loaded before this happens though. I've tried on another Ubuntu 16.04 machine that isn't loaded and I can't get it to hang. On the face of it, it looks like whatever fix worked for x86 hasn't worked for AArch64. Have you any suggestions on how to proceed? The obvious solution is to disable the test on AArch64, but this may just be hiding the problem?

[vitalybuka]

The test is remove D58396

lib/msan/msan_interceptors.cc

void MSanAtExitWrapper() {
   UnpoisonParam(1);
   ((void(*)())r->func)();
   InternalFree(r);
 }
 
 void MSanCxaAtExitWrapper(void *arg) {
   UnpoisonParam(1);
   MSanAtExitRecord *r = (MSanAtExitRecord *)arg;
+  // libc before 2.27 had race which caused occasional double handler execution
+  // https://sourceware.org/ml/libc-alpha/2017-08/msg01204.html
+  if (!r->func)
+    return;
   r->func(r->arg);
-  InternalFree(r);
+  r->func = nullptr;
 }
 
 static int setup_at_exit_wrapper(void(*f)(), void *arg, void *dso);
 
 // Unpoison argument shadow for C++ module destructors.
 INTERCEPTOR(int, __cxa_atexit, void (*func)(void *), void *arg,
             void *dso_handle) {
   if (msan_init_is_running) return REAL(__cxa_atexit)(func, arg, dso_handle);

test/msan/cxa_atexit_race.cc

+// RUN: %clangxx_msan %s -o %t && %run %t 2>&1 | FileCheck %s
+
+#include <atomic>
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+extern "C" int
+__cxa_atexit(void (*func)(void *), void *arg, void *d);
+
+void handler(void *) {
+}
+
+std::atomic_int counter;
+
+void *thread(void *) {
+  for (int i = 0; i < 10000; ++i) {
+    __cxa_atexit(&handler, 0, (void *)&handler);
+    ++counter;
+  }
+  return 0;
+}
+
+int main(void) {
+  printf("TEST_MAIN\n");
+  pthread_t pt;
+  for (int i = 0; i < 2; ++i)
+    pthread_create(&pt, 0, &thread, 0);
+  while (counter < 1000) {
+  };
+  return 0;
+}
+// CHECK: TEST_MAIN
+// CHECK-NOT: MemorySanitizer