-
Notifications
You must be signed in to change notification settings - Fork 120
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
SEGV when using Clang [12, 16] #637
Comments
@krzysztof-jusiak Also, are you moving development to https://github.com/qlibs/ut ? Or are they two separate things now? I was having trouble getting ut2 to work with runtime tests. |
@krzysztof-jusiak Have you had a chance to look at this? |
Thanks @pfeatherstone for pointing it out. I've looked but I haven't fixed it yet and got distracted (there is UB in the string_view name assignment to be fixed, not sure how it got introduced and why actions didn't catch it). BTW the issue doesn't happen with - |
Thank you for your response. I gave qlibs/ut a go but I had to do a lot of fighting in order to get things to compile and run tests at runtime. Maybe it's more user-friendly for compile-time tests whereas boost-ext/ut is more friendly towards runtime tests?? |
I see the issue with UT, it's uninitialized variable and string_view pointing to already dead string. static_assert(("sum"_test = [] { // compile-time only
expect(sum(1, 2, 3) == 6_i);
}));
int main() {
"sum"_test = [] { // compile time and run-time
expect(sum(1, 2, 3) == 5_i);
};
"sum"_test = [] constexpr { // compile-time and run-time
expect(sum(1, 2, 3) == 6_i);
};
"sum"_test = [] mutable { // run-time only
expect(sum(1, 2, 3) == 6_i);
};
"sum"_test = [] consteval { // compile-time only
expect(sum(1, 2, 3) == 6_i);
};
} |
Awesome I'll give qlibs an other go later. Otherwise, it would be great to get that bug fixed |
Hmm, the more I look at it the more I'm convincing myself it's actually a clang bug as the generated assembly seems wrong for static variables causing the memory being overwritten, but need to confirm that with smaller repro case. |
Do you think qlibs/ut2 would suffer from the same problem? |
no, it's related to how clang handles static storage, either way I'm going to fix/workaround it. qlibs/ut is much simpler in that regards so doesn't suffer from it. |
I have found other issues with ut2 when using clang qlibs/ut#3 |
I might remove Clang from my CI. Or i'm sorry to say, move back to doctest. As much as I love this library, it requires a very modern compiler. |
May consider moving to 1.1.9 version which is stable and works with previous clang versions (no junit reporter though) |
I think this might be a symptom of a larger issue that seems ODR related. In a library I'm working on, using UT across separate translation units causes segfaults with clang (16 thru 18, maybe others). However, when I combine all tests into a single TU, where the It has to be the full combination of single TU, manual instantiation, and manual run to eliminate the segfaults though. Neglect any one of those, and * boom *. While tracking down that combination to success, I also caused MSVC to experience the same segfault (but unfortunately I didn't note down the combination needed to produce that one), so I don't think this is clang-specific, it's just that clang is the least tolerant / most likely to blow up. |
Looking back through the CI and my git history on that branch, it looks like the combination for this was manual instantiation of |
Expected Behavior
I expect the library not to SIGSEGV fault
Actual Behavior
It SIGSEGV faults when compiling with Clang 12, 13, 14, 15, 16.
It looks to be ok with 17 and 18.
Steps to Reproduce the Problem
https://godbolt.org/z/avKr3dj6j
Specifications
The text was updated successfully, but these errors were encountered: