Kill "KillTheDoctor"

We still have the program called "KillTheDoctor" [1] in our source
tree. Its original intention was to stop requiring user interaction
for crashing regression tests under Windows (infamous Dr. Watson,
nowadays "[program] has stopped working" [2]), I don't think it is
useful anymore. It's also a very hacky approach, as admitted in the
source comment itself ("I hate Windows.")

I don't think it was the right approach in the first place. Today,
LLVM tools disable error reporting themselves [3,4].
SEM_NO­GP­FAULT­ERROR­BOX [5] can be passed when launching arbitrary
programs with error reporting disabled.

The only reference to it in the current source tree is compiler-rt [6]
and explicitly mentions it should be integrated with "not --crash".
However, it apparently already handles this case [7] and is already
used outside of compiler-rt, eg. [8]. Removing the special case for
Windows in [6] still passes the tests on my Windows machine (at least
check-asan and check-builtins).

Therefore, I propose to remove KillTheDoctor.

Michael

[1] https://github.com/llvm/llvm-project/tree/master/llvm/utils/KillTheDoctor
[2] https://www.ecosia.org/images?q="has+stopped+working"
[3] https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/Windows/Signals.inc#L484
[4] https://github.com/llvm/llvm-project/blob/master/llvm/utils/unittest/UnitTestMain/TestMain.cpp#L38
[5] How do I prevent a child process from displaying the Windows Error Reporting dialog? - The Old New Thing
[6] https://github.com/llvm/llvm-project/blob/master/compiler-rt/test/lit.common.cfg.py#L219
[7] https://github.com/llvm/llvm-project/blob/master/llvm/utils/not/not.cpp#L48
[8] https://github.com/llvm/llvm-project/blob/master/clang/test/Frontend/remove-file-on-signal.c#L2

+Saleem as one of the people who know a lot about Windows.

I don’t have any technical insight on this, but if we can, it would be great to drop this.

-Chris

+Saleem as one of the people who know a lot about Windows.

I don’t have any technical insight on this, but if we can, it would be great to drop this.

-Chris

We still have the program called “KillTheDoctor” [1] in our source
tree. Its original intention was to stop requiring user interaction
for crashing regression tests under Windows (infamous Dr. Watson,
nowadays “[program] has stopped working” [2]), I don’t think it is
useful anymore. It’s also a very hacky approach, as admitted in the
source comment itself (“I hate Windows.”)

I don’t think it was the right approach in the first place. Today,
LLVM tools disable error reporting themselves [3,4].
SEM_NO­GP­FAULT­ERROR­BOX [5] can be passed when launching arbitrary
programs with error reporting disabled.

The only reference to it in the current source tree is compiler-rt [6]
and explicitly mentions it should be integrated with “not --crash”.
However, it apparently already handles this case [7] and is already
used outside of compiler-rt, eg. [8]. Removing the special case for
Windows in [6] still passes the tests on my Windows machine (at least
check-asan and check-builtins).

Therefore, I propose to remove KillTheDoctor.

At this point, the expected environment is pretty much Windows 10. That is plenty new enough to support the use of SetThreadErrorMode. I don’t see a strong reason to use the DebugAPI to trace the process when we can ensure that we set the error mode properly in LLVMInitializeCore. This seems pretty reasonable to me. I think that conferring with Ried and Nico is a good idea to ensure that the chrome team doesn’t have any issues with this.

+1 from me.

I don't expect this would cause any problems for Chromium's infra.

- Hans

I opened a patch review at https://reviews.llvm.org/D81801.

Michael