Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tsan] Fix build for FreeBSD and NetBSD after 0784b1eefa36 #79019

Merged
merged 1 commit into from
Jan 22, 2024

Conversation

DimitryAndric
Copy link
Collaborator

In 0784b1e some code for re-execution was moved to ReExecIfNeeded(), but also extended with a few Linux-only features. This leads to compile errors on FreeBSD, or other non-Linux platforms:

compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp:247:25: error: use of undeclared identifier 'personality'
  247 |   int old_personality = personality(0xffffffff);
      |                         ^
compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp:249:54: error: use of undeclared identifier 'ADDR_NO_RANDOMIZE'
  249 |       (old_personality != -1) && ((old_personality & ADDR_NO_RANDOMIZE) == 0);
      |                                                      ^
compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp:281:46: error: use of undeclared identifier 'ADDR_NO_RANDOMIZE'
  281 |       CHECK_NE(personality(old_personality | ADDR_NO_RANDOMIZE), -1);
      |                                              ^

Surround the affected part with a #if SANITIZER_LINUX block for now.

In 0784b1e some code for re-execution was moved to
`ReExecIfNeeded()`, but also extended with a few Linux-only features.
This leads to compile errors on FreeBSD, or other non-Linux platforms:

    compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp:247:25: error: use of undeclared identifier 'personality'
      247 |   int old_personality = personality(0xffffffff);
          |                         ^
    compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp:249:54: error: use of undeclared identifier 'ADDR_NO_RANDOMIZE'
      249 |       (old_personality != -1) && ((old_personality & ADDR_NO_RANDOMIZE) == 0);
          |                                                      ^
    compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp:281:46: error: use of undeclared identifier 'ADDR_NO_RANDOMIZE'
      281 |       CHECK_NE(personality(old_personality | ADDR_NO_RANDOMIZE), -1);
          |                                              ^

Surround the affected part with a `#if SANITIZER_LINUX` block for now.
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 22, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Dimitry Andric (DimitryAndric)

Changes

In 0784b1e some code for re-execution was moved to ReExecIfNeeded(), but also extended with a few Linux-only features. This leads to compile errors on FreeBSD, or other non-Linux platforms:

compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp:247:25: error: use of undeclared identifier 'personality'
  247 |   int old_personality = personality(0xffffffff);
      |                         ^
compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp:249:54: error: use of undeclared identifier 'ADDR_NO_RANDOMIZE'
  249 |       (old_personality != -1) && ((old_personality & ADDR_NO_RANDOMIZE) == 0);
      |                                                      ^
compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp:281:46: error: use of undeclared identifier 'ADDR_NO_RANDOMIZE'
  281 |       CHECK_NE(personality(old_personality | ADDR_NO_RANDOMIZE), -1);
      |                                              ^

Surround the affected part with a #if SANITIZER_LINUX block for now.


Full diff: https://github.com/llvm/llvm-project/pull/79019.diff

1 Files Affected:

  • (modified) compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp (+4-2)
diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
index 9a66a7feb5b395..0d0b1aba1f852a 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
@@ -243,12 +243,13 @@ static void ReExecIfNeeded() {
     reexec = true;
   }
 
+#    if SANITIZER_LINUX
   // ASLR personality check.
   int old_personality = personality(0xffffffff);
   bool aslr_on =
       (old_personality != -1) && ((old_personality & ADDR_NO_RANDOMIZE) == 0);
 
-#    if SANITIZER_ANDROID && (defined(__aarch64__) || defined(__x86_64__))
+#      if SANITIZER_ANDROID && (defined(__aarch64__) || defined(__x86_64__))
   // After patch "arm64: mm: support ARCH_MMAP_RND_BITS." is introduced in
   // linux kernel, the random gap between stack and mapped area is increased
   // from 128M to 36G on 39-bit aarch64. As it is almost impossible to cover
@@ -261,7 +262,7 @@ static void ReExecIfNeeded() {
     CHECK_NE(personality(old_personality | ADDR_NO_RANDOMIZE), -1);
     reexec = true;
   }
-#    endif
+#      endif
 
   if (reexec) {
     // Don't check the address space since we're going to re-exec anyway.
@@ -288,6 +289,7 @@ static void ReExecIfNeeded() {
       Die();
     }
   }
+#    endif  // SANITIZER_LINUX
 
   if (reexec)
     ReExec();

@DimitryAndric
Copy link
Collaborator Author

Now that I look at this again, I notice that the shared CheckASLR() (https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp#L2261) is already called before __tsan::InitializePlatformEarly():

https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp#L695

So why is there any need to check for ASLR again in ReExecIfNeeded()? It would already have been re-exec'd at this point?

@thurstond
Copy link
Contributor

Thanks for the patch! Sorry for the breakage.

Now that I look at this again, I notice that the shared CheckASLR() (https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp#L2261) is already called before __tsan::InitializePlatformEarly():

https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp#L695

So why is there any need to check for ASLR again in ReExecIfNeeded()? It would already have been re-exec'd at this point?

CheckASLR() unconditionally disables ASLR (or instructs the user to that effect) for some platforms, such as NetBSD and FreeBSD. It's a very simple check that does not need to inspect the memory mappings for compatibility, because it will always disable ASLR for those platforms.

TSan on Linux, however, does support some degree of ASLR (e.g., the default of 28-bits of entropy). ReExecIfNeeded() checks if the address space layout is compatible with TSan; if not, then it disables ASLR. But in the common case, it will not disable ASLR.

@DimitryAndric DimitryAndric merged commit 042bb28 into main Jan 22, 2024
6 checks passed
@DimitryAndric DimitryAndric deleted the users/DimitryAndric/tsan-fix-freebsd branch January 22, 2024 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants