- User Since
- Dec 4 2015, 11:34 AM (84 w, 6 d)
This had to be reverted due to a masking error in its dependency which caused failures on 10.11 buildbots. Since it got reverted anyway, I took the opportunity to look into some false positives in swift, and found that they were caused by this patch. In order to prevent false positives, this now uses a blacklist of sections which definitely do not store pointers instead of a whitelist of sections which do (to avoid missing sections). Would appreciate a re-review.
Use blacklist instead of whitelist to avoid false positives
Dependency was reverted.
Remove accidental arcanist squash
Use blacklist instead of whitelist
Wed, Jul 19
Fix 10.11 buildbots by masking dyld section addresses
Tue, Jul 18
Interesting to me that this would cause timeouts, but yeah, I'll revert and look into it.
I'm running the swift stdlib test suite with LSan, and it complains about the dyld insertion:
Still working on swift, but looks like bootstrapped llvm-tblgen has a few leaks. They don't show up on Linux, but they also don't really look like false positives:
Mon, Jul 17
should do the trick.
re check-llvm, turns out it was a KP with libedit, re-running with that disabled using LLVM_USE_SANITIZER instead of manual cflags
I spoke too soon, looks like check-clang is clean but check-llvm has a few leaks. I'll look into whether those seem like false positives.
I ran bootstrapped check-clang with -fsanitize=address and this patch, all tests passed. (Verified that it was running LSan as well by making sure tests failed with a CHECK(0) in DoLeakCheck()).
sec_names -> kGlobalVarSecNames
Fri, Jul 14
Please wait on enabling this on-by-default until we perform some more testing on our side.
Merged into D35422
Address comments and remove MemoryMappedSection class
Fix backwards conditional
Fix a typo
Wed, Jul 12
Looking further into it, _exit family functions don't get called on linux when return from main is used (GIexit in libc calls the exit syscall directly), so interception won't work. TSan and LSan get around this by calling exit() from their atexit() handlers, but we've established that we don't want to do this because of dlclose() issues. I think we're stuck with the current behavior, at least on Linux.
Hmmm, it actually appears that even TSan doesn't successfully intercept _exit on Linux, only on Darwin (a CHECK(0) statement in OnExit() is never reached on Linux). If we can't successfully intercept _exit, and we can't exit from atexit, I'm not sure that there's a good way to do this.
Only adding this for non-standalone builds makes testing hard, since you don't have consistent behavior between standalone and non-standalone ubsan builds. I'm debating leaving UBSan be, and just fixing this for the other sanitizers (ie making sure ASan returns failure when halt_on_error=0).
I'll put up the partial solution where we respect exitcode in all sanitizers that already use interceptors when halt_on_error=0. I won't add interception to UBSan. It's unfortunate for me personally, since my codebases only use standalone UBSan, but I can understand why we wouldn't want to add interception to UBSan just for this one feature.
Tue, Jul 11
There's a further complication with intercepting in UBSan - there doesn't seem to be a way to tell whether you're building standalone at compile time (although I could add a cmake define for that), so the UBSan interceptor will conflict with the TSan/ASan interceptor.
The only issue I have come across is that standalone UBSan currently doesn't link against interception. If we want to intercept _exit, we'll need to add interception as a dependency. Is that something we're okay with?
Mon, Jul 10
It might be possible to change the default exitcode for UBSan, but I still think it should be possible for the user to make UBSan errors to return failure. Two primary reasons:
- Consistency: As far as I'm aware, all of the other sanitizers return a failing exit code when they detect an error in a program, and I'm not sure why UBSan should be different in that respect.
- Utility: A common use case I could imagine would be the case where a user wants to prevent UB regressions in their codebase by running a unit test suite with UBSan enabled. If it's not possible to return a failing exit code (either by default or otherwise), the test suite will continue passing just fine when UB is introduced.
Yes, I'm going to look into handling this via memory region scanning, but it's possible (even likely) that the performance hit will be prohibitive.
Yeah, I think that makes sense. Life will get a bit tricky when Windows support gets added and we can't use weak hooks to determine whether LSan is running, but I'm sure there's a way to handle that.
Sun, Jul 9
Fri, Jul 7
Thu, Jul 6
Wed, Jun 28
This doesn't provide as much of a performance benefit as I expected. In addition, the fact that a leak comes from a system library doesn't make it less of a leak. It's not a great user experience to see system library leaks in your program output, but I don't think there is much we can/should do about it.
An alternative solution here would be to add a supression for dispatch_once instead, but that would be less configurable (unless we required the user to provide it) and slightly less performant.
Tue, Jun 27
Add back cfi comment
As far as I'm aware, the feature exists both to save time building and testing sanitizers you don't need, and to get around differing build requirements across the sanitizers (it's possible, especially when cross-compiling, that some sanitizers build in a given configuration and others do not).
Test cfi even when cfi runtime isn't built
Broke cfi in cases when the cfi runtime isn't built.