[PATCH] D57179: Enhance support for NetBSD in SafeStack

[Kamil Rytarowski via Phabricator via llvm-commits]

krytarowski marked 3 inline comments as done.
krytarowski 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));
----------------
vitalybuka wrote:
> Why do we need weak here?
It was needed for sanitizer common but it seems to work fine for safestack without the weak attribute.


================
Comment at: lib/safestack/safestack_platform.h:84
 #if SANITIZER_NETBSD
+  DEFINE__REAL(int, _lwp_kill, int a, int b);
   (void)pid;
----------------
vitalybuka wrote:
> 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
For LWP functions this is optional, for consistence I prefer to keep it.


================
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);
----------------
vitalybuka wrote:
> 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.
> 
NetBSD does not use/support `preinit_array`, except on archs that demand it (amd64/i386 aren't in the set).

How does it crash with allocating the memory? By calling real malloc? Dynamic loader on NetBSD uses its internal copy of xmalloc() for allocations.

According to my quick testing with gdb(1) and an example program, dlsym(3) does not allocate anything through xmalloc() (at least for RTLD_SELF).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57179