Add address, undefined behaviour, and thread sanitizer CI configurations #972
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
msimberg
force-pushed
the
all-sanitizers
branch
from
fe31f80
to
70e2391
Compare
msimberg
force-pushed
the
all-sanitizers
branch
from
dc73d2e
to
325c0b0
Compare
msimberg
changed the title
|
I'm going to ignore memory sanitizer in this PR, because it requires recompiling everything with |
msimberg
force-pushed
the
all-sanitizers
branch
from
e2c1050
to
92e4d93
Compare
This was referenced Jan 22, 2024
msimberg
force-pushed
the
all-sanitizers
branch
6 times, most recently
from
1a4455d
to
0cc5f2e
Compare
msimberg
force-pushed
the
all-sanitizers
branch
from
0cc5f2e
to
aa83635
Compare
msimberg
force-pushed
the
all-sanitizers
branch
from
aa83635
to
fd2dfd9
Compare
|
cscs-ci run |
msimberg
force-pushed
the
all-sanitizers
branch
from
fd2dfd9
to
4bcf07a
Compare
|
cscs-ci run |
msimberg
force-pushed
the
all-sanitizers
branch
3 times, most recently
from
c5d5661
to
80822bb
Compare
|
cscs-ci run |
It appears that thread sanitizer is not safe to use with stacks that migrate between os threads. See https://groups.google.com/g/boost-developers-archive/c/xNNhBnfhANA and https://stackoverflow.com/questions/73358829/segfault-in-tsan-in-boost-ci-need-the-help-of-an-expert for a case reported in Boost asio/beast. Stacktraces in pika tend to look like the following when stealing is enabled: ``` )(pika::latch&), pika::util::detail::pack_c<unsigned long, 0ul>, std::reference_wrapper<pika::latch> > >::then_sender_type, std::allocator<int> >::ensure_started_sender_type::shared_state::set_predecessor_done (this=0x7b280000a640) at /home/mjs/src/pika/libs/pika/execution /include/pika/execution/algorithms/ensure_started.hpp:26 ``` Note, stealing of staged threads (thread descriptions) is not disabled, as they haven't allocated or used a stack yet.
…t with address sanitizer
msimberg
force-pushed
the
all-sanitizers
branch
from
80822bb
to
83abc5c
Compare
|
cscs-ci run |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds two new CI pipelines with sanitizers:
The existing sanitizer CI configurations are unchanged.
The address sanitizer setup seems to pass for all tests, with some suppressions.
detect_stack_use_after_returnseems problematic so it's currently disabled (#992). Additionally, thetask_overhead_reportseems to trigger very high memory usage with address sanitizer.detect_stack_use_after_returnmay be triggering similar problems in other tests. A potential use-after-scope has been suppressed for now inbind(#993).For thread sanitizer I've suppressed data races coming from various lockfree queues (#990), the MPSC queue (#989). I've also disabled stealing of pending threads on the
local_*schedulers as thread sanitizer seems to dislike stacks migrating across OS threads (see 6a5b074 for more details).In the interest of getting what works into main I'm going to go ahead with the current state of this PR without making the two new CI configurations required checks. The two already existing sanitizer CI configurations remain required checks. I'm going to continue working on the remaining failures in follow-up PRs.