[CompilerRT] Fix build of compiler-rt with musl

[ccross]

Summary

Use the correct types for OFF_T, sanitizer_time_t and
sanitizer_dirent and forward time_t related functions
to fix using compiler-rt with 32-bit musl libc.

Also redirect the time_t functions that are affected by
https://musl.libc.org/time64.html to use their 64-bit
ABI names.

[MaskRay]

Thanks for making musl better:) I have thought about whether SANITIZER_MUSL is better or SANITIZER_LINUX && !SANITIZER_GLIBC (maybe include !SANITIZER_ANDROID) is better.
I favor SANITIZER_LINUX && !SANITIZER_GLIBC since any newly developed libc will likely go with 64-bit off_t and time_t for 32-bit platforms to not be bogged into the time64 problems...
With this view glibc is the odd one.

[MaskRay]

Revoke LGTM. musl before 1.2 used 32-bit time_t (https://musl.libc.org/time64.html). I do not know how 32-bit musl systems currently support sanitizers. Some distro folks can chime in @q66 @thesamesam)

arnd has a comment whether we could make OFF_T (and possibly other common types) a configure-time choice.

For musl>=1.2 on 32-bit systems, the symbols have time64 suffixes (e.g. clock_gettime to __clock_gettime64). Both systems exist, so it seems that we need new interceptors for time64 symbols.

The way the current patch is written as, it will punish the existing musl<1.2 users (if they somehow work) while not addressing time64 systems for musl>=1.2...

[ccross]

In D119358#3310112, @MaskRay wrote:

Revoke LGTM. musl before 1.2 used 32-bit time_t (https://musl.libc.org/time64.html). I do not know how 32-bit musl systems currently support sanitizers. Some distro folks can chime in @q66 @thesamesam)

arnd has a comment whether we could make OFF_T (and possibly other common types) a configure-time choice.

For musl>=1.2 on 32-bit systems, the symbols have time64 suffixes (e.g. clock_gettime to __clock_gettime64). Both systems exist, so it seems that we need new interceptors for time64 symbols.

The way the current patch is written as, it will punish the existing musl<1.2 users (if they somehow work) while not addressing time64 systems for musl>=1.2...

64-bit musl has a clock_gettime symbol and no __clock_gettime64 so this should work fine for 64-bit musl. This code currently won't compile for any version of 32-bit musl due to off_t always having been defined as a 64-bit type so I don't think this actually makes anything worse.

Fully supporting sanitizing with 32-bit musl will need to handle clock_gettime with a 32-bit time_t and __clock_gettime64 with a 64-bit time_t, and similarly for time, mktime, gmtime, localtime, ctime, gmtime_r, localtime_r, ctime_r, nanosleep, clock_getres, clock_gettime, clock_settime, clock_nanosleep, timer_gettime, timer_settime and stime. Alternatively, sanitizing could intercept the newer __clock_gettime64 style symbols for 32-bit musl, which would catch anything built against a musl version from less than 2 years ago.

Either solution would be specific to musl and would need to be wrapped in #if defined(SANITIZER_MUSL) && (defined(__i386__) || defined(__arm__) ...).

[MaskRay]

I am convinced that 32-bit just doesn't work for all musl versions. This patch should be quite to good to go if you demonstrate that some time64 symbols are property intercepted, using the #define approach like #if SANITIZER_NETBSD #define clock_gettime __clock_gettime50. We do not need to care about musl<1.2. That would greatly add complexity to sanitizers.

Either solution would be specific to musl and would need to be wrapped in #if defined(SANITIZER_MUSL) && (defined(i386) || defined(arm) ...).

Perhaps check __SIZEOF_POINTER__ or _ILP32

[MaskRay]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

135 ↗

(On Diff #412890)

Unlike others (we assume another libc will behave more like musl instead of glibc/Android Bionic), this block is indeed musl specific and introducing SANITIZER_MUSL may make more sense.

135 ↗

(On Diff #412890)

defined(_ILP32) may be cleaner than enumerating the 32-bit architectures

[ccross]

Add SANITIZER_MUSL to determine when to use the time32 compatibility redirections.

[ccross]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

135 ↗

(On Diff #412890)

I enumerated them because it is not all 32-bit architectures, it is only the ones that musl supported pre-1.2.0. Any 32-bit architectures newly added to musl would not set REDIR_TIME64 because they wouldn't need any backwards compatibility ABI.

[MaskRay]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

135 ↗

(On Diff #412890)

OK, I see.

You need to define SANITIZER_MUSL in compiler-rt/lib/sanitizer_common/sanitizer_platform.h

[MaskRay]

fix using compiler-rt with 32-bit musl libc.

This sentence in the summary needs to be clarified for the list of 32-bit architectures using #define _REDIR_TIME64 1.

[ccross]

In D119358#3370467, @MaskRay wrote:

fix using compiler-rt with 32-bit musl libc.

This sentence in the summary needs to be clarified for the list of 32-bit architectures using #define _REDIR_TIME64 1.

Updated.

[MaskRay]

In D119358#3370522, @ccross wrote:

In D119358#3370467, @MaskRay wrote:

fix using compiler-rt with 32-bit musl libc.

This sentence in the summary needs to be clarified for the list of 32-bit architectures using #define _REDIR_TIME64 1.

Updated.

If you make edits to the webpage, you can copy the new summary to your local git commit message with arc amend :)

[ccross]

Rebased

[pirama]

Looks like all comments are addressed. I'll merge after a quick ninja check.

[pirama]

My local ninja check-sanitizer passed but the automated build has failed: https://reviews.llvm.org/B153417. Let me do a ninja check-all and fix.

[pirama]

In D119358#3371019, @pirama wrote:

My local ninja check-sanitizer passed but the automated build has failed: https://reviews.llvm.org/B153417. Let me do a ninja check-all and fix.

Actually a false alarm. https://reviews.llvm.org/D121326 caused the break.

compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h

 #if SANITIZER_SOLARIS && !defined(_LP64)
 typedef long pid_t;
 #else
 typedef int pid_t;
 #endif
 
 #if SANITIZER_FREEBSD || SANITIZER_NETBSD || SANITIZER_MAC ||             \
     (SANITIZER_SOLARIS && (defined(_LP64) || _FILE_OFFSET_BITS == 64)) || \
+    (SANITIZER_LINUX && !SANITIZER_GLIBC && !SANITIZER_ANDROID) ||        \
     (SANITIZER_LINUX && (defined(__x86_64__) || defined(__hexagon__)))
 typedef u64 OFF_T;
 #else
 typedef uptr OFF_T;
 #endif
 typedef u64  OFF64_T;
 
 #if (SANITIZER_WORDSIZE == 64) || SANITIZER_MAC

compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h

 
 struct __sanitizer_group {
   char *gr_name;
   char *gr_passwd;
   int gr_gid;
   char **gr_mem;
 };
 
-#  if (defined(__x86_64__) && !defined(_LP64)) || defined(__hexagon__)
+#  if (SANITIZER_LINUX && !SANITIZER_GLIBC && !SANITIZER_ANDROID) || \
+      (defined(__x86_64__) && !defined(_LP64)) || defined(__hexagon__)
 typedef long long __sanitizer_time_t;
 #else
 typedef long __sanitizer_time_t;
 #endif
 
 typedef long __sanitizer_suseconds_t;
 
 struct __sanitizer_timeval {

 
 #if SANITIZER_MAC
 struct __sanitizer_dirent {
   unsigned long long d_ino;
   unsigned long long d_seekoff;
   unsigned short d_reclen;
   // more fields that we don't care about
 };
-#  elif SANITIZER_ANDROID || defined(__x86_64__) || defined(__hexagon__)
+#  elif (SANITIZER_LINUX && !SANITIZER_GLIBC) || defined(__x86_64__) || \
+      defined(__hexagon__)
 struct __sanitizer_dirent {
   unsigned long long d_ino;
   unsigned long long d_off;
   unsigned short d_reclen;
   // more fields that we don't care about
 };
 #  else
 struct __sanitizer_dirent {