-
Notifications
You must be signed in to change notification settings - Fork 36.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci: Revert tsan task changes #26775
The head ref may contain hidden characters: "2212-tsan-revert-\u{1F3A8}"
ci: Revert tsan task changes #26775
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK fabb6af, I have reviewed the code and it looks OK, I agree it can be merged.
Can we revert just #26759? Is there an issue with using Clang-15 with the suppression back in place? Going forward, we should probably have better rationale for removing (already marked intermittent) suppresions, than "it works on my machine" and/or assuming it was magically fixed by some toolchain backport (which couldn't have been the case here anyway). We should also figure out what is different about the Cirrus env; as the suppressed issue re-appeared immediately after the merge of #26759. |
There are two unrelated issues:
It doesn't help that both are intermittent and the false positive is not reproducible so far. (I tried with a |
As the issues are intermittent, my best guess would be they are easier to reproduce on cloud providers with 3rd-party load on neighbouring CPUs or neighbouring threads on the same CPU. |
There is also a third (unrelated?) issue:
|
fabb6af ci: Remove duplicate CC and CXX from tsan task (MarcoFalke) fa5d9a0 Revert "ci: Use clang-15 in tsan task" (MarcoFalke) faa835e Revert "test: Drop no longer needed `race:epoll_ctl` TSan suppression" (MarcoFalke) Pull request description: Looks like there are still bugs in clang-15, so we need to roll back all the way to the previously used version (clang-13). ACKs for top commit: hebasto: ACK fabb6af, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: d62203049847ab9095ee3fc89e18bdd721d1d9d5a7ef7a9f524c80e6be58d1d9f6aa2f14533df1ea77eb59597fba6fa9b987b17eb03b2c3f7cb577ab59cd59c0
I made some progress reproducing on a fresh install of Ubuntu Jammy:
(Then figure out whether to |
See llvm/llvm-project@9fb8058 and the previous commit llvm/llvm-project@b332134 |
faf4aca ci: Use TSan new runtime (llvm-16, take 3) (MarcoFalke) Pull request description: The previous two attempts failed: * llvm-14: Failed in bitcoin/bitcoin#24572 * llvm-15: Failed in bitcoin/bitcoin#26775 However, now that the bug is known and fixed, it should be good to go. See also bitcoin/bitcoin#26775 (comment) ACKs for top commit: fanquake: ACK faf4aca - I still see [this](bitcoin/bitcoin#27298 (comment)) failure on aarch64, but that isn't really a regression, as running this tests was already broken for me. I'll open a separate issue, and we can follow up. Tree-SHA512: 372b53c4d42ca7f527dae4a2b5bc5ab33c816930daf7a3479d20ea7749159a0b19cfd8d76244b95b03130e4a3d12ddbbb74668b8f7e9fc272cf1084f53b7ff9b
faf4aca ci: Use TSan new runtime (llvm-16, take 3) (MarcoFalke) Pull request description: The previous two attempts failed: * llvm-14: Failed in bitcoin#24572 * llvm-15: Failed in bitcoin#26775 However, now that the bug is known and fixed, it should be good to go. See also bitcoin#26775 (comment) ACKs for top commit: fanquake: ACK faf4aca - I still see [this](bitcoin#27298 (comment)) failure on aarch64, but that isn't really a regression, as running this tests was already broken for me. I'll open a separate issue, and we can follow up. Tree-SHA512: 372b53c4d42ca7f527dae4a2b5bc5ab33c816930daf7a3479d20ea7749159a0b19cfd8d76244b95b03130e4a3d12ddbbb74668b8f7e9fc272cf1084f53b7ff9b
Looks like there are still bugs in clang-15, so we need to roll back all the way to the previously used version (clang-13).