Differential D119358
[CompilerRT] Fix build of compiler-rt with musl Authored by ccross on Feb 9 2022, 10:59 AM.
Details Use the correct types for OFF_T, sanitizer_time_t and Also redirect the time_t functions that are affected by
Diff Detail Event TimelineComment Actions 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. Comment Actions 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... Comment Actions 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__) ...). Comment Actions 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.
Perhaps check __SIZEOF_POINTER__ or _ILP32
Comment Actions
This sentence in the summary needs to be clarified for the list of 32-bit architectures using #define _REDIR_TIME64 1. Comment Actions If you make edits to the webpage, you can copy the new summary to your local git commit message with arc amend :) Comment Actions 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. | |||||||||||||||||||||
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.