[rtsan] Add error message when dlopened #14
Conversation
|
@davidtrevelyan I need your thoughts on this - if you agree that we should submit it I am going to break it into at least 2 reviews (possibly 3) and do upstream PRs |
There was a problem hiding this comment.
This is excellent! Thank you so much for putting this together @cjappl. Two very minor comments about test debug output, and one more interesting comment about test coverage for instrumented binaries.
Great work man!
|
|
||
| // Launching a non-instrumented binary that dlopen's an instrumented library should fail. | ||
| // RUN: not %run %t %t.so 2>&1 | FileCheck %s --check-prefix=CHECK-FAIL | ||
| // Launching a non-instrumented binary with an explicit DYLD_INSERT_LIBRARIES should work. |
There was a problem hiding this comment.
Would it also be helpful to have a test for an instrumented binary as well?
There was a problem hiding this comment.
I'm going to try to put a test for this in the final version - I tested it locally and it "just works" like normal. I assume just one of the copies of rtsan is chosen over the other one
| #else // defined(SHARED_LIB) | ||
| int main(int argc, char *argv[]) { | ||
| void *handle = dlopen(argv[1], RTLD_NOW); | ||
| fprintf(stderr, "handle = %p\n", handle); |
There was a problem hiding this comment.
Minor: I see this is probably helpful for debugging, but it looks like the test doesn't need this print out. Just flagging in case you meant to remove it
| void *handle = dlopen(argv[1], RTLD_NOW); | ||
| fprintf(stderr, "handle = %p\n", handle); | ||
| void (*foo)() = (void (*)())dlsym(handle, "foo"); | ||
| fprintf(stderr, "foo = %p\n", foo); |
There was a problem hiding this comment.
Minor: Likewise, I think this isn't needed for the test logic - flagging in case you want to delete it
Ok - a lot going on here.
Top level goal
This is all to get the following error to be printed when a plug-in is loaded in a DAW without the interceptors being enabled:
This will (ideally) lead people to track down this issue faster.
Problem description
A DAW loading a plug-in is just a host process loading a shared library with the
dlopencall. So this pertains to ALL other shared libraries that get loaded in this method.If you build a shared library instrumented with rtsan, and that library is opened with
dlopenand the interceptors are not working - we would ideally like to have a nice error message and abort instead of just silently carrying on. This is what At Salot's issue was in the discord - just silent failure loading his rtsan-enabled plugin in a DAW with no indication of what is going wrong.The other sanitizers (namely ASan and TSan) have this error and emit it under the same circumstance, you can see evidence of it here for ASan.
This entire fix is to put in
InitializePlatformEarly()into__rtsan_init()and make sure it is called in every binary at least once.WTF does this have to do with clang and LLVM?
OK - we have previously believed that on Mac, the first intercepted function called will always enable RTSan. This is absolutely correct, but what if the interceptors are not installed, as in the case of a shared library?
We need to install into our shared library the initializer in the global namespace. This means creating another pass, a Module pass that calls
__rtsan_ensure_initialized()in the shared library.When this shared library is instrumented, this call is inserted.
When the shared library is opened with
dlopen-__rtsan_ensure_initialized()will be calledWhen that is called, it will call
InitializePlatformEarlyand abort the process if the interceptors are not working.These steps were directly copped from TSan and Asan, if you look in the LLVM pass for threadsanitizer, you will see this code exactly. Almost all tests and code have been taken and translated from the TSan version of this same thing.
Other ways to solve this
Other details
RTSAN_OPTIONS="verify_interceptors=0"