Page MenuHomePhabricator

Support OpenBSD in common interceptors
AbandonedPublic

Authored by devnexen on Mar 2 2018, 11:25 AM.

Details

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

Diff Detail

Event Timeline

devnexen created this revision.Mar 2 2018, 11:25 AM
Herald added subscribers: Restricted Project, llvm-commits, mgorny and 2 others. · View Herald TranscriptMar 2 2018, 11:25 AM
vitalybuka accepted this revision.Mar 2 2018, 2:22 PM

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
4446

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?

This revision is now accepted and ready to land.Mar 2 2018, 2:22 PM
vitalybuka requested changes to this revision.Mar 2 2018, 2:22 PM
This revision now requires changes to proceed.Mar 2 2018, 2:22 PM
devnexen added inline comments.Mar 2 2018, 2:27 PM
lib/sanitizer_common/CMakeLists.txt
19 ↗(On Diff #136816)

Ok makes sense.

lib/sanitizer_common/sanitizer_common_interceptors.inc
4446

Yes.

devnexen retitled this revision from OpenBSD UBsan support / common part 2 to OpenBSD pthread interceptor subset.Mar 2 2018, 2:28 PM
devnexen updated this revision to Diff 136856.Mar 2 2018, 2:33 PM
vitalybuka requested changes to this revision.Mar 2 2018, 2:57 PM
vitalybuka added inline comments.
lib/sanitizer_common/sanitizer_common_interceptors.inc
4446

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

This revision now requires changes to proceed.Mar 2 2018, 2:57 PM
devnexen updated this revision to Diff 136868.Mar 2 2018, 3:34 PM
vitalybuka accepted this revision.Mar 2 2018, 3:55 PM

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

lib/sanitizer_common/sanitizer_common_interceptors.inc
4436

You need to set SANITIZER_INTERCEPT_PTHREAD_ATTR_GET_SCHED in sanitizer_platform_interceptors.h

This revision is now accepted and ready to land.Mar 2 2018, 3:55 PM
devnexen edited the summary of this revision. (Show Details)Mar 2 2018, 3:58 PM
krytarowski added inline comments.Mar 3 2018, 3:57 AM
lib/sanitizer_common/sanitizer_common_interceptors.inc
4436

This part is still missing.

devnexen added inline comments.Mar 3 2018, 4:02 AM
lib/sanitizer_common/sanitizer_common_interceptors.inc
4436

Yes I forgot to push my local changes ...

devnexen updated this revision to Diff 136907.Mar 3 2018, 4:05 AM

The title and description does not match the diff.

The title and description does not match the diff.

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

devnexen retitled this revision from OpenBSD pthread interceptor subset to OpenBSD UBsan support, having common INIT_PTHREAD_ATTR_GET and for non OpenBSD systems INIT_PTHREAD_ATTR_GET_SCHED macros.Mar 3 2018, 4:12 AM
krytarowski retitled this revision from OpenBSD UBsan support, having common INIT_PTHREAD_ATTR_GET and for non OpenBSD systems INIT_PTHREAD_ATTR_GET_SCHED macros to Support OpenBSD in common interceptors.Mar 3 2018, 4:17 AM
krytarowski edited the summary of this revision. (Show Details)
--------------------------
|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.

devnexen updated this revision to Diff 136912.Mar 3 2018, 4:32 AM
krytarowski added inline comments.Mar 3 2018, 4:36 AM
lib/sanitizer_common/sanitizer_platform_interceptors.h
468

This is wrong.

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

devnexen added inline comments.Mar 3 2018, 4:47 AM
lib/sanitizer_common/sanitizer_platform_interceptors.h
468

Fair point

devnexen updated this revision to Diff 136914.Mar 3 2018, 4:57 AM

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

krytarowski added inline comments.Mar 3 2018, 5:19 AM
lib/sanitizer_common/sanitizer_platform_interceptors.h
469

OpenBSD here is wrong.

devnexen edited the summary of this revision. (Show Details)Mar 3 2018, 5:19 AM
devnexen updated this revision to Diff 136916.Mar 3 2018, 5:25 AM
Closed by commit rCRT326653: Support OpenBSD in common interceptors (authored by kamil, committed by ). · Explain WhyMar 3 2018, 6:16 AM
This revision was automatically updated to reflect the committed changes.

Reverted, broken syntax.

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

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 ?

[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 added inline comments.Mar 3 2018, 7:08 AM
lib/sanitizer_common/sanitizer_common_interceptors.inc
4442

Seems to be the guilty line ... Sorry.

krytarowski reopened this revision.Mar 3 2018, 7:08 AM
This revision is now accepted and ready to land.Mar 3 2018, 7:08 AM
krytarowski requested changes to this revision.Mar 3 2018, 7:08 AM
This revision now requires changes to proceed.Mar 3 2018, 7:08 AM
krytarowski added inline comments.Mar 3 2018, 7:09 AM
lib/sanitizer_common/sanitizer_common_interceptors.inc
4442

After fixing this line, it's still broken.

devnexen added inline comments.Mar 3 2018, 7:10 AM
lib/sanitizer_common/sanitizer_common_interceptors.inc
4442

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

krytarowski added inline comments.Mar 3 2018, 7:12 AM
lib/sanitizer_common/sanitizer_common_interceptors.inc
4442

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

devnexen added inline comments.Mar 3 2018, 7:15 AM
lib/sanitizer_common/sanitizer_common_interceptors.inc
4442

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

devnexen updated this revision to Diff 136921.Mar 3 2018, 7:18 AM

This is helpful I missed the big picture. thx

devnexen updated this revision to Diff 136923.Mar 3 2018, 7:45 AM

Hopefully will fix on non OpenBSD systems the build failures.

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

devnexen updated this revision to Diff 137832.Mar 9 2018, 1:46 PM
vitalybuka accepted this revision.Mar 9 2018, 2:09 PM

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

devnexen abandoned this revision.Mar 15 2018, 3:35 PM

Oh yes good point ...