ci: enable AddressSanitizer #3026
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3026 +/- ##
==========================================
- Coverage 77.82% 77.71% -0.11%
==========================================
Files 1064 1056 -8
Lines 74408 73394 -1014
==========================================
- Hits 57906 57037 -869
+ Misses 16502 16357 -145 see 235 files with indirect coverage changes 📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
Saviq
force-pushed
the
cleanup-sanitizers
branch
from
5370a6e
to
32c2f13
Compare
Saviq
force-pushed
the
ci-enable-asan
branch
from
15d401c
to
5cd4c56
Compare
|
|
|
This, I imagine: https://github.com/google/sanitizers/wiki/AddressSanitizer#turning-off-instrumentation |
mattkae
force-pushed
the
ci-enable-asan
branch
3 times, most recently
from
bbdd1a4
to
4a5defa
Compare
|
|
@AlanGriffiths Requesting you as a reviewer here since I modified some C++ stuff in a way that you might be interested in |
|
Ah it's breaking again 🤔 |
FullProbeStack.* and InputPlatformProbe.*These are failing (surprisingly only on clang) because |
| #if defined(__SANITIZE_ADDRESS__) | ||
| (void)udev_environment; | ||
| (void)KilledByInvalidMemoryAccess; | ||
| #else |
There was a problem hiding this comment.
I wonder if silently passing the tests is the right pattern.
Having a list of tests excluded from running in sanitize builds seems better. (I think all "DeathTests" will be problematic - maybe just exclude them from the build?)
There was a problem hiding this comment.
Yeah I think so too. I wonder if I could disable them in a smarter way globally
There was a problem hiding this comment.
- Have a
gtest_filterlist to exclude from the run - Move them to a separate file (or files) and don't build them
There was a problem hiding this comment.
For prior art see:
Lines 76 to 90 in 605712f
|
Trying to figure out why the asan_clang build works locally but fails in Github actions right now |
|
Got the build error for clang asan: |
|
We get many spurious failures in the asan build due to some threading issues, which isn't great... |
If we identify threading issues in our code we should be fixing them |
| class CopyableWlArray | ||
| { | ||
| public: | ||
| explicit CopyableWlArray(wl_array* in_data) | ||
| : data_{copy(in_data)} | ||
| { | ||
| } | ||
|
|
||
| CopyableWlArray(CopyableWlArray const& other) | ||
| : CopyableWlArray{other.data_} | ||
| { | ||
| } | ||
|
|
||
| CopyableWlArray& operator=(CopyableWlArray other) | ||
| { | ||
| std::swap(data_, other.data_); | ||
| return *this; | ||
| } | ||
|
|
||
| ~CopyableWlArray() | ||
| { | ||
| wl_array_release(data_); | ||
| } | ||
|
|
||
| [[nodiscard]] wl_array* data() const | ||
| { | ||
| return data_; | ||
| } | ||
|
|
||
| private: | ||
| static wl_array* copy(wl_array* in_data) | ||
| { | ||
| auto data = new wl_array(); | ||
| wl_array_init(data); | ||
| wl_array_copy(data, in_data); | ||
| return data; | ||
| } | ||
|
|
||
| wl_array* data_; | ||
| }; |
There was a problem hiding this comment.
This fails to delete data_, but the whole thing can be further simplfied...
| class CopyableWlArray | |
| { | |
| public: | |
| explicit CopyableWlArray(wl_array* in_data) | |
| : data_{copy(in_data)} | |
| { | |
| } | |
| CopyableWlArray(CopyableWlArray const& other) | |
| : CopyableWlArray{other.data_} | |
| { | |
| } | |
| CopyableWlArray& operator=(CopyableWlArray other) | |
| { | |
| std::swap(data_, other.data_); | |
| return *this; | |
| } | |
| ~CopyableWlArray() | |
| { | |
| wl_array_release(data_); | |
| } | |
| [[nodiscard]] wl_array* data() const | |
| { | |
| return data_; | |
| } | |
| private: | |
| static wl_array* copy(wl_array* in_data) | |
| { | |
| auto data = new wl_array(); | |
| wl_array_init(data); | |
| wl_array_copy(data, in_data); | |
| return data; | |
| } | |
| wl_array* data_; | |
| }; | |
| class CopyableWlArray | |
| { | |
| public: | |
| explicit CopyableWlArray(wl_array const* in_data) | |
| { | |
| wl_array_init(&data_); | |
| wl_array_copy(&data_, const_cast<wl_array*>(in_data)); // wl_array_copy() doesn't mark source as const | |
| } | |
| CopyableWlArray(CopyableWlArray const& other) | |
| : CopyableWlArray{&other.data_} | |
| { | |
| } | |
| CopyableWlArray& operator=(CopyableWlArray other) | |
| { | |
| std::swap(data_, other.data_); | |
| return *this; | |
| } | |
| ~CopyableWlArray() | |
| { | |
| wl_array_release(&data_); | |
| } | |
| [[nodiscard]] wl_array* data() const | |
| { | |
| // Our generated `send_modifiers_map_event()` requires a non-const wl_array, so | |
| // this workaround may be the wrong thing | |
| return const_cast<wl_array*>(&data_); | |
| } | |
| private: | |
| wl_array data_; | |
| }; |
|
@mattkae I assume you are still investigating the ubuntu:asan_clang failure? |
Yup. Doesn't repro on my machine, testing some things out now |
|
Fixed & Cleaned up the commits. Just waiting on one last check 🤞 |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
No description provided.