Page MenuHomePhabricator

[Support] On Windows, handle interrupt signals without crash message
ClosedPublic

Authored by jhenderson on Wed, May 13, 3:58 AM.

Details

Summary

On Linux, the signal handlers are not run on signals such as SIGINT due to CTRL-C. This makes sense, as such signals are not really crashes. Prior to this change, this wasn't the case on Windows, however. This patch changes the Windows behaviour to be consistent with Linux, and adds testing that verifies this.

The test uses llvm-symbolizer, but any tool with an interactive mode would do the job.

Fixes https://bugs.llvm.org/show_bug.cgi?id=45754.

Diff Detail

Event Timeline

jhenderson created this revision.Wed, May 13, 3:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptWed, May 13, 3:58 AM
Herald added a subscriber: hiraditya. · View Herald Transcript

If anybody would be willing to verify the test on cygwin, that would be helpful. I'm not 100% confident it will work there, but don't have cygwin installed to verify.

jhenderson planned changes to this revision.Wed, May 13, 4:00 AM

Forgot to run this with python 3, and there's a small problem which I need to fix.

Made test python3 compatible.

I do not think I am a good person to accept/review this. Your comments in PR45754 looks reasonable to me though.

aganea accepted this revision.Wed, May 20, 8:55 AM

LGTM.

This revision is now accepted and ready to land.Wed, May 20, 8:55 AM
rnk accepted this revision.Wed, May 20, 2:30 PM

lgtm

I don't have Cygwin anymore. IMO your code change seems low risk for cygwin, although the test could always fail with Cygwin Python, but IMO it's not worth worrying about.

MaskRay accepted this revision.Wed, May 20, 3:18 PM
MaskRay added inline comments.
llvm/test/Support/interrupts.test
5

count 0 < %t.err

or 2>&1 | count 0

On Linux, the signal handlers are not run on signals such as SIGINT due to CTRL-C.

That is the default behavior on a *NIX platform: on SIGINT, unregister the handler and call raise(SIGINT) to let the kernel terminate the process.

If a callback is registered via llvm::sys::SetInterruptFunction (used by bugpoint and clangd), that callback will run instead.

jhenderson marked 2 inline comments as done.Thu, May 21, 3:03 AM

On Linux, the signal handlers are not run on signals such as SIGINT due to CTRL-C.

That is the default behavior on a *NIX platform: on SIGINT, unregister the handler and call raise(SIGINT) to let the kernel terminate the process.

If a callback is registered via llvm::sys::SetInterruptFunction (used by bugpoint and clangd), that callback will run instead.

I wasn't quite sure what this comment was suggesting/discussing. Either way, I'll tweak my commit message to say that this matches the default behaviour for LLVM on Unix (see sys::CleanupOnSignal).

llvm/test/Support/interrupts.test
5

I'm deliberately avoiding the latter, since llvm-symbolizer prints to stdout as part of the script below.

This revision was automatically updated to reflect the committed changes.
jhenderson marked an inline comment as done.