From 79e5ecfa7a6c19ff08d39664bd30f72ab6e4f940 Mon Sep 17 00:00:00 2001
From: James Henderson <james.henderson@sony.com>
Date: Wed, 13 May 2020 10:29:02 +0100
Subject: [PATCH] On Windows, handle interrupt signals without crash message

For LLVM on *nix systems, the signal handlers are not run on signals
such as SIGINT due to CTRL-C. See sys::CleanupOnSignal. 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.

Reviewed by: MaskRay, rnk, aganea

Differential Revision: https://reviews.llvm.org/D79847
---
 llvm/lib/Support/Windows/Signals.inc | 16 +++++++----
 llvm/test/Support/interrupts.test    | 43 ++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 5 deletions(-)
 create mode 100644 llvm/test/Support/interrupts.test

diff --git a/llvm/lib/Support/Windows/Signals.inc b/llvm/lib/Support/Windows/Signals.inc
index 22f6ecfbc056..0c3681fa9654 100644
--- a/llvm/lib/Support/Windows/Signals.inc
+++ b/llvm/lib/Support/Windows/Signals.inc
@@ -584,7 +584,7 @@ void llvm::sys::AddSignalHandler(sys::SignalHandlerCallback FnPtr,
   LeaveCriticalSection(&CriticalSection);
 }
 
-static void Cleanup() {
+static void Cleanup(bool ExecuteSignalHandlers) {
   if (CleanupExecuted)
     return;
 
@@ -600,7 +600,10 @@ static void Cleanup() {
       llvm::sys::fs::remove(FilesToRemove->back());
       FilesToRemove->pop_back();
     }
-  llvm::sys::RunSignalHandlers();
+
+  if (ExecuteSignalHandlers)
+    llvm::sys::RunSignalHandlers();
+
   LeaveCriticalSection(&CriticalSection);
 }
 
@@ -610,7 +613,7 @@ void llvm::sys::RunInterruptHandlers() {
   // error handler). We must ensure that the critical section is properly
   // initialized.
   InitializeThreading();
-  Cleanup();
+  Cleanup(true);
 }
 
 /// Find the Windows Registry Key for a given location.
@@ -803,7 +806,7 @@ void sys::CleanupOnSignal(uintptr_t Context) {
 }
 
 static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
-  Cleanup();
+  Cleanup(true);
 
   // We'll automatically write a Minidump file here to help diagnose
   // the nasty sorts of crashes that aren't 100% reproducible from a set of
@@ -834,7 +837,10 @@ static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
 static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType) {
   // We are running in our very own thread, courtesy of Windows.
   EnterCriticalSection(&CriticalSection);
-  Cleanup();
+  // This function is only ever called when a CTRL-C or similar control signal
+  // is fired. Killing a process in this way is normal, so don't trigger the
+  // signal handlers.
+  Cleanup(false);
 
   // If an interrupt function has been set, go and run one it; otherwise,
   // the process dies.
diff --git a/llvm/test/Support/interrupts.test b/llvm/test/Support/interrupts.test
new file mode 100644
index 000000000000..1201e12829c4
--- /dev/null
+++ b/llvm/test/Support/interrupts.test
@@ -0,0 +1,43 @@
+## Show that SIGINT and similar signals don't cause crash messages to be
+## reported.
+# RUN: %python %s wrapper llvm-symbolizer 2> %t.err
+# RUN: count 0 < %t.err
+
+import os
+import signal
+import subprocess
+import sys
+import time
+
+def run_symbolizer():
+    proc = subprocess.Popen([sys.argv[2]], stdout=subprocess.PIPE, stdin=subprocess.PIPE)
+    # Write then read some output to ensure the process has started fully.
+    proc.stdin.write(b'foo\n')
+    proc.stdin.flush()
+    proc.stdout.readline()
+    # Windows handles signals differently.
+    if os.name == 'nt':
+        os.kill(0, signal.CTRL_BREAK_EVENT)
+    else:
+        proc.send_signal(signal.SIGINT)
+
+# On Windows, this function spawns the subprocess in its own (hidden) console,
+# so that signals do not interfere with the calling test. This isn't necessary
+# on other systems.
+def run_wrapper():
+    args = [sys.executable, __file__, 'symbolizer'] + sys.argv[2:]
+    if os.name == 'nt':
+        startupinfo = subprocess.STARTUPINFO()
+        startupinfo.dwFlags |= subprocess.STARTF_USESHOWWINDOW
+        proc = subprocess.Popen(args,
+                                stderr=sys.stderr,
+                                startupinfo=startupinfo,
+                                creationflags=subprocess.CREATE_NEW_CONSOLE)
+    else:
+        proc = subprocess.Popen(args,
+                                stderr=subprocess.PIPE)
+
+if sys.argv[1] == 'wrapper':
+    run_wrapper()
+else:
+    run_symbolizer()