[sanitizer] Extract SANITIZER_FREEBSD version of ThreadDescriptorSizeFallback

[vitalybuka]

This should fix SANITIZER_FREEBSD and simplify
SANITIZER_GLIBC version.

Also the PR make readers aware of problematic
ThreadDescriptorSizeFallback for SANITIZER_FREEBSD.
Maybe it will encourage FreeBSD maintainers to
improve the functions, or prove that it's not needed at
all.

[llvmbot]

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

Author: Vitaly Buka (vitalybuka)

Changes

This should fix SANITIZER_FREEBSD and simplify
SANITIZER_GLIBC version.

---

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

1 Files Affected:

• (modified) compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp (+43-12)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
index 4fc99197aae3d5..6b83bbb9a9e94e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
@@ -219,25 +219,14 @@ static void GetGLibcVersion(int *major, int *minor, int *patch) {
 }
 #  endif  // SANITIZER_GLIBC && !SANITIZER_GO
 
-// On glibc x86_64, ThreadDescriptorSize() needs to be precise due to the usage
-// of g_tls_size. On other targets, ThreadDescriptorSize() is only used by lsan
-// to get the pointer to thread-specific data keys in the thread control block.
-#  if (SANITIZER_FREEBSD || SANITIZER_GLIBC) && !SANITIZER_GO
-// sizeof(struct pthread) from glibc.
-static uptr thread_descriptor_size;
-
-// FIXME: Implementation is very GLIBC specific, but it's used by FreeBSD.
+#  if SANITIZER_GLIBC && !SANITIZER_GO
 static uptr ThreadDescriptorSizeFallback() {
 #    if defined(__x86_64__) || defined(__i386__) || defined(__arm__) || \
         SANITIZER_RISCV64
-#      if SANITIZER_GLIBC
   int major;
   int minor;
   int patch;
   GetGLibcVersion(&major, &minor, &patch);
-#      else   // SANITIZER_GLIBC
-  return 0;
-#      endif  // SANITIZER_GLIBC
 #    endif
 
 #    if defined(__x86_64__) || defined(__i386__) || defined(__arm__)
@@ -304,6 +293,48 @@ static uptr ThreadDescriptorSizeFallback() {
   return 1776;  // from glibc.ppc64le 2.20-8.fc21
 #    endif
 }
+#  endif  // SANITIZER_GLIBC && !SANITIZER_GO
+
+#  if SANITIZER_FREEBSD && !SANITIZER_GO
+// FIXME: Implementation is very GLIBC specific, but it's used by FreeBSD.
+static uptr ThreadDescriptorSizeFallback() {
+#    if defined(__s390__) || defined(__sparc__)
+  // The size of a prefix of TCB including pthread::{specific_1stblock,specific}
+  // suffices. Just return offsetof(struct pthread, specific_used), which hasn't
+  // changed since 2007-05. Technically this applies to i386/x86_64 as well but
+  // we call _dl_get_tls_static_info and need the precise size of struct
+  // pthread.
+  return FIRST_32_SECOND_64(524, 1552);
+#    endif
+
+#    if defined(__mips__)
+  // TODO(sagarthakur): add more values as per different glibc versions.
+  return FIRST_32_SECOND_64(1152, 1776);
+#    endif
+
+#    if SANITIZER_LOONGARCH64
+  return 1856;  // from glibc 2.36
+#    endif
+
+#    if defined(__aarch64__)
+  // The sizeof (struct pthread) is the same from GLIBC 2.17 to 2.22.
+  return 1776;
+#    endif
+
+#    if defined(__powerpc64__)
+  return 1776;  // from glibc.ppc64le 2.20-8.fc21
+#    endif
+
+  return 0;
+}
+#  endif  // SANITIZER_FREEBSD && !SANITIZER_GO
+
+#  if (SANITIZER_FREEBSD || SANITIZER_GLIBC) && !SANITIZER_GO
+// On glibc x86_64, ThreadDescriptorSize() needs to be precise due to the usage
+// of g_tls_size. On other targets, ThreadDescriptorSize() is only used by lsan
+// to get the pointer to thread-specific data keys in the thread control block.
+// sizeof(struct pthread) from glibc.
+static uptr thread_descriptor_size;
 
 uptr ThreadDescriptorSize() { return thread_descriptor_size; }
 

[vitalybuka]

ThreadDescriptorSize() Is used in a few FREEBSD branches, but it looks very wrong.

[rorth]

I have a hard time believing that that's any point in this: apart from being glibc-specific, as of FreeBSD 14.1 there isn't a s390, sparc, mips, or loongarch64 port of FreeBSD.

[vitalybuka]

If you are confident, I can drop them in followup PR, so bisecting could be easiers.

According to https://www.freebsd.org/platforms/
Recent Tier 1 are only x86_64 and aarch64, the former works with 0.

I guess implication of having these values invalid is incorrect DTLS detection which will false positive for msan and lsan.

[rorth]

FWIW, this does indeed fix the build failure on FreeBSD 14.0, without affecting Solaris. Unfortunately, the build fails later for different reasons:

FAILED: projects/compiler-rt/lib/builtins/CMakeFiles/clang_rt.builtins-i386.dir/extendhfsf2.c.o
[...]
In file included from compiler-rt/lib/builtins/extendhfsf2.c:11:
In file included from compiler-rt/lib/builtins/fp_extend_impl.inc:38:
compiler-rt/lib/builtins/fp_extend.h:57:9: error
: _Float16 is not supported on this target
   57 | typedef _Float16 src_t;
      |         ^
1 error generated.

While _Float16 is supported for 64-bit x86, it isn't for 32-bit (since that doesn't support SSE2 for whatever reason). However, the builtins CMakeFile.list only checks for _Float16 support for the default multilib, missing this issue.

[vitalybuka]

FWIW, this does indeed fix the build failure on FreeBSD 14.0, without affecting Solaris. Unfortunately, the build fails later for different reasons:

FAILED: projects/compiler-rt/lib/builtins/CMakeFiles/clang_rt.builtins-i386.dir/extendhfsf2.c.o
[...]
In file included from compiler-rt/lib/builtins/extendhfsf2.c:11:
In file included from compiler-rt/lib/builtins/fp_extend_impl.inc:38:
compiler-rt/lib/builtins/fp_extend.h:57:9: error
: _Float16 is not supported on this target
   57 | typedef _Float16 src_t;
      |         ^
1 error generated.

While _Float16 is supported for 64-bit x86, it isn't for 32-bit (since that doesn't support SSE2 for whatever reason). However, the builtins CMakeFile.list only checks for _Float16 support for the default multilib, missing this issue.

_Float16 is unrelated to the PR issue?

[fmayer]

Change mechanically LGTM, but I can't comment on the FreeBSD