[PATCH] D40898: [Sanitizers] Basic sanitizer Solaris support (PR 33274)

[Kamil Rytarowski via Phabricator via llvm-commits]

krytarowski added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_internal_defs.h:94
 # define SANITIZER_CAN_USE_PREINIT_ARRAY 1
+// Before Solaris 11.4, .preinit_array is fully supported only with GNU ld.
+// FIXME: Check for those conditions.
----------------
Is this supported in SmartOS?


================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:482
 // Like getenv, but reads env directly from /proc (on Linux) or parses the
-// 'environ' array (on FreeBSD) and does not use libc. This function should be
-// called first inside __asan_init.
+// 'environ' array (on FreeBSD and Solaris) and does not use libc. This
+// function should be called first inside __asan_init.
----------------
I would make this list of OSes generic (we could include there NetBSD as well).


================
Comment at: lib/sanitizer_common/sanitizer_platform_limits_posix.cc:17
 
-#if SANITIZER_LINUX || SANITIZER_FREEBSD || SANITIZER_MAC
+#if SANITIZER_LINUX || SANITIZER_FREEBSD || SANITIZER_MAC || SANITIZER_SOLARIS
 // Tests in this file assume that off_t-dependent data structures match the
----------------
When I was adding NetBSD support I was prompted to move all the NetBSD definitions to a dedicated file: sanitizer_platform_limits_netbsd.cc. I think the same should be repeated for Solaris.


================
Comment at: lib/sanitizer_common/sanitizer_posix_libcdep.cc:65
   if (beg_aligned < end_aligned)
+#if SANITIZER_SOLARIS
+    // In the default compilation environment, madvise() is declared to take a
----------------
Can we cast all systems to (char*)?


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D40898