[PATCH] D57179: Enhance support for NetBSD in SafeStack

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: lib/safestack/safestack_platform.h:35
+
+extern "C" void *__mmap(void *, size_t, int, int, int, int, off_t)
+    __attribute__((weak));
----------------
Why do we need weak here?


================
Comment at: lib/safestack/safestack_platform.h:46
+#if SANITIZER_NETBSD
+static void *GetRealLibcAddress(const char *symbol) {
+  void *real = dlsym(RTLD_NEXT, symbol);
----------------
maybe merge this and "#if SANITIZER_NETBSD" above?


================
Comment at: lib/safestack/safestack_platform.h:84
 #if SANITIZER_NETBSD
+  DEFINE__REAL(int, _lwp_kill, int a, int b);
   (void)pid;
----------------
Why do you rely need DEFINE__REAL? I just copied TgKill for other platforms. Not sure that syscalls are necessary.
mmap/protect is a different story, see below


================
Comment at: lib/safestack/safestack_platform.h:116
 inline int Mprotect(void *addr, size_t length, int prot) {
 #if SANITIZER_NETBSD
+  DEFINE__REAL(int, mprotect, void *a, size_t b, int c);
----------------
We don't want to hit interpector here, because it can be called called from preinit_array.
DEFINE__REAL will avoid interceptors, however dlsym from preinit_array may crash as it can to try to allocate memory.
So we use syscall on linux. 
sanitizer_common/sanitizer_netbsd still had such problem, so I didn't bother to copy DEFINE_REAL.



Repository:
  rL LLVM

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

https://reviews.llvm.org/D57179