Skip to content

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
Member

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
@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