See https://www.musl-libc.org/
This is activated with -DCOMPILER_RT_BUILD_WITH_MUSL=ON.
It defines MUSL=1.
This is done like that since musl does have this kind of define, see
https://www.openwall.com/lists/musl/2013/03/29/13.
Details
Details
Diff Detail
Diff Detail
- Repository
- rCRT Compiler Runtime
- Build Status
Buildable 33904 Build 33903: arc lint + arc unit
Event Timeline
Comment Actions
With the following example:
#include <stdlib.h>
int main() {
char *x = (char*)malloc(10 * sizeof(char*));
free(x);
return x[5];
}I get:
=================================================================
==22531==ERROR: AddressSanitizer: heap-use-after-free on address 0x607000000025 at pc 0x5652fccda39b bp 0x7ffe5e5c5ca0 sp 0x7ffe5e5c5c98
READ of size 1 at 0x607000000025 thread T0
#0 0x5652fccda39a in main /home/billiob/dev/foo/foo.c:5:10
#1 0x7ff78eebf228 (/lib/ld-musl-x86_64.so.1+0x1c228)
0x607000000025 is located 5 bytes inside of 80-byte region [0x607000000020,0x607000000070)
freed by thread T0 here:
#0 0x5652fcc9b9e0 in __interceptor_free /home/billiob/dev/compiler-rt/lib/asan/asan_malloc_linux.cc:123:3
#1 0x5652fccda36a in main /home/billiob/dev/foo/foo.c:4:3
#2 0x7ff78eebf228 (/lib/ld-musl-x86_64.so.1+0x1c228)
previously allocated by thread T0 here:
#0 0x5652fcc9bce8 in __interceptor_malloc /home/billiob/dev/compiler-rt/lib/asan/asan_malloc_linux.cc:145:3
#1 0x5652fccda35f in main /home/billiob/dev/foo/foo.c:3:20
#2 0x7ff78eebf228 (/lib/ld-musl-x86_64.so.1+0x1c228)
SUMMARY: AddressSanitizer: heap-use-after-free /home/billiob/dev/foo/foo.c:5:10 in main
Shadow bytes around the buggy address:
0x0c0e7fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c0e7fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c0e7fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c0e7fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c0e7fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c0e7fff8000: fa fa fa fa[fd]fd fd fd fd fd fd fd fd fd fa fa
0x0c0e7fff8010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0e7fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0e7fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0e7fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0e7fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
Shadow gap: cc
==22531==ABORTINGComment Actions
Thanks for doing this!
This patch has too many #ifdefs, we can't accept it in this form.
#ifdefs cost too much in maintenance.
My usual suggestions:
- turn #ifdefs into if () where possible
- move code to separate musl-specific files
However, may I have a different suggestion?
The current libc-related sanitizer code is a huge mess because we don't have control over the LIBCs we have to support .
For the one libc we do have control over, our code is 100x simpler, and resides in that libc itself (our implementation of HWASAN for Android's bionic)
Will it be possible to have musl support the sanitizers instead of sanitizers support musl?
Comment Actions
I second the question, especially since I'm pretty sure that everything the existing asan code here is doing that makes it "work" on musl is poking at implementation internals that are not public interfaces and have no reason to continue "working" however they are now. A long time ago the question was raised of whether/how libcs could provide mechanisms for sanitizers to work, but the ideas I saw at the time were based on hooks, which we didn't consider reasonable from the musl side - as soon as hooks are offered as stable API, they WILL be used not just by sanitizers but by applications.
Also, I think most of the ifdefs above are conceptually wrong - they're hardcoding assumptions about musl and other libcs rather than detecting them. These should all be replaced by tests in the build system and #ifdef HAVE_... or similar.
Comment Actions
I looked at how the current files were written and I decided to do it like that.
This is mostly a proof of concept and I'm looking for feedback on how to improve it.
I'm not affiliated with musl (but I'm glad @dalias looked into it) and I don't have enough knowledge to know what to do there. Could you please help me a bit more on what would be expected?
What should I do to get this feature merged? Would feature-testing (that would still add many #ifdefs) be good enough?
Comment Actions
Frankly, I would like to see a parallel effort in Musl.
I've seen the recent comment at https://github.com/google/sanitizers/issues/1080#issuecomment-506565699
suggesting that Musl is not likely to accept this approach.
But please look at the situation from our perspective:
this amount of #ifdefs in the code will increase the cost of maintenance for us, w/o giving much to anyone who doesn't use Musl.
And this is a wrong technical decision anyway, as we've known in 2011 (just didn't have other choice)
and as we've confirmed with our current implementation of HWASAN in Bionic.
If you are enthusiastic about having sanitizers support Musl w/o changes in Musl,
then the right way would be to move all Musl-specific code to Musl-specific files.
This will be quite a bit of code duplication, but it won't put any burden on us.
Comment Actions
(!defined(__MUSL__) || !__MUSL__) is pretty ugly. There is obviously a need for SANITIZER_MUSL, but then it's up to Linux maintainers whether to accept it.
Comment Actions
I think entire SANITIZER_MUSL is unneeded. #if SANITIZER_LINUX && !SANITIZER_ANDROID should be #if SANITIZER_GNU or
#if SANITIZER_GNU || SANITIZER_FREEBSD, --which we already have--, since we are pretty much using only GNU extensions (some of which
FreeBSD supports) inside those parts. This makes it a bit cleaner, sticks more to the standards, and does not need a compiler switch. i.e.
diff --git a/compiler-rt/lib/interception/interception_linux.cpp b/compiler-rt/lib/interception/interception_linux.cpp index 950cd512653..488e094e652 100644 --- a/compiler-rt/lib/interception/interception_linux.cpp +++ b/compiler-rt/lib/interception/interception_linux.cpp @@ -63,8 +63,8 @@ bool InterceptFunction(const char *name, uptr *ptr_to_real, uptr func, return addr && (func == wrapper); } -// Android and Solaris do not have dlvsym -#if !SANITIZER_ANDROID && !SANITIZER_SOLARIS && !SANITIZER_OPENBSD +// dlvsym is GNU extension, FreeBSD has support for it +#if SANITIZER_GNU || SANITIZER_FREEBSD static void *GetFuncAddr(const char *name, const char *ver) { return dlvsym(RTLD_NEXT, name, ver); } @@ -75,7 +75,7 @@ bool InterceptFunction(const char *name, const char *ver, uptr *ptr_to_real, *ptr_to_real = (uptr)addr; return addr && (func == wrapper); } -#endif // !SANITIZER_ANDROID +#endif // SANITIZER_GNU || SANITIZER_FREEBSD } // namespace __interception diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp index e71515f12e9..0176e77e143 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp @@ -57,8 +57,10 @@ #include <net/route.h> #endif -#if !SANITIZER_ANDROID +#if SANITIZER_GNU #include <fstab.h> +#endif +#if !SANITIZER_ANDROID #include <sys/mount.h> #include <sys/timeb.h> #include <utmpx.h> @@ -109,7 +111,7 @@ typedef struct user_fpregs elf_fpregset_t; #include <wordexp.h> #endif -#if SANITIZER_LINUX && !SANITIZER_ANDROID +#if SANITIZER_GNU #include <glob.h> #include <obstack.h> #include <mqueue.h> @@ -140,17 +142,32 @@ typedef struct user_fpregs elf_fpregset_t; #include <linux/serial.h> #include <sys/msg.h> #include <sys/ipc.h> -#include <crypt.h> -#endif // SANITIZER_LINUX && !SANITIZER_ANDROID +#endif // SANITIZER_GNU -#if SANITIZER_ANDROID +#if SANITIZER_LINUX && !SANITIZER_GNU #include <linux/kd.h> #include <linux/mtio.h> #include <linux/ppp_defs.h> #include <linux/if_ppp.h> +#if !SANITIZER_ANDROID +#include <linux/ax25.h> +#include <linux/cyclades.h> +#include <linux/ipx.h> +// #include <linux/mtio.h> doesn't have the required mtconfiginfo +// #include <linux/netrom.h> doesn't have the required nr_parms_struct +#include <linux/scc.h> +#include <linux/mroute.h> +#include <linux/if_eql.h> +#include <linux/lp.h> +#include <linux/if_plip.h> +#include <linux/netrom.h> +#include <linux/ipc.h> +#include <linux/shm.h> #endif +#endif // SANITIZER_LINUX && !SANITIZER_GNU #if SANITIZER_LINUX +#include <crypt.h> #include <link.h> #include <sys/vfs.h> #include <sys/epoll.h> @@ -201,12 +218,12 @@ namespace __sanitizer { unsigned struct_statfs64_sz = sizeof(struct statfs64); #endif // SANITIZER_MAC && !SANITIZER_IOS -#if !SANITIZER_ANDROID +#if SANITIZER_GNU unsigned struct_fstab_sz = sizeof(struct fstab); unsigned struct_statfs_sz = sizeof(struct statfs); unsigned struct_sockaddr_sz = sizeof(struct sockaddr); unsigned ucontext_t_sz = sizeof(ucontext_t); -#endif // !SANITIZER_ANDROID +#endif // SANITIZER_GNU #if SANITIZER_LINUX unsigned struct_epoll_event_sz = sizeof(struct epoll_event); @@ -231,7 +248,7 @@ namespace __sanitizer { // has been removed from glibc 2.28. #if defined(__aarch64__) || defined(__s390x__) || defined (__mips64) \ || defined(__powerpc64__) || defined(__arch64__) || defined(__sparcv9) \ - || defined(__x86_64__) || (defined(__riscv) && __riscv_xlen == 64) + || defined(__x86_64__) #define SIZEOF_STRUCT_USTAT 32 #elif defined(__arm__) || defined(__i386__) || defined(__mips__) \ || defined(__powerpc__) || defined(__s390__) || defined(__sparc__) @@ -245,12 +262,12 @@ namespace __sanitizer { unsigned struct_crypt_data_sz = sizeof(struct crypt_data); #endif // SANITIZER_LINUX && !SANITIZER_ANDROID -#if SANITIZER_LINUX && !SANITIZER_ANDROID +#if SANITIZER_GNU unsigned struct_timex_sz = sizeof(struct timex); unsigned struct_msqid_ds_sz = sizeof(struct msqid_ds); unsigned struct_mq_attr_sz = sizeof(struct mq_attr); unsigned struct_statvfs_sz = sizeof(struct statvfs); -#endif // SANITIZER_LINUX && !SANITIZER_ANDROID +#endif // SANITIZER_GNU const uptr sig_ign = (uptr)SIG_IGN; const uptr sig_dfl = (uptr)SIG_DFL; @@ -262,7 +279,7 @@ namespace __sanitizer { #endif -#if SANITIZER_LINUX && !SANITIZER_ANDROID +#if SANITIZER_GNU unsigned struct_shminfo_sz = sizeof(struct shminfo); unsigned struct_shm_info_sz = sizeof(struct shm_info); int shmctl_ipc_stat = (int)IPC_STAT; @@ -298,12 +315,12 @@ unsigned struct_ElfW_Phdr_sz = sizeof(ElfW(Phdr)); unsigned struct_ElfW_Phdr_sz = sizeof(Elf_Phdr); #endif -#if SANITIZER_LINUX && !SANITIZER_ANDROID +#if SANITIZER_GNU int glob_nomatch = GLOB_NOMATCH; int glob_altdirfunc = GLOB_ALTDIRFUNC; #endif -#if SANITIZER_LINUX && !SANITIZER_ANDROID && \ +#if SANITIZER_GNU && \ (defined(__i386) || defined(__x86_64) || defined(__mips64) || \ defined(__powerpc64__) || defined(__aarch64__) || defined(__arm__) || \ defined(__s390__)) @@ -417,7 +434,9 @@ unsigned struct_ElfW_Phdr_sz = sizeof(Elf_Phdr); unsigned struct_input_id_sz = sizeof(struct input_id); unsigned struct_mtpos_sz = sizeof(struct mtpos); unsigned struct_rtentry_sz = sizeof(struct rtentry); +#if SANITIZER_GNU unsigned struct_termio_sz = sizeof(struct termio); +#endif unsigned struct_vt_consize_sz = sizeof(struct vt_consize); unsigned struct_vt_sizes_sz = sizeof(struct vt_sizes); unsigned struct_vt_stat_sz = sizeof(struct vt_stat); @@ -443,7 +462,9 @@ unsigned struct_ElfW_Phdr_sz = sizeof(Elf_Phdr); #endif // SANITIZER_LINUX #if SANITIZER_LINUX && !SANITIZER_ANDROID +#if SANITIZER_GNU unsigned struct_ax25_parms_struct_sz = sizeof(struct ax25_parms_struct); +#endif unsigned struct_cyclades_monitor_sz = sizeof(struct cyclades_monitor); #if EV_VERSION > (0x010000) unsigned struct_input_keymap_entry_sz = sizeof(struct input_keymap_entry); @@ -455,13 +476,17 @@ unsigned struct_ElfW_Phdr_sz = sizeof(Elf_Phdr); unsigned struct_kbentry_sz = sizeof(struct kbentry); unsigned struct_kbkeycode_sz = sizeof(struct kbkeycode); unsigned struct_kbsentry_sz = sizeof(struct kbsentry); +#if SANITIZER_GNU unsigned struct_mtconfiginfo_sz = sizeof(struct mtconfiginfo); unsigned struct_nr_parms_struct_sz = sizeof(struct nr_parms_struct); +#endif unsigned struct_scc_modem_sz = sizeof(struct scc_modem); unsigned struct_scc_stat_sz = sizeof(struct scc_stat); +#if SANITIZER_GNU unsigned struct_serial_multiport_struct_sz = sizeof(struct serial_multiport_struct); unsigned struct_serial_struct_sz = sizeof(struct serial_struct); +#endif unsigned struct_sockaddr_ax25_sz = sizeof(struct sockaddr_ax25); unsigned struct_unimapdesc_sz = sizeof(struct unimapdesc); unsigned struct_unimapinit_sz = sizeof(struct unimapinit); @@ -477,7 +502,11 @@ unsigned struct_ElfW_Phdr_sz = sizeof(Elf_Phdr); unsigned struct_sioc_vif_req_sz = sizeof(struct sioc_vif_req); #endif +#if defined(BUFSIZ) const unsigned long __sanitizer_bufsiz = BUFSIZ; +#else + const unsigned long __sanitizer_bufsiz = 4096; +#endif const unsigned IOCTL_NOT_PRESENT = 0; @@ -869,31 +898,41 @@ unsigned struct_ElfW_Phdr_sz = sizeof(Elf_Phdr); unsigned IOCTL_LPSETIRQ = LPSETIRQ; unsigned IOCTL_LPTIME = LPTIME; unsigned IOCTL_LPWAIT = LPWAIT; +#if SANITIZER_GNU unsigned IOCTL_MTIOCGETCONFIG = MTIOCGETCONFIG; unsigned IOCTL_MTIOCSETCONFIG = MTIOCSETCONFIG; +#endif unsigned IOCTL_PIO_CMAP = PIO_CMAP; unsigned IOCTL_PIO_FONT = PIO_FONT; unsigned IOCTL_PIO_UNIMAP = PIO_UNIMAP; unsigned IOCTL_PIO_UNIMAPCLR = PIO_UNIMAPCLR; unsigned IOCTL_PIO_UNISCRNMAP = PIO_UNISCRNMAP; +#if SANITIZER_GNU unsigned IOCTL_SCSI_IOCTL_GET_IDLUN = SCSI_IOCTL_GET_IDLUN; unsigned IOCTL_SCSI_IOCTL_PROBE_HOST = SCSI_IOCTL_PROBE_HOST; unsigned IOCTL_SCSI_IOCTL_TAGGED_DISABLE = SCSI_IOCTL_TAGGED_DISABLE; unsigned IOCTL_SCSI_IOCTL_TAGGED_ENABLE = SCSI_IOCTL_TAGGED_ENABLE; +#endif unsigned IOCTL_SIOCAIPXITFCRT = SIOCAIPXITFCRT; unsigned IOCTL_SIOCAIPXPRISLT = SIOCAIPXPRISLT; unsigned IOCTL_SIOCAX25ADDUID = SIOCAX25ADDUID; unsigned IOCTL_SIOCAX25DELUID = SIOCAX25DELUID; +#if SANITIZER_GNU unsigned IOCTL_SIOCAX25GETPARMS = SIOCAX25GETPARMS; +#endif unsigned IOCTL_SIOCAX25GETUID = SIOCAX25GETUID; unsigned IOCTL_SIOCAX25NOUID = SIOCAX25NOUID; +#if SANITIZER_GNU unsigned IOCTL_SIOCAX25SETPARMS = SIOCAX25SETPARMS; +#endif unsigned IOCTL_SIOCDEVPLIP = SIOCDEVPLIP; unsigned IOCTL_SIOCIPXCFGDATA = SIOCIPXCFGDATA; unsigned IOCTL_SIOCNRDECOBS = SIOCNRDECOBS; +#if SANITIZER_GNU unsigned IOCTL_SIOCNRGETPARMS = SIOCNRGETPARMS; unsigned IOCTL_SIOCNRRTCTL = SIOCNRRTCTL; unsigned IOCTL_SIOCNRSETPARMS = SIOCNRSETPARMS; +#endif unsigned IOCTL_TIOCGSERIAL = TIOCGSERIAL; unsigned IOCTL_TIOCSERGETMULTI = TIOCSERGETMULTI; unsigned IOCTL_TIOCSERSETMULTI = TIOCSERSETMULTI; @@ -964,7 +1003,7 @@ CHECK_SIZE_AND_OFFSET(dl_phdr_info, dlpi_phdr); CHECK_SIZE_AND_OFFSET(dl_phdr_info, dlpi_phnum); #endif // SANITIZER_LINUX || SANITIZER_FREEBSD -#if (SANITIZER_LINUX || SANITIZER_FREEBSD) && !SANITIZER_ANDROID +#if (SANITIZER_GNU || SANITIZER_FREEBSD) CHECK_TYPE_SIZE(glob_t); CHECK_SIZE_AND_OFFSET(glob_t, gl_pathc); CHECK_SIZE_AND_OFFSET(glob_t, gl_pathv); @@ -998,6 +1037,7 @@ CHECK_TYPE_SIZE(iovec); CHECK_SIZE_AND_OFFSET(iovec, iov_base); CHECK_SIZE_AND_OFFSET(iovec, iov_len); +#if SANITIZER_GNU || SANITIZER_ANDROID CHECK_TYPE_SIZE(msghdr); CHECK_SIZE_AND_OFFSET(msghdr, msg_name); CHECK_SIZE_AND_OFFSET(msghdr, msg_namelen); @@ -1011,6 +1051,11 @@ CHECK_TYPE_SIZE(cmsghdr); CHECK_SIZE_AND_OFFSET(cmsghdr, cmsg_len); CHECK_SIZE_AND_OFFSET(cmsghdr, cmsg_level); CHECK_SIZE_AND_OFFSET(cmsghdr, cmsg_type); +#endif + +#ifndef __GLIBC_PREREQ +#define __GLIBC_PREREQ(x, y) 0 +#endif #if SANITIZER_LINUX && (__ANDROID_API__ >= 21 || __GLIBC_PREREQ (2, 14)) CHECK_TYPE_SIZE(mmsghdr); @@ -1116,7 +1161,7 @@ CHECK_SIZE_AND_OFFSET(mntent, mnt_passno); CHECK_TYPE_SIZE(ether_addr); -#if (SANITIZER_LINUX || SANITIZER_FREEBSD) && !SANITIZER_ANDROID +#if SANITIZER_GNU || SANITIZER_FREEBSD CHECK_TYPE_SIZE(ipc_perm); # if SANITIZER_FREEBSD CHECK_SIZE_AND_OFFSET(ipc_perm, key); @@ -1178,7 +1223,8 @@ CHECK_SIZE_AND_OFFSET(ifaddrs, ifa_dstaddr); CHECK_SIZE_AND_OFFSET(ifaddrs, ifa_data); #endif -#if SANITIZER_LINUX +// mallinfo is annoying, cause it's SVID instead of POSIX or C standard... +#if SANITIZER_GNU COMPILER_CHECK(sizeof(__sanitizer_struct_mallinfo) == sizeof(struct mallinfo)); #endif @@ -1228,7 +1274,8 @@ COMPILER_CHECK(__sanitizer_XDR_DECODE == XDR_DECODE); COMPILER_CHECK(__sanitizer_XDR_FREE == XDR_FREE); #endif -#if SANITIZER_LINUX && !SANITIZER_ANDROID +// #if SANITIZER_LINUX && !SANITIZER_ANDROID +#if SANITIZER_GNU COMPILER_CHECK(sizeof(__sanitizer_FILE) <= sizeof(FILE)); CHECK_SIZE_AND_OFFSET(FILE, _flags); CHECK_SIZE_AND_OFFSET(FILE, _IO_read_ptr); @@ -1247,7 +1294,7 @@ CHECK_SIZE_AND_OFFSET(FILE, _chain); CHECK_SIZE_AND_OFFSET(FILE, _fileno); #endif -#if SANITIZER_LINUX && !SANITIZER_ANDROID +#if SANITIZER_GNU COMPILER_CHECK(sizeof(__sanitizer__obstack_chunk) <= sizeof(_obstack_chunk)); CHECK_SIZE_AND_OFFSET(_obstack_chunk, limit); CHECK_SIZE_AND_OFFSET(_obstack_chunk, prev); -- 2.23.0
(This is the patch I've been using to get compiler-rt to compile with musl, albeit without sanitisers)
edit: Nevermind, I don't know where I got that SANITIZER_GNU from, basically it checks for __GLIBC__, but it seems it is not in the official master tree.
Comment Actions
I have deployed the patch from @ronchaine in https://reviews.llvm.org/D63785#1921014 ; replacing instances of SANITIZER_GNU with defined(__GLIBC__) as suggested, and this builds correctly, disabling sanitizers per USE flags in Gentoo linux. We are tracking this issue as https://bugs.gentoo.org/713072 locally, so it would be useful to see whether we can make some progress here.
Comment Actions
@ronchaine Can you post a standalone patch? https://llvm.org/docs/GettingStarted.html#sending-patches has instructions. Once you've configured arc, it is simply arc diff 'HEAD^'.
Comment Actions
@ronchaine Can you post a standalone patch? https://llvm.org/docs/GettingStarted.html#sending-patches has instructions. Once you've configured arc, it is simply arc diff 'HEAD^'.
Yeah, I can do this. I need to rebase that branch to the latest master and figure out how that arc works. Just give me 24 hours.