[compiler-rt] [tsan] Fix ASLR edge case, and improve diagnostics (PR #97125)

[Thurston Dang via llvm-commits]

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

>From 4d54427724583aded98ec69bf928dd453c2c63f9 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Fri, 28 Jun 2024 23:46:54 +0000
Subject: [PATCH 1/2] [tsan] Fix ASLR edge case, and improve diagnostics

In extremely rare cases (estimated 1 in 3 million), minor allocations that happen after
InitializePlatformEarly() - when the memory layout is checked for
compatibility - may result in the memory layout unexpectedly being incompatible in InitializePlatform().
We fix this by adding another memory layout check (and opportunity to
re-exec without ASLR) in InitializePlatform().

To improve future debuggability, this patch also dumps the process map
if the memory layout is unexpectedly incompatible.
---
 .../lib/tsan/rtl/tsan_platform_linux.cpp      | 27 ++++++++++++++-----
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
index c723dba556ed2..6c138ba5ee882 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
@@ -215,7 +215,7 @@ void InitializeShadowMemoryPlatform() {
 #endif  // #if !SANITIZER_GO
 
 #  if !SANITIZER_GO
-static void ReExecIfNeeded() {
+static void ReExecIfNeeded(bool ignore_heap) {
   // Go maps shadow memory lazily and works fine with limited address space.
   // Unlimited stack is not a problem as well, because the executable
   // is not compiled with -pie.
@@ -266,7 +266,7 @@ static void ReExecIfNeeded() {
 
   if (reexec) {
     // Don't check the address space since we're going to re-exec anyway.
-  } else if (!CheckAndProtect(false, false, false)) {
+  } else if (!CheckAndProtect(false, ignore_heap, false)) {
     // ASLR personality check.
     // N.B. 'personality' is sometimes forbidden by sandboxes, so we only call
     // this as a last resort (when the memory mapping is incompatible and TSan
@@ -290,10 +290,10 @@ static void ReExecIfNeeded() {
       CHECK_NE(personality(old_personality | ADDR_NO_RANDOMIZE), -1);
       reexec = true;
     } else {
-      VReport(1,
-              "FATAL: ThreadSanitizer: memory layout is incompatible, "
-              "even though ASLR is disabled.\n"
-              "Please file a bug.\n");
+      Printf("FATAL: ThreadSanitizer: memory layout is incompatible, "
+             "even though ASLR is disabled.\n"
+             "Please file a bug.\n");
+      DumpProcessMap();
       Die();
     }
   }
@@ -376,7 +376,8 @@ void InitializePlatformEarly() {
 #  endif
 
 #  if !SANITIZER_GO
-  ReExecIfNeeded();
+  // Heap has not been allocated yet
+  ReExecIfNeeded(false);
 #  endif
 }
 
@@ -394,6 +395,17 @@ void InitializePlatform() {
 #    endif
   }
 
+  // We called ReExecIfNeeded() in InitializePlatformEarly(), but there are
+  // intervening allocations that result in an edge case:
+  // 1) InitializePlatformEarly(): memory layout is compatible
+  // 2) Intervening allocations happen
+  // 3) InitializePlatform(): memory layout is incompatible and fails
+  //    CheckAndProtect()
+#  if !SANITIZER_GO
+  // Heap has already been allocated
+  ReExecIfNeeded(true);
+#  endif
+
   // Earlier initialization steps already re-exec'ed until we got a compatible
   // memory layout, so we don't expect any more issues here.
   if (!CheckAndProtect(true, true, true)) {
@@ -401,6 +413,7 @@ void InitializePlatform() {
         "FATAL: ThreadSanitizer: unexpectedly found incompatible memory "
         "layout.\n");
     Printf("FATAL: Please file a bug.\n");
+    DumpProcessMap();
     Die();
   }
 

>From dee27439eb93d470c85352a811bf13f6ace24b50 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 29 Jun 2024 00:07:47 +0000
Subject: [PATCH 2/2] Formatting

---
 compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
index 6c138ba5ee882..621c679a05db4 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
@@ -290,9 +290,10 @@ static void ReExecIfNeeded(bool ignore_heap) {
       CHECK_NE(personality(old_personality | ADDR_NO_RANDOMIZE), -1);
       reexec = true;
     } else {
-      Printf("FATAL: ThreadSanitizer: memory layout is incompatible, "
-             "even though ASLR is disabled.\n"
-             "Please file a bug.\n");
+      Printf(
+          "FATAL: ThreadSanitizer: memory layout is incompatible, "
+          "even though ASLR is disabled.\n"
+          "Please file a bug.\n");
       DumpProcessMap();
       Die();
     }
@@ -401,10 +402,10 @@ void InitializePlatform() {
   // 2) Intervening allocations happen
   // 3) InitializePlatform(): memory layout is incompatible and fails
   //    CheckAndProtect()
-#  if !SANITIZER_GO
+#    if !SANITIZER_GO
   // Heap has already been allocated
   ReExecIfNeeded(true);
-#  endif
+#    endif
 
   // Earlier initialization steps already re-exec'ed until we got a compatible
   // memory layout, so we don't expect any more issues here.