Page MenuHomePhabricator

Remove KillTheDoctor
Needs ReviewPublic

Authored by Meinersbur on Jun 13 2020, 4:53 PM.

Details

Summary

Remove the program "KillTheDoctor". It implemented a workaround for stopping "Dr. Watson" (Name of the error reporting tool in older version of Windows) from launching on regression tests. Win32 has an API for disabling error reporting which is already implemented in sys::DisableSystemDialogsOnCrash(), gtest and relevant test cases such as compiler-rt/test/asan/TestCases/ill.cpp.

For more details and discussion, see http://lists.llvm.org/pipermail/llvm-dev/2020-June/142099.html

It would be an idea to generally launch programs with the SEM_NO­GP­FAULT­ERROR­BOX flag in llvm-lit (and maybe not), so its is not necessary to insert code in test case programs that are not based on an LLVM tool or gtest.

Diff Detail

Event Timeline

Meinersbur created this revision.Jun 13 2020, 4:53 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJun 13 2020, 4:53 PM
Herald added subscribers: Restricted Project, dexonsmith, steven_wu and 2 others. · View Herald Transcript
Meinersbur edited the summary of this revision. (Show Details)Jun 13 2020, 5:05 PM

The reason KillTheDoctor exists is because of test-suite. It compiles and runs programs that do not contain calls to that API, and can do so with non-llvm compilers. I personally don't use it anymore as I don't run test-suite on Windows anymore, but I'm still not aware of any way short of changing registry settings to run test-suite on Windows without hanging other than this.

It's been 10 years, but I believe I tried the solution from https://devblogs.microsoft.com/oldnewthing/20160204-00/?p=92972 , but that didn't stop all modal dialog boxes from popping up.

With that, I'm not totally against removing it if nobody is using it. Just want to be clear that we don't have an alternate solution for what it solves.

I found some historic information by @Bigcheese: https://stackoverflow.com/questions/3735456/disable-windows-automated-handling-of-errors-in-subprocesses

According to this, problem was not SEM_NO­GP­FAULT­ERROR­BOX, but Dr. Watson ignoring it for unhandled exceptions (and the dialog box launched by the msvc runtime when an assertion failed and not in a debugger). I think the modern Windows error reporting does not have that issue anymore and the dialog box implementation of assert messagebox can be removed with _CrtSetReportMode.

lenary added a subscriber: lenary.Jun 15 2020, 6:44 AM
lenary added inline comments.
compiler-rt/test/cfi/lit.cfg.py
22–24

These changes look wrong.

pcc added a subscriber: pcc.Jun 15 2020, 8:48 AM

Win32 has an API for disabling error reporting which is already implemented in sys::DisableSystemDialogsOnCrash(), gtest and relevant test cases such as compiler-rt/test/asan/TestCases/ill.cpp.

But not by the CFI tests. I suspect that this change will cause some of the CFI tests to start to fail (if they are unsupported on your machine, you might not see the failure). What is the output of running check-cfi with this change?