[PATCH] D43909: [3/3] OpenBSD UBsan support / common

[Kamil Rytarowski via Phabricator via llvm-commits]

krytarowski added inline comments.


================
Comment at: cmake/base-config-ix.cmake:142
       if(NOT MSVC)
-        test_target_arch(x86_64 "" "-m64")
-        test_target_arch(i386 __i386__ "-m32")
+        if(CMAKE_SYSTEM_NAME MATCHES "OpenBSD")
+          if (CMAKE_SIZEOF_VOID_P EQUAL 4)
----------------
Rationale?


================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:4410
 INTERCEPTOR_PTHREAD_ATTR_GET(guardsize, sizeof(SIZE_T))
+#if !SANITIZER_OPENBSD
 INTERCEPTOR_PTHREAD_ATTR_GET(schedparam, struct_sched_param_sz)
----------------
Same here as bellow regarding `INIT_PTHREAD_ATTR_GET`


================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:4442
+#if !SANITIZER_OPENBSD
 #define INIT_PTHREAD_ATTR_GET                             \
   COMMON_INTERCEPT_FUNCTION(pthread_attr_getdetachstate); \
----------------
Split this `INIT_PTHREAD_ATTR_GET` into two versions, one with shared interceptors, the other with interceptors for !OPENBSD.


================
Comment at: lib/sanitizer_common/sanitizer_interface_internal.h:72
 
+#if !SANITIZER_OPENBSD
   SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
----------------
Rationale?


================
Comment at: lib/sanitizer_common/sanitizer_internal_defs.h:104
 #if ((SANITIZER_LINUX && !SANITIZER_ANDROID) || \
-  SANITIZER_FREEBSD) && !defined(PIC)
+  SANITIZER_FREEBSD || SANITIZER_OPENBSD) && !defined(PIC)
 # define SANITIZER_CAN_USE_PREINIT_ARRAY 1
----------------
Extract FREEBSD and OPENBSD from this `#if` and follow SOLARIS adding a dedicated entry with a comment.


================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:316
+#elif SANITIZER_OPENBSD
+  return internal_syscall(SYSCALL(fstatat), AT_FDCWD, (uptr)path, (uptr)buf, 0);
 #elif SANITIZER_USES_CANONICAL_LINUX_SYSCALLS
----------------
`internal_syscall_ptr`


================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:401
+#if SANITIZER_OPENBSD || SANITIZER_USES_CANONICAL_LINUX_SYSCALLS
+  return internal_syscall(SYSCALL(readlinkat), AT_FDCWD, (uptr)path,
+                          (uptr)buf, bufsize);
----------------
`internal_syscall_ptr`


================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:410
+#if SANITIZER_OPENBSD || SANITIZER_USES_CANONICAL_LINUX_SYSCALLS
   return internal_syscall(SYSCALL(unlinkat), AT_FDCWD, (uptr)path, 0);
 #else
----------------
`internal_syscall_ptr`


================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:418
+#if SANITIZER_OPENBSD || SANITIZER_USES_CANONICAL_LINUX_SYSCALLS
   return internal_syscall(SYSCALL(renameat), AT_FDCWD, (uptr)oldpath, AT_FDCWD,
                           (uptr)newpath);
----------------
`internal_syscall_ptr`


================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:467
+#if SANITIZER_FREEBSD || SANITIZER_OPENBSD
   return (uptr)pthread_self();
 #elif SANITIZER_NETBSD
----------------
This is wrong for FreeBSD and OpenBSD.

`thr_self()` for FreeBSD
`syscall(SYS_getthrid)` for OpenBSD


================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:486
   internal_syscall_ptr(SYSCALL(gettimeofday), &tv, 0);
-  return (u64)tv.tv_sec * 1000*1000*1000 + tv.tv_usec * 1000;
+  return (u64)tv.tv_sec * 1000 * 1000 * 1000 + tv.tv_usec * 1000;
 }
----------------
Please remove all unrelated changes for this file.


================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:1732
+#if SANITIZER_OPENBSD
+  return UNKNOWN;
+#endif
----------------
Use `#else` at the end.


================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:1780
+  *sp = 0;
+#else
 #if SANITIZER_NETBSD
----------------
`#elif SANITIZER_NETBSD`


================
Comment at: lib/sanitizer_common/sanitizer_platform_interceptors.h:460
 
-#define SANITIZER_INTERCEPT_ACCT SI_NETBSD
+#define SANITIZER_INTERCEPT_ACCT SI_NETBSD || SI_OPENBSD
 #define SANITIZER_INTERCEPT_USER_FROM_UID SI_NETBSD
----------------
Please use braces in this file, like: `(SI_NETBSD || SI_OPENBSD)`.


================
Comment at: lib/sanitizer_common/sanitizer_procmaps.h:99
 #endif  // SANITIZER_LINUX || SANITIZER_FREEBSD || SANITIZER_NETBSD ||
         // SANITIZER_MAC || SANITIZER_SOLARIS
 #endif  // SANITIZER_PROCMAPS_H
----------------
missing OPENBSD


================
Comment at: lib/sanitizer_common/sanitizer_stacktrace.h:26
 # define SANITIZER_CAN_FAST_UNWIND 0
+#elif SANITIZER_OPENBSD
+# define SANITIZER_CAN_FAST_UNWIND 0
----------------
I would merge it with `SANITIZER_WINDOWS`.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D43909