[PATCH] D39124: Add NetBSD improvements in sanitizers

[Dmitry Vyukov via Phabricator via llvm-commits]

dvyukov added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:4420
+  void *ctx;
+  char newname[32]; // PTHREAD_MAX_NAMELEN_NP=32
+  int namelen;
----------------
Please move everything except "void *ctx" into COMMON_INTERCEPTOR_ENTER scope as all other interceptors do. This reduces chances of actual code appear outside of interceptor scope.


================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:4424
+  namelen = internal_snprintf(newname, sizeof(newname), name, arg);
+  COMMON_INTERCEPTOR_READ_STRING(ctx, newname, namelen);
+  COMMON_INTERCEPTOR_SET_PTHREAD_NAME(ctx, thread, newname);
----------------
This should be:

COMMON_INTERCEPTOR_READ_STRING(ctx, name, 0);

reading our local stack variable does not have any effect.


================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:191
 uptr internal_open(const char *filename, int flags) {
-#if SANITIZER_USES_CANONICAL_LINUX_SYSCALLS
+#if SANITIZER_NETBSD
+  return internal_syscall_ptr(SYSCALL(open), (uptr)filename, flags);
----------------
Uh, this doubles number of ifdefs because of netbsd. The code will be much cleaner if we introduce internal_syscall/syscall64/syscall_ptr for all OSes. Just on all other OSes they all will be defined as syscall. Are there any major obstacles for doing it this way?


================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:452
+#if SANITIZER_NETBSD
+  int res = internal_syscall_ptr(SYSCALL(nanosleep), &ts, &ts);
+#else
----------------
nanosleep declared as returning int, why is this syscall_ptr?


================
Comment at: lib/tsan/rtl/tsan_platform.h:47
+C/C++ on netbsd/amd64 can reuse the same mapping:
+ * The address space startx from 0x1000 (option with 0x0) and ends with
+   0x7f7ffffff000.
----------------
s/startx/starts/


Repository:
  rL LLVM

https://reviews.llvm.org/D39124