This also makes the sanitizer_stoptheworld_test cross-platform by using the STL, rather than pthread.
|490 ms||x64 debian > SanitizerCommon-Unit._/Sanitizer-x86_64-Test::StopTheWorld.SuspendThreadsAdvanced|
Script: -- /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/lib/sanitizer_common/tests/./Sanitizer-x86_64-Test --gtest_filter=StopTheWorld.SuspendThreadsAdvanced
Catch all SEH Exceptions like EXCEPTION_ACCESS_VIOLATION during execution of the callback in StopTheWorld and report them.
This makes the last failing test SegvInCallback pass on Windows 🎉
I reviewed this code, it looks reasonable to me, but I don't have a lot of time to do a careful review. In the future, we should identify a new Windows sanitizer code owner.
My reading of the comments suggests that we don't need to catch exceptions to make that test pass. On Linux, the suspender thread blocks most signals, which presumably interacts really badly with SIGSEGV: the suspender thread (or process? unclear) gets killed, but the user threads remain suspended. This ends up looking like a timeout, which is bad.
I think the test exists to ensure that crashes during StopTheWorld don't hang, not to show that the process recovers. Exiting the process with an error is fine. If that just happens naturally, I suggest disabling the SegvInCallback test on Windows.
Please update the summary, in particular remove:
So I couldn't test if this port actually works
Many other SanitizerUnitTests fail to compile (look in the attached log):
I assume you have resolve the issue in the summary?
This broke building on MinGW. When building in MinGW configurations, it's built with -nostdinc++ (to make sure that sanitizers don't end up depending on the C++ standard I presume?), which makes <algorithm> not be found.
Can you make the code not rely on that header?
I'll go ahead and revert this one soon to restore the mingw build (and i386 msvc build too).
This fails to build on i386 (where things build successfully before), as you'd have to use .Esp instead of .Rsp there.
This is the one thing that fails to build when leaving out <algorithm>.
Yes, now building does succeed for both i386 and x86_64 mingw. Thanks!
For me all sanitizer tests pass on X64 and I386.
Could someone test if the tests also pass on ARM and ARM64?
The sanitizers aren't buildable for Windows on ARM in general (iirc there's a bunch of code in the common code that only is supported for x86, for things like intercepting/hotpatching stuff), so there's no need for this to work there I think, but the stack pointer register name seems correct anyway.
Building does produce this new warning though:
../lib/sanitizer_common/sanitizer_stoptheworld_win.cpp:56:52: warning: stack frame size (1304) exceeds limit (570) in '__sanitizer::(anonymous namespace)::SuspendedThreadsListWindows::GetRegistersAndSP(unsigned long long, __sanitizer::InternalMmapVector<unsigned long long>*, unsigned long long*) const' [-Wframe-larger-than] PtraceRegistersStatus SuspendedThreadsListWindows::GetRegistersAndSP( ^ 1 warning generated.