Support OpenBSD in common interceptors

[devnexen]

Summary

Extract INIT_PTHREAD_ATTR_GET_SCHED from INIT_PTHREAD_ATTR_GET,
as the former is not supported on OpenBSD.

Supported interceptors
getdetachstate, getguardsize, getscope, getstacksize, getgrouplist and getstack
Unsupported
getschedparam, getgroupmembership and getschedpolicy

Patch by David CARLIER

[vitalybuka]

Could you please improve title?
Ubsan should not install pthread interceptors.

lib/sanitizer_common/CMakeLists.txt

19 ↗

(On Diff #136816)

CMakeLists.txt change should be in the patch which adds this files

lib/sanitizer_common/sanitizer_common_interceptors.inc

4430

if we SANITIZER_INTERCEPT_PTHREAD_ATTR_GET is not enough
we should add SANITIZER_INTERCEPT_PTHREAD_ATTR_GET_SCHED

are *sched* calls missing on openbsd?

[devnexen]

lib/sanitizer_common/CMakeLists.txt

19 ↗

(On Diff #136816)

Ok makes sense.

lib/sanitizer_common/sanitizer_common_interceptors.inc

4430

Yes.

[vitalybuka]

lib/sanitizer_common/sanitizer_common_interceptors.inc

4430

So please add new SANITIZER_INTERCEPT_ to avoid "#if !SANITIZER_OPENBSD" here

[vitalybuka]

Maybe retitle to "Don't intercept pthread_attr_getschedparam and pthread_attr_getschedpolicy on OpenBSD" ?

lib/sanitizer_common/sanitizer_common_interceptors.inc

4449

You need to set SANITIZER_INTERCEPT_PTHREAD_ATTR_GET_SCHED in sanitizer_platform_interceptors.h

[krytarowski]

lib/sanitizer_common/sanitizer_common_interceptors.inc

4449

This part is still missing.

[devnexen]

lib/sanitizer_common/sanitizer_common_interceptors.inc

4449

Yes I forgot to push my local changes ...

[krytarowski]

The title and description does not match the diff.

[devnexen]

In D44035#1026512, @krytarowski wrote:

The title and description does not match the diff.

You re right, I always have been bad to describe ...

[krytarowski]

--------------------------
|Index: lib/sanitizer_common/sanitizer_common_interceptors.inc
|===================================================================
|--- lib/sanitizer_common/sanitizer_common_interceptors.inc
|+++ lib/sanitizer_common/sanitizer_common_interceptors.inc
--------------------------
Patching file lib/sanitizer_common/sanitizer_common_interceptors.inc using Plan A...
Hunk #1 failed at 3590.
Hunk #2 succeeded at 4407.
Hunk #3 succeeded at 4423.
Hunk #4 succeeded at 4455.
Hunk #5 succeeded at 7174.
1 out of 5 hunks failed--saving rejects to lib/sanitizer_common/sanitizer_common_interceptors.inc.rej
done

$ cat  lib/sanitizer_common/sanitizer_common_interceptors.inc.rej
@@ -3590,7 +3590,7 @@
 //  * GNU version returns message pointer, which points to either buf or some
 //    static storage.
 #if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE) || \
-    SANITIZER_MAC || SANITIZER_ANDROID || SANITIZER_NETBSD
+    SANITIZER_MAC || SANITIZER_ANDROID || SANITIZER_NETBSD || SANITIZER_OPENBSD
 // POSIX version. Spec is not clear on whether buf is NULL-terminated.
 // At least on OSX, buf contents are valid even when the call fails.
 INTERCEPTOR(int, strerror_r, int errnum, char *buf, SIZE_T buflen) {

Please fix it.

[krytarowski]

lib/sanitizer_common/sanitizer_platform_interceptors.h

468 ↗

(On Diff #136912)

This is wrong.

GETGROUPLIST covers getgroupmembership(3) that is unavailable on OpenBSD. Please split this interceptor.

[devnexen]

lib/sanitizer_common/sanitizer_platform_interceptors.h

468 ↗

(On Diff #136912)

Fair point

[krytarowski]

Please improve the description. List supported interceptors and changes in existing interceptors.

[krytarowski]

lib/sanitizer_common/sanitizer_platform_interceptors.h

469 ↗

(On Diff #136914)

OpenBSD here is wrong.

[krytarowski]

Reverted, broken syntax.

[krytarowski]

Please don't submit untested patches, they should be at least build-tested.

[devnexen]

In D44035#1026574, @krytarowski wrote:

Please don't submit untested patches, they should be at least build-tested.

I might have broken something on non OpenBSD systems because I had tested (and retested just now). What were the compilations errors ?

[krytarowski]

[63/93] Building CXX object projects/compiler-rt/lib/esan/CMakeFiles/clang_rt.esan-x86_64.dir/esan_interceptors.cpp.o
FAILED: projects/compiler-rt/lib/esan/CMakeFiles/clang_rt.esan-x86_64.dir/esan_interceptors.cpp.o 
/usr/bin/c++   -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iprojects/compiler-rt/lib/esan -I/b/sanitizer-x86_64-linux-fuzzer/build/llvm/projects/compiler-rt/lib/esan -Iinclude -I/b/sanitizer-x86_64-linux-fuzzer/build/llvm/include -I/b/sanitizer-x86_64-linux-fuzzer/build/llvm/projects/compiler-rt/lib/esan/.. -fPIC -fvisibility-inlines-hidden -Werror=date-time -std=c++11 -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment -ffunction-sections -fdata-sections -Wall -std=c++11 -Wno-unused-parameter -O3 -DNDEBUG    -m64 -fno-lto -fPIC -fno-builtin -fno-exceptions -fomit-frame-pointer -funwind-tables -fno-stack-protector -fvisibility=hidden -fno-lto -O3 -g -Wno-variadic-macros -Wno-non-virtual-dtor -fno-rtti -MD -MT projects/compiler-rt/lib/esan/CMakeFiles/clang_rt.esan-x86_64.dir/esan_interceptors.cpp.o -MF projects/compiler-rt/lib/esan/CMakeFiles/clang_rt.esan-x86_64.dir/esan_interceptors.cpp.o.d -o projects/compiler-rt/lib/esan/CMakeFiles/clang_rt.esan-x86_64.dir/esan_interceptors.cpp.o -c /b/sanitizer-x86_64-linux-fuzzer/build/llvm/projects/compiler-rt/lib/esan/esan_interceptors.cpp
In file included from /b/sanitizer-x86_64-linux-fuzzer/build/llvm/projects/compiler-rt/lib/esan/esan_interceptors.cpp:178:0:
/b/sanitizer-x86_64-linux-fuzzer/build/llvm/projects/compiler-rt/lib/esan/../sanitizer_common/sanitizer_common_interceptors.inc:4445:0: warning: "INIT_PTHREAD_ATTR_GET_SCHED" redefined
 #define INIT_PTHREAD_ATTR_GET_SCHED
 
/b/sanitizer-x86_64-linux-fuzzer/build/llvm/projects/compiler-rt/lib/esan/../sanitizer_common/sanitizer_common_interceptors.inc:4441:0: note: this is the location of the previous definition
 #define INIT_PTHREAD_ATTR_GET_SCHED                       \
 
/b/sanitizer-x86_64-linux-fuzzer/build/llvm/projects/compiler-rt/lib/esan/../sanitizer_common/sanitizer_common_interceptors.inc:4460:2: error: #endif without #if
 #endif
  ^~~~~
[64/93] Building CXX object projects/compiler-rt/lib/asan/CMakeFiles/RTAsan_dynamic.x86_64.dir/asan_fake_stack.cc.o

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fuzzer/builds/12946/steps/build%20clang/logs/stdio

[devnexen]

lib/sanitizer_common/sanitizer_common_interceptors.inc

4455

Seems to be the guilty line ... Sorry.

[krytarowski]

lib/sanitizer_common/sanitizer_common_interceptors.inc

4455

After fixing this line, it's still broken.

[devnexen]

lib/sanitizer_common/sanitizer_common_interceptors.inc

4455

What is the error ? I can t try in another system at the moment.

[krytarowski]

lib/sanitizer_common/sanitizer_common_interceptors.inc

4455

I've pasted it on top and included a hot fix for this line previously.

[devnexen]

lib/sanitizer_common/sanitizer_common_interceptors.inc

4455

Bizarre you still get the same error than before ...

[krytarowski]

Another failure example:

http://lab.llvm.org:8011/builders/sanitizer-ppc64be-linux/builds/5853

[devnexen]

In D44035#1026598, @krytarowski wrote:

Another failure example:

http://lab.llvm.org:8011/builders/sanitizer-ppc64be-linux/builds/5853

This is helpful I missed the big picture. thx

[devnexen]

Hopefully will fix on non OpenBSD systems the build failures.

[devnexen]

ping (maybe not @krytarowski)

[devnexen]

ping @vitalybuka

[vitalybuka]

Can't apply the patch.
Can you please rebase it?

[vitalybuka]

Should this be abandoned D44035?
It looks the same as D44036

[devnexen]

Oh yes good point ...

lib/sanitizer_common/sanitizer_common_interceptors.inc

 
 #if SANITIZER_INTERCEPT_STRERROR_R
 // There are 2 versions of strerror_r:
 //  * POSIX version returns 0 on success, negative error code on failure,
 //    writes message to buf.
 //  * GNU version returns message pointer, which points to either buf or some
 //    static storage.
 #if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE) || \
-    SANITIZER_MAC || SANITIZER_ANDROID || SANITIZER_NETBSD
+    SANITIZER_MAC || SANITIZER_ANDROID || SANITIZER_NETBSD || SANITIZER_OPENBSD
 // POSIX version. Spec is not clear on whether buf is NULL-terminated.
 // At least on OSX, buf contents are valid even when the call fails.
 INTERCEPTOR(int, strerror_r, int errnum, char *buf, SIZE_T buflen) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, strerror_r, errnum, buf, buflen);
   // FIXME: under ASan the call below may write to freed memory and corrupt
   // its metadata. See
   // https://github.com/google/sanitizers/issues/321.
#define INTERCEPTOR_PTHREAD_RWLOCKATTR_GET(what, sz) \
   INTERCEPTOR_PTHREAD_OBJECT_ATTR_GET(pthread_rwlockattr_get##what, sz)
 #define INTERCEPTOR_PTHREAD_CONDATTR_GET(what, sz) \
   INTERCEPTOR_PTHREAD_OBJECT_ATTR_GET(pthread_condattr_get##what, sz)
 #define INTERCEPTOR_PTHREAD_BARRIERATTR_GET(what, sz) \
   INTERCEPTOR_PTHREAD_OBJECT_ATTR_GET(pthread_barrierattr_get##what, sz)
 #endif
 
 #if SANITIZER_INTERCEPT_PTHREAD_ATTR_GET
-INTERCEPTOR_PTHREAD_ATTR_GET(detachstate, sizeof(int))
-INTERCEPTOR_PTHREAD_ATTR_GET(guardsize, sizeof(SIZE_T))
-INTERCEPTOR_PTHREAD_ATTR_GET(schedparam, struct_sched_param_sz)
-INTERCEPTOR_PTHREAD_ATTR_GET(schedpolicy, sizeof(int))
-INTERCEPTOR_PTHREAD_ATTR_GET(scope, sizeof(int))
-INTERCEPTOR_PTHREAD_ATTR_GET(stacksize, sizeof(SIZE_T))
-INTERCEPTOR(int, pthread_attr_getstack, void *attr, void **addr, SIZE_T *size) {
-  void *ctx;
-  COMMON_INTERCEPTOR_ENTER(ctx, pthread_attr_getstack, attr, addr, size);
-  // FIXME: under ASan the call below may write to freed memory and corrupt
-  // its metadata. See
-  // https://github.com/google/sanitizers/issues/321.
-  int res = REAL(pthread_attr_getstack)(attr, addr, size);
-  if (!res) {
-    if (addr) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, addr, sizeof(*addr));
-    if (size) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, size, sizeof(*size));
-  }
-  return res;
-}
+#define COMMON_INTERCEPTOR_PTHREAD_ATTR_GET                                       \
+INTERCEPTOR_PTHREAD_ATTR_GET(detachstate, sizeof(int))                            \
+INTERCEPTOR_PTHREAD_ATTR_GET(guardsize, sizeof(SIZE_T))                           \
+INTERCEPTOR_PTHREAD_ATTR_GET(scope, sizeof(int))                                  \
+INTERCEPTOR_PTHREAD_ATTR_GET(stacksize, sizeof(SIZE_T))                           \
+INTERCEPTOR(int, pthread_attr_getstack, void *attr, void **addr, SIZE_T *size) {  \
+  void *ctx;                                                                      \
+  COMMON_INTERCEPTOR_ENTER(ctx, pthread_attr_getstack, attr, addr, size);         \
+  // FIXME: under ASan the call below may write to freed memory and corrupt       \
+  // its metadata. See                                                            \
+  // https://github.com/google/sanitizers/issues/321.                             \
+  int res = REAL(pthread_attr_getstack)(attr, addr, size);                        \
+  if (!res) {                                                                     \
+    if (addr) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, addr, sizeof(*addr));           \
+    if (size) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, size, sizeof(*size));           \
+  }                                                                               \
+  return res;                                                                     \
+}                                                                             
 
+#if !SANITIZER_OPENBSD
+#define SANITIZER_INTERCEPTOR_PTHREAD_ATTR_GET                  \
+COMMON_INTERCEPTOR_PTHREAD_ATTR_GET                             \
+INTERCEPTOR_PTHREAD_ATTR_GET(schedparam, struct_sched_param_sz) \
+INTERCEPTOR_PTHREAD_ATTR_GET(schedpolicy, sizeof(int))          
+#else
+#define SANITIZER_INTERCEPTOR_PTHREAD_ATTR_GET                  \
+COMMON_INTERCEPTOR_PTHREAD_ATTR_GET
+#endif
+
+SANITIZER_INTERCEPTOR_PTHREAD_ATTR_GET
+
 // We may need to call the real pthread_attr_getstack from the run-time
 // in sanitizer_common, but we don't want to include the interception headers
 // there. So, just define this function here.
 namespace __sanitizer {
 extern "C" {
 int real_pthread_attr_getstack(void *attr, void **addr, SIZE_T *size) {
   return REAL(pthread_attr_getstack)(attr, addr, size);
 }
 }  // extern "C"
 }  // namespace __sanitizer
 
-#define INIT_PTHREAD_ATTR_GET                             \
+#define COMMON_INIT_PTHREAD_ATTR_GET                      \
   COMMON_INTERCEPT_FUNCTION(pthread_attr_getdetachstate); \
   COMMON_INTERCEPT_FUNCTION(pthread_attr_getguardsize);   \
-  COMMON_INTERCEPT_FUNCTION(pthread_attr_getschedparam);  \
-  COMMON_INTERCEPT_FUNCTION(pthread_attr_getschedpolicy); \
   COMMON_INTERCEPT_FUNCTION(pthread_attr_getscope);       \
   COMMON_INTERCEPT_FUNCTION(pthread_attr_getstacksize);   \
   COMMON_INTERCEPT_FUNCTION(pthread_attr_getstack);
+
+#if !SANITIZER_OPENBSD
+#define INIT_PTHREAD_ATTR_GET                             \
+  COMMON_INIT_PTHREAD_ATTR_GET                            \
+  COMMON_INTERCEPT_FUNCTION(pthread_attr_getschedparam);  \
+  COMMON_INTERCEPT_FUNCTION(pthread_attr_getschedpolicy); \
+#else
+#define INIT_PTHREAD_ATTR_GET  COMMON_INIT_PTHREAD_ATTR_GET
+#endif
 #else
 #define INIT_PTHREAD_ATTR_GET
 #endif
 
 #if SANITIZER_INTERCEPT_PTHREAD_ATTR_GETINHERITSCHED
 INTERCEPTOR_PTHREAD_ATTR_GET(inheritsched, sizeof(int))
 
 #define INIT_PTHREAD_ATTR_GETINHERITSCHED \