sanitizer_common: Introduce internal_madvise and start using it.

[pcc]

Summary

A recent change to sanitizer_common caused us to issue the syscall
madvise(MADV_HUGEPAGE) during HWASAN initialization. This may lead to a
problem if madvise is instrumented (e.g. because libc is instrumented
or the user intercepted it). For example, on Android the syscall may
fail if the kernel does not support transparent hugepages, which leads
to an attempt to set errno in a HWASAN instrumented function. Avoid
this problem by introducing a syscall wrapper and using it to issue
this syscall.

Tested only on Linux; includes untested updates for the other
platforms.

[eugenis]

LGTM

[ro]

compiler-rt/lib/sanitizer_common/sanitizer_solaris.cpp

79

I don't know why but you reintroduced an older/broken version of the madvise declaration here. The one you removed from sanitizer_posix_libcdep.cpp had the first arg as void * for a reason.

[pcc]

compiler-rt/lib/sanitizer_common/sanitizer_solaris.cpp

79

Oh, I thought that was a mistake and that madvise was supposed to be defined like this on Solaris (according to the comment that I removed on sanitizer_posix_libcdep.cpp lines 70-72, and man pages that I found online, e.g. https://docs.oracle.com/cd/E36784_01/html/E36874/madvise-3c.html). If it's actually defined to take void * on Solaris then I can fix it.

[ro]

compiler-rt/lib/sanitizer_common/sanitizer_solaris.cpp

79

On the contrary: I had provided a code snippet using `void *', carefully tested on both Solaris 11.4 and Illumos in D84046. Unfortunately, the author of that patch chose to make the same change as you did, silently and at the last minute and breaking the Solaris build ;-(

I had to restore the build in D84388 and thus was particularly dismayed to see the same change going in again for no apparent reason and without testing.

Strangely, 1-stage builds with gcc 9 or 10 still do work, as can be seen on the Solaris buildbots, but when I tried a 2-stage build last night, it failed again as it had before:

/vol/llvm/src/llvm-project/dist/compiler-rt/lib/sanitizer_common/sanitizer_solaris.cpp:79:16: error: conflicting types for 'madvise'
extern "C" int madvise(caddr_t, size_t, int);
               ^
/usr/include/sys/mman.h:232:12: note: previous declaration is here
extern int madvise(void *, size_t, int);
           ^
1 error generated.

The man page you link is from Solaris 11.2, about 8 years ago. The signature has been changed in 11.4 exactly to avoid unnecessary incompatibilities with other systems.

So yes, please restore the void * version.

[pcc]

compiler-rt/lib/sanitizer_common/sanitizer_solaris.cpp

79

Okay, I sent D86166 to fix this.

I have a followup question which I will raise there, since I think the comment really needs to say *why* the declaration is this way, otherwise someone else may come along and "fix" it again.

[ro]

compiler-rt/lib/sanitizer_common/sanitizer_solaris.cpp

79

Okay, I sent D86166 to fix this.

I have a followup question which I will raise there, since I think the comment really needs to say *why* the declaration is this way, otherwise someone else may come along and "fix" it again.

But why on earth do people insist on changing/breaking working code on platforms that by their own admission they know nothing about instead of first asking someone that does for clarification? That's completely beyond me!

I do run Solaris buildbots for a reason.

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

 #if !SANITIZER_OPENBSD
 uptr internal_munmap(void *addr, uptr length) {
   return internal_syscall(SYSCALL(munmap), (uptr)addr, length);
 }
 
 int internal_mprotect(void *addr, uptr length, int prot) {
   return internal_syscall(SYSCALL(mprotect), (uptr)addr, length, prot);
 }
+
+int internal_madvise(uptr addr, uptr length, int advice) {
+  return internal_syscall(SYSCALL(madvise), addr, length, advice);
+}
 #endif
 
 uptr internal_close(fd_t fd) {
   return internal_syscall(SYSCALL(close), fd);
 }
 
 uptr internal_open(const char *filename, int flags) {
 #if SANITIZER_USES_CANONICAL_LINUX_SYSCALLS

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

uptr internal_munmap(void *addr, uptr length) {
   if (&__munmap) return __munmap(addr, length);
   return munmap(addr, length);
 }
 
 int internal_mprotect(void *addr, uptr length, int prot) {
   return mprotect(addr, length, prot);
 }
 
+int internal_madvise(uptr addr, uptr length, int advice) {
+  return madvise((void *)addr, length, advice);
+}
+
 uptr internal_close(fd_t fd) {
   return close(fd);
 }
 
 uptr internal_open(const char *filename, int flags) {
   return open(filename, flags);
 }
 

compiler-rt/lib/sanitizer_common/sanitizer_netbsd.cpp

uptr internal_munmap(void *addr, uptr length) {
   return _REAL(munmap, addr, length);
 }
 
 int internal_mprotect(void *addr, uptr length, int prot) {
   DEFINE__REAL(int, mprotect, void *a, uptr b, int c);
   return _REAL(mprotect, addr, length, prot);
 }
 
+int internal_madvise(uptr addr, uptr length, int advice) {
+  DEFINE__REAL(int, madvise, void *a, uptr b, int c);
+  return _REAL(madvise, (void *)addr, length, advice);
+}
+
 uptr internal_close(fd_t fd) {
   CHECK(&_sys_close);
   return _sys_close(fd);
 }
 
 uptr internal_open(const char *filename, int flags) {
   CHECK(&_sys_open);
   return _sys_open(filename, flags);

compiler-rt/lib/sanitizer_common/sanitizer_openbsd.cpp

 }
 
 uptr internal_munmap(void *addr, uptr length) { return munmap(addr, length); }
 
 int internal_mprotect(void *addr, uptr length, int prot) {
   return mprotect(addr, length, prot);
 }
 
+int internal_madvise(uptr addr, uptr length, int advice) {
+  return madvise((void *)addr, length, advice);
+}
+
 int internal_sysctlbyname(const char *sname, void *oldp, uptr *oldlenp,
                           const void *newp, uptr newlen) {
   Printf("internal_sysctlbyname not implemented for OpenBSD");
   Die();
   return 0;
 }
 
 uptr ReadBinaryName(/*out*/char *buf, uptr buf_len) {

compiler-rt/lib/sanitizer_common/sanitizer_posix.h

 uptr internal_read(fd_t fd, void *buf, uptr count);
 uptr internal_write(fd_t fd, const void *buf, uptr count);
 
 // Memory
 uptr internal_mmap(void *addr, uptr length, int prot, int flags,
                    int fd, u64 offset);
 uptr internal_munmap(void *addr, uptr length);
 int internal_mprotect(void *addr, uptr length, int prot);
+int internal_madvise(uptr addr, uptr length, int advice);
 
 // OS
 uptr internal_filesize(fd_t fd);  // -1 on error.
 uptr internal_stat(const char *path, void *buf);
 uptr internal_lstat(const char *path, void *buf);
 uptr internal_fstat(fd_t fd, void *buf);
 uptr internal_dup(int oldfd);
 uptr internal_dup2(int oldfd, int newfd);

compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp

 
 #if SANITIZER_FREEBSD
 // The MAP_NORESERVE define has been removed in FreeBSD 11.x, and even before
 // that, it was never implemented.  So just define it to zero.
 #undef MAP_NORESERVE
 #define MAP_NORESERVE 0
 #endif
 
-#if SANITIZER_SOLARIS
-// Illumos' declaration of madvie cannot be made visible if _XOPEN_SOURCE
-// is defined as g++ does on Solaris.
-extern "C" int madvise(void *, size_t, int);
-#endif
-
 typedef void (*sa_sigaction_t)(int, siginfo_t *, void *);
 
 namespace __sanitizer {
 
 u32 GetUid() {
   return getuid();
 }
 
 uptr GetThreadSelf() {
   return (uptr)pthread_self();
 }
 
 void ReleaseMemoryPagesToOS(uptr beg, uptr end) {
   uptr page_size = GetPageSizeCached();
   uptr beg_aligned = RoundUpTo(beg, page_size);
   uptr end_aligned = RoundDownTo(end, page_size);
   if (beg_aligned < end_aligned)
-    // In the default Solaris compilation environment, madvise() is declared
-    // to take a caddr_t arg; casting it to void * results in an invalid
-    // conversion error, so use char * instead.
-    madvise((char *)beg_aligned, end_aligned - beg_aligned,
+    internal_madvise(beg_aligned, end_aligned - beg_aligned,
                      SANITIZER_MADVISE_DONTNEED);
 }
 
 void SetShadowRegionHugePageMode(uptr addr, uptr size) {
 #ifdef MADV_NOHUGEPAGE  // May not be defined on old systems.
   if (common_flags()->no_huge_pages_for_shadow)
-    madvise((char *)addr, size, MADV_NOHUGEPAGE);
+    internal_madvise(addr, size, MADV_NOHUGEPAGE);
   else
-    madvise((char *)addr, size, MADV_HUGEPAGE);
+    internal_madvise(addr, size, MADV_HUGEPAGE);
 #endif  // MADV_NOHUGEPAGE
 }
 
 bool DontDumpShadowMemory(uptr addr, uptr length) {
 #if defined(MADV_DONTDUMP)
-  return madvise((char *)addr, length, MADV_DONTDUMP) == 0;
+  return internal_madvise(addr, length, MADV_DONTDUMP) == 0;
 #elif defined(MADV_NOCORE)
-  return madvise((char *)addr, length, MADV_NOCORE) == 0;
+  return internal_madvise(addr, length, MADV_NOCORE) == 0;
 #else
   return true;
 #endif  // MADV_DONTDUMP
 }
 
 static rlim_t getlim(int res) {
   rlimit rlim;
   CHECK_EQ(0, getrlimit(res, &rlim));

compiler-rt/lib/sanitizer_common/sanitizer_solaris.cpp

 DECLARE__REAL_AND_INTERNAL(uptr, munmap, void *addr, uptr length) {
   return _REAL(munmap)(addr, length);
 }
 
 DECLARE__REAL_AND_INTERNAL(int, mprotect, void *addr, uptr length, int prot) {
   return _REAL(mprotect)(addr, length, prot);
 }
 
+// Illumos' declaration of madvise cannot be made visible if _XOPEN_SOURCE
+// is defined as g++ does on Solaris.
+extern "C" int madvise(caddr_t, size_t, int);
+
+int internal_madvise(uptr addr, uptr length, int advice) {
+  return madvise((caddr_t)addr, length, advice);
+}
+
 DECLARE__REAL_AND_INTERNAL(uptr, close, fd_t fd) {
   return _REAL(close)(fd);
 }
 
 extern "C" int _REAL64(open)(const char *, int, ...);
 
 uptr internal_open(const char *filename, int flags) {
   return _REAL64(open)(filename, flags);