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_NOGPFAULTERRORBOX [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=%22has+stopped+working%22
[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] https://devblogs.microsoft.com/oldnewthing/20160204-00/?p=92972
[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> On Jun 7, 2020, at 12:44 AM, Michael Kruse via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > 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_NOGPFAULTERRORBOX [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=%22has+stopped+working%22 > [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] https://devblogs.microsoft.com/oldnewthing/20160204-00/?p=92972 > [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 > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
On Sun, Jun 7, 2020 at 6:19 PM Chris Lattner via llvm-dev < llvm-dev at lists.llvm.org> wrote:> +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 > > > On Jun 7, 2020, at 12:44 AM, Michael Kruse via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > > > > 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_NOGPFAULTERRORBOX [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.> > > > Michael > > > > [1] > https://github.com/llvm/llvm-project/tree/master/llvm/utils/KillTheDoctor > > [2] https://www.ecosia.org/images?q=%22has+stopped+working%22 > > [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] https://devblogs.microsoft.com/oldnewthing/20160204-00/?p=92972 > > [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 > > _______________________________________________ > > LLVM Developers mailing list > > llvm-dev at lists.llvm.org > > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-- Saleem Abdulrasool compnerd (at) compnerd (dot) org -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200608/13c99801/attachment.html>
I opened a patch review at https://reviews.llvm.org/D81801. Michael Am So., 7. Juni 2020 um 02:44 Uhr schrieb Michael Kruse <llvmdev at meinersbur.de>:> > 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_NOGPFAULTERRORBOX [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=%22has+stopped+working%22 > [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] https://devblogs.microsoft.com/oldnewthing/20160204-00/?p=92972 > [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