ci: build TSan with clang 15 and add -Werror=thread-safety, fix-up stacktraces #5375
Conversation
kwvg
force-pushed
the
tsafety
branch
6 times, most recently
from
cb0f27e to
0084613
Compare
kwvg
changed the title
src/stacktraces.cpp
Outdated
| #if __clang__ | ||
| #error not supported on WIN32 (no dlsym support) | ||
| #elif WIN32 | ||
| #if WIN32 |
There was a problem hiding this comment.
also, feels like these changes should be in another PR? This PR should just be adding clang-15 and thread-safety
There was a problem hiding this comment.
The fixes for stacktraces are directly aimed to resolve problems with a Clang/LLVM target, which didn't exist in our CI suite until this PR, so it was retained here. The changes weren't significant enough to warrant its own PR and aren't verifiable without this PR's merger (sans stracktrace changes)
There was a problem hiding this comment.
I get no useful info in debug.log on failed ASSERT_IF_DEBUGs anymore (on macos). 6424d78 broke it.
develop:
Assertion failure:
assertion: (pCycleQuorumBaseBlockIndex->nHeight % llmqParams.dkgInterval == 2)
file: utils.cpp, line: 68
function: GetHashModifier
this PR:
Posix Signal: Abort trap: 6
Reverting it https://gitlab.com/UdjinM6/dash/-/commit/aa051f96eb2053ea95f021d07c67b7995c6e518f helps but it breaks CI. Applying https://gitlab.com/UdjinM6/dash/-/commit/dfb27a9d6767921f59701b20d92834cb918d62a2 or https://gitlab.com/UdjinM6/dash/-/commit/a0fde69efe98a8c499900f8e3bd87634b33fd06d on top makes CI happy (and it still works on macos too).
|
Changes integrated in latest push! |
Clang's warnings are far noisier and warnings about deprecations and the like from our dependencies are out of our control (and with dependencies like Boost, clog up the build log to the point GitLab stops logging)
`#elifdef` is introduced since C++23 but we target at most, C++17 and since a lot of conditional logic relies on else statements that should only check for presence rather than evaluate its value, it's better to consistently use `#if defined(__MACRO__)` instead of `#ifdef __MACRO__` for checking platform flags until the time we bump to C++23
…ix-up stacktraces (dashpay#5375)" This reverts commit 4963660.
Description
Pull request was inspired by the need to debug lock problems when working on #5352.
As far as I'm aware, only macOS has
-Werror=thread-safetyas part of its defaultCXXFLAGSdespite the capability being present on Linux as well. This PR introduces thread safety checks for that into our thread sanitizer build.Additionally, since we're using Clang, something that on first glimpse, appears to be something that
stacktraces.cppisn't happy with, due to-Wl,-wrapbeing available only on GCC, that no longer seems to be the case, since the version of Clang with comes withfocal, itsllddoes have support for-wrap(see man page forlldonfocal).The current
stableversion of Clang/LLVM is 15, at the time of this pull request (see https://apt.llvm.org/) butfocalships with an older version, requiring us to use the official LLVM APT repository. I feel we should be testing with recent compilers alongside the ones shipped by LTS distributions.Certain bugs are only made apparent when testing on rolling release distros or distros that have faster update cycles, like Fedora (see #5295 for an illustration of that), which ship with more recent compilers. Until we overhaul our CI systems to test using those distros directly (our current infrastructure is centered around using a "development image" with an LTS distro as the base), this is the best we can do.
A similar pull request testing against the latest GCC stable will be welcome as that is currently outside the scope of this PR as the changes made were to make sure that builds were operating as expected on Clang/LLVM 15.