[compiler-rt] fix build on illumos

[devnexen]

Summary

• there are additional fields for glob_t struct thus size check is failing.
• to access old mman.h api based on caddr_t, _XOPEN_SOURCE needs not to be defined.
• prxmap_t constified.

[ro]

Thanks for doing this. It's a first step, but according to a quick test some time ago, much more will be needed. I expect there will be follow-up
patches to address those issues (missing support for ld --version-script, unconditional use of ld --as-needed, probably more)?

One point up front: please be more detailed and specific about which failures you're dealing with here. E.g. the comment about

old mman.h api based on caddr_t

is simply wrong: apart from failing to mention the affected function (madvise in my testing on OpenIndiana 2020.04), the problem is that
madvise isn't declared at all if XOPEN_SOURCE is defined.

Solaris 10 had the same problem, but at some point int the Solaris 11 cycle the prototype was made visible if __EXTENSIONS__ is defined,
just as expected. Independent of the current patch Illumos may want to do this, too.

compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_solaris.cpp

205

Please remove the "non posix" part: POSIX always allows for
additional fields. The point is that they differ between Solaris
and Illumos.

I guess "There are additional fields that we are not interested in."
would be better.

compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp

27

If this goes in at all, it needs to be before the various sanitizer_*.h includes above: they can include system headers, too.

In fact, I'm quite unhappy about this: it seems a terribly large hammer to get just a single missing declaration, and I fear there will be undesired side effects now or in the future.

It seems way easier to just declare madvise like

#if SANITIZER_SOLARIS
// Illumos' declaration of madvise cannot be made visible if _XOPEN_SOURCE is
// defined as g++ does on Solaris.
extern "C" int madvise(void *, size_t, int);
#endif

just before ReleaseMemoryPagesToOS. This works on both Illumos and Solaris and the risk is far lower.

AFAIK there's currently no way to distinguish between Illumos and Solaris at compile time right now. That would allow introduction of code that's needed by one but harms the other.

[devnexen]

Changes per feedback :
madvise prototype for access.

[ro]

LGTM, thanks.

[ro]

Unfortunately I hadn't noticed that you'd silently changed the madvise prototype from the version I'd suggested (and tested on both Solaris 11.4 and OpenIndiana) to one using caddr_t instead of void * for the first arg, this way breaking the Solaris build. Now fixed in D84388.

Please don't do something like this: it just creates unnecessary work.

This is exactly the reason why I asked for a way to distinguish between Illumos and Solaris at compile time (e.g. by predefining __illumos__).
Fixing one by breaking the other just isn't an option.

[devnexen]

In D84046#2168732, @ro wrote:

Unfortunately I hadn't noticed that you'd silently changed the madvise prototype from the version I'd suggested (and tested on both Solaris 11.4 and OpenIndiana) to one using caddr_t instead of void * for the first arg, this way breaking the Solaris build. Now fixed in D84388.

Please don't do something like this: it just creates unnecessary work.

This is exactly the reason why I asked for a way to distinguish between Illumos and Solaris at compile time (e.g. by predefining __illumos__).
Fixing one by breaking the other just isn't an option.

Alright apologies I copied/pasted the man page. I agree with additional illumos preprocessor constant, would be handy for some situations...

compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_solaris.cpp

 // There are more undocumented fields in dl_phdr_info that we are not interested
 // in.
 COMPILER_CHECK(sizeof(__sanitizer_dl_phdr_info) <= sizeof(dl_phdr_info));
 CHECK_SIZE_AND_OFFSET(dl_phdr_info, dlpi_addr);
 CHECK_SIZE_AND_OFFSET(dl_phdr_info, dlpi_name);
 CHECK_SIZE_AND_OFFSET(dl_phdr_info, dlpi_phdr);
 CHECK_SIZE_AND_OFFSET(dl_phdr_info, dlpi_phnum);
 
-CHECK_TYPE_SIZE(glob_t);
+// There are non posix additional fields
+COMPILER_CHECK(sizeof(__sanitizer_glob_t) <= sizeof(glob_t));
 CHECK_SIZE_AND_OFFSET(glob_t, gl_pathc);
 CHECK_SIZE_AND_OFFSET(glob_t, gl_pathv);
 CHECK_SIZE_AND_OFFSET(glob_t, gl_offs);
 
 CHECK_TYPE_SIZE(addrinfo);
 CHECK_SIZE_AND_OFFSET(addrinfo, ai_flags);
 CHECK_SIZE_AND_OFFSET(addrinfo, ai_family);
 CHECK_SIZE_AND_OFFSET(addrinfo, ai_socktype);

compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp

 #include "sanitizer_flags.h"
 #include "sanitizer_platform_limits_netbsd.h"
 #include "sanitizer_platform_limits_openbsd.h"
 #include "sanitizer_platform_limits_posix.h"
 #include "sanitizer_platform_limits_solaris.h"
 #include "sanitizer_posix.h"
 #include "sanitizer_procmaps.h"
 
+#if SANITIZER_SOLARIS
+#ifdef _XOPEN_SOURCE
+#undef _XOPEN_SOURCE
+#endif
+#endif
+
 #include <errno.h>
 #include <fcntl.h>
 #include <pthread.h>
 #include <signal.h>
 #include <stdlib.h>
 #include <sys/mman.h>
 #include <sys/resource.h>
 #include <sys/stat.h>

compiler-rt/lib/sanitizer_common/sanitizer_procmaps_solaris.cpp

void ReadProcMaps(ProcSelfMapsBuff *proc_maps) {
   }
 }
 
 bool MemoryMappingLayout::Next(MemoryMappedSegment *segment) {
   if (Error()) return false; // simulate empty maps
   char *last = data_.proc_self_maps.data + data_.proc_self_maps.len;
   if (data_.current >= last) return false;
 
-  prxmap_t *xmapentry = (prxmap_t*)data_.current;
+  prxmap_t *xmapentry =
+      const_cast<prxmap_t *>(reinterpret_cast<const prxmap_t *>(data_.current));
 
   segment->start = (uptr)xmapentry->pr_vaddr;
   segment->end = (uptr)(xmapentry->pr_vaddr + xmapentry->pr_size);
   segment->offset = (uptr)xmapentry->pr_offset;
 
   segment->protection = 0;
   if ((xmapentry->pr_mflags & MA_READ) != 0)
     segment->protection |= kProtectionRead;