[PATCH] D39124: Add NetBSD improvements in sanitizers

[Kamil Rytarowski via Phabricator via llvm-commits]

krytarowski added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:4422
+  COMMON_INTERCEPTOR_READ_STRING(ctx, name, 0);
+  COMMON_INTERCEPTOR_SET_PTHREAD_NAME(ctx, thread, name, arg);
+  return REAL(pthread_setname_np)(thread, name, arg);
----------------
dvyukov wrote:
> COMMON_INTERCEPTOR_* is interface with the rest of the system (asan, tsan, msan, lsan, ...). It _must_ be platform-independent. We don't want gazillion of OS/arch-specific details plague rest of the codebase. See how you now need to update all of asan/tsan/msan (and forgot esan/esan_interceptors.cpp).
> Keep the interface as it is now and handle NetBSDs-specific difference in NetBSDs-specific interceptor. Namely, do the printf here and pass formatted name to COMMON_INTERCEPTOR_SET_PTHREAD_NAME.
Unless esan will be simple to be ported, it will wait for finishing LLDB.

Right now lsan isn't possible due to `StopTheWorld()` OS capability requirements.  I plan to introduce a new `ptrace(2)` API to self-introspect in the future. But this must wait for finishing `ptrace(2)`/signals/etc correctness for debuggers (there are few bugs out there).

I'm going to research this NetBSD-specific interceptor.


================
Comment at: lib/tsan/rtl/tsan_platform_posix.cc:127
       continue;
+#if !SANITIZER_NETBSD
     if (segment.start >= VdsoBeg())  // vdso
----------------
dvyukov wrote:
> Why is this necessary?
> VdsoBeg is 0xf000000000000000ull, why do we need to continue after this address on NetBSD?
This can be considered as a leftover from porting efforts. I will drop it.


Repository:
  rL LLVM

https://reviews.llvm.org/D39124