[compiler-rt] [msan] Block signals during MsanThread::Destroy (PR #98405)

[Thurston Dang via llvm-commits]

https://github.com/thurstond updated https://github.com/llvm/llvm-project/pull/98405

>From 65df606f23710a6dcb8187c48bbaaa87baff9e9c Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Wed, 10 Jul 2024 22:53:23 +0000
Subject: [PATCH 1/6] [msan] Block signals during MsanThread::Destroy

MSan may segfault inside a signal handler, if MSan instrumentation is trying to access thread-local storage that has already
been destroyed. This fixes the issue by blocking asychronous signals
inside MsanThread::Destroy, as suggested by Paul Pluzhnikov.

Note: ed8565cf0b64ea5e88cc94f321b1870bb105d09d changed *BlockSignals to
only block asynchronous signals, despite the name.
---
 compiler-rt/lib/msan/msan_thread.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/compiler-rt/lib/msan/msan_thread.cpp b/compiler-rt/lib/msan/msan_thread.cpp
index ff9b90bb81f0c..75cdc42917bb4 100644
--- a/compiler-rt/lib/msan/msan_thread.cpp
+++ b/compiler-rt/lib/msan/msan_thread.cpp
@@ -3,6 +3,7 @@
 #include "msan_thread.h"
 #include "msan_interface_internal.h"
 
+#include "sanitizer_common/sanitizer_linux.h"
 #include "sanitizer_common/sanitizer_tls_get_addr.h"
 
 namespace __msan {
@@ -56,6 +57,7 @@ void MsanThread::TSDDtor(void *tsd) {
 }
 
 void MsanThread::Destroy() {
+  ScopedBlockSignals block(nullptr);
   malloc_storage().CommitBack();
   // We also clear the shadow on thread destruction because
   // some code may still be executing in later TSD destructors

>From b68c6d52a470391c8ad6f4f22f88dc1c5aecd1ec Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Wed, 10 Jul 2024 22:59:28 +0000
Subject: [PATCH 2/6] Scope change to Linux only

---
 compiler-rt/lib/msan/msan_thread.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/compiler-rt/lib/msan/msan_thread.cpp b/compiler-rt/lib/msan/msan_thread.cpp
index 75cdc42917bb4..cc4dfe601ead6 100644
--- a/compiler-rt/lib/msan/msan_thread.cpp
+++ b/compiler-rt/lib/msan/msan_thread.cpp
@@ -57,7 +57,9 @@ void MsanThread::TSDDtor(void *tsd) {
 }
 
 void MsanThread::Destroy() {
+#if SANITIZER_LINUX
   ScopedBlockSignals block(nullptr);
+#endif
   malloc_storage().CommitBack();
   // We also clear the shadow on thread destruction because
   // some code may still be executing in later TSD destructors

>From 1594a91dcd64482c6c4f05deae84175184629d4d Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Wed, 10 Jul 2024 23:35:11 +0000
Subject: [PATCH 3/6] clang-format

---
 compiler-rt/lib/msan/msan_thread.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/compiler-rt/lib/msan/msan_thread.cpp b/compiler-rt/lib/msan/msan_thread.cpp
index cc4dfe601ead6..e61c609471a5f 100644
--- a/compiler-rt/lib/msan/msan_thread.cpp
+++ b/compiler-rt/lib/msan/msan_thread.cpp
@@ -1,8 +1,8 @@
 
-#include "msan.h"
 #include "msan_thread.h"
-#include "msan_interface_internal.h"
 
+#include "msan.h"
+#include "msan_interface_internal.h"
 #include "sanitizer_common/sanitizer_linux.h"
 #include "sanitizer_common/sanitizer_tls_get_addr.h"
 

>From ad3327e41c5469806d78da6875519395b34a491d Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Thu, 11 Jul 2024 03:35:48 +0000
Subject: [PATCH 4/6] Move location of ScopedBlockSignals (and remove #if
 SANITIZER_LINUX), per Vitaly's feedback

---
 compiler-rt/lib/msan/msan_linux.cpp  | 1 +
 compiler-rt/lib/msan/msan_thread.cpp | 2 --
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/compiler-rt/lib/msan/msan_linux.cpp b/compiler-rt/lib/msan/msan_linux.cpp
index c68aec60ae13e..708a06d59a347 100644
--- a/compiler-rt/lib/msan/msan_linux.cpp
+++ b/compiler-rt/lib/msan/msan_linux.cpp
@@ -292,6 +292,7 @@ void MsanTSDDtor(void *tsd) {
     CHECK_EQ(0, pthread_setspecific(tsd_key, tsd));
     return;
   }
+  ScopedBlockSignals block(nullptr);
   msan_current_thread = nullptr;
   // Make sure that signal handler can not see a stale current thread pointer.
   atomic_signal_fence(memory_order_seq_cst);
diff --git a/compiler-rt/lib/msan/msan_thread.cpp b/compiler-rt/lib/msan/msan_thread.cpp
index e61c609471a5f..e6f99a7790606 100644
--- a/compiler-rt/lib/msan/msan_thread.cpp
+++ b/compiler-rt/lib/msan/msan_thread.cpp
@@ -57,9 +57,7 @@ void MsanThread::TSDDtor(void *tsd) {
 }
 
 void MsanThread::Destroy() {
-#if SANITIZER_LINUX
   ScopedBlockSignals block(nullptr);
-#endif
   malloc_storage().CommitBack();
   // We also clear the shadow on thread destruction because
   // some code may still be executing in later TSD destructors

>From 51ffac78c87ace74339a6b2d687d00e59c4b5c50 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Thu, 11 Jul 2024 03:37:28 +0000
Subject: [PATCH 5/6] Revert msan_thread.cpp changes

---
 compiler-rt/lib/msan/msan_thread.cpp | 2 --
 1 file changed, 2 deletions(-)

diff --git a/compiler-rt/lib/msan/msan_thread.cpp b/compiler-rt/lib/msan/msan_thread.cpp
index e6f99a7790606..280a7be2d8f43 100644
--- a/compiler-rt/lib/msan/msan_thread.cpp
+++ b/compiler-rt/lib/msan/msan_thread.cpp
@@ -3,7 +3,6 @@
 
 #include "msan.h"
 #include "msan_interface_internal.h"
-#include "sanitizer_common/sanitizer_linux.h"
 #include "sanitizer_common/sanitizer_tls_get_addr.h"
 
 namespace __msan {
@@ -57,7 +56,6 @@ void MsanThread::TSDDtor(void *tsd) {
 }
 
 void MsanThread::Destroy() {
-  ScopedBlockSignals block(nullptr);
   malloc_storage().CommitBack();
   // We also clear the shadow on thread destruction because
   // some code may still be executing in later TSD destructors

>From c53b1e04e5b14e5681b2b6c1605c910621051450 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Thu, 11 Jul 2024 03:38:45 +0000
Subject: [PATCH 6/6] Fully undo msan_thread changes

---
 compiler-rt/lib/msan/msan_thread.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/compiler-rt/lib/msan/msan_thread.cpp b/compiler-rt/lib/msan/msan_thread.cpp
index 280a7be2d8f43..ff9b90bb81f0c 100644
--- a/compiler-rt/lib/msan/msan_thread.cpp
+++ b/compiler-rt/lib/msan/msan_thread.cpp
@@ -1,8 +1,8 @@
 
-#include "msan_thread.h"
-
 #include "msan.h"
+#include "msan_thread.h"
 #include "msan_interface_internal.h"
+
 #include "sanitizer_common/sanitizer_tls_get_addr.h"
 
 namespace __msan {