[PATCH] D85870: sanitizer_common: Introduce internal_madvise and start using it.

[Rainer Orth via Phabricator via llvm-commits]

ro added inline comments.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_solaris.cpp:79
+// is defined as g++ does on Solaris.
+extern "C" int madvise(caddr_t, size_t, int);
+
----------------
pcc wrote:
> ro wrote:
> > 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.
> 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.
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85870/new/

https://reviews.llvm.org/D85870