Make tests fail if ASan finds an errors #10005
Conversation
The AddressSanitizer also loads the LeakSanitizer flags and even though the documentation suggests that exitcode can be set per sanitizer, this doesn't appear to be the case and our tests exit with code 0 after the AddressSanitizer found a problem. After this change, around 100 tests will fail due to several issues.
|
Starting build on |
|
I find this surprising; it used to affect only lsan when this was implemented. I trust that you verified this, though. The only other thing I can say is that maybe now it's a good idea to verify whether it's possible to switch on leak sanitizer. With llvm 5, this just wasn't possible, and JITted functions also created problems because their memory management escaped asan. That's why the exitcode was hardcoded to 0. Maybe you can switch it on once, and see how much explodes. |
|
@phsft-bot build only on ROOT-ubuntu2004-clang with flags -Dasan=ON |
|
Starting build on |
|
@phsft-bot build just on ROOT-ubuntu2004-clang with flags -Dasan=ON |
Yes, the relevant code is https://github.com/llvm/llvm-project/blob/7f99e1870f776a25e03526e3190105c094750d98/compiler-rt/lib/asan/asan_flags.cpp#L113-L116 which AFAICT has been the case basically since it was introduced by llvm/llvm-project@1b73bde. Now it's possible that earlier toolchains on macOS did not have
That's another thing that deviates from the documentation: LSan is supposed to be on-by-default with ASan on Linux x86, but it's not for us... |
It's off intentionally because of what I wrote previously. root/core/sanitizer/SanitizerSetup.cxx Lines 19 to 23 in 498ac1d |
|
The matrix whitelist doesn't allow selecting the node that's running asan in the nightlies. Maybe ubuntu19 ist still there? |
|
@phsft-bot build just on ROOT-ubuntu1904-clang with flags -Dasan=ON |
Ah yes, I only checked Regarding the test failures, I'm currently going through them one-by-one to create issues on GitHub. |
|
Here's the culprit: |
Sigh. That's even older than the commit linked above, but the documentation for LSan flags hasn't been updated since... |
|
@phsft-bot build just on ROOT-ubuntu2004-clang with flags -Dasan=ON |
|
@phsft-bot build just on ROOT-ubuntu2004-clang/default with flags -Dasan=ON |
|
Starting build on |
Yes, as expected many tests are failing now, so we have to decide on a strategy here: Do we want to fix all issues first or merge this one first to make it easier to debug things? I'm definitely in favor of the latter, it's a waste of time to check the verbose output of 30+ tests by hand because they're not failing if the Address Sanitizer found something... |
|
We need to have everyone aboard for this, to fix it within two days not two months. We will discuss on Monday during the team meeting. |
I'd merge it as well. The build will be red for weeks or months, but the shifter should have a look and ping people every once in a while. |
|
@phsft-bot build just on ROOT-ubuntu2004-clang/default with flags -Dasan=ON |
|
Starting build on |
The AddressSanitizer also loads the LeakSanitizer flags and even though the documentation suggests that
exitcodecan be set per sanitizer, this doesn't appear to be the case and our tests exit with code 0 after the AddressSanitizer found a problem. After this change, around 100 tests will fail due to several issues.