MI Windows Ctrl-C Handling

[EwanCrawford]

Summary

Currently hitting Ctrl-C in Windows LLDB-MI just exits MI. But according to test_lldbmi_stopped_when_interrupt() in TestMiSignal.py Ctrl-C should send a SIGINT signal to the inferior.
Patch adds this functionality to Windows so SIGINT is sent on Ctrl-C.

[ki.stfu]

tools/lldb-mi/MICmnStreamStdin.cpp

18–22

store it before "In-house headers" section

219

missing else? to follow if - else if - else if here?

tools/lldb-mi/Platform.cpp

26

we shouldn't use SIGxxx constants. See how it was made in tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp (look for m_SIGINT).

I'm not sure whether it should be 0 or not. AFAIK @abidh refactored this code, maybe he knows it?

[abidh]

tools/lldb-mi/MICmnStreamStdin.cpp

219

Why we need to check for this error type here anyway. Will not the ctrl-c handler be called without this change. I also like to avoid #ifdef code as much as possible.

tools/lldb-mi/Platform.cpp

26

This is certainly an oversight to use 0 here. We are still using SIGINT in a few places in lldb-mi. Greg suggested that it is better to not depend on hard coded values and instead get those values from process at run time. But we may have to use hard coded SIGINT here and in MIDriverMain.cpp as those values are needed before process is initialized.

If you want to fully accurate, it may be good to check the argument dwCtrlType value too.

[EwanCrawford]

tools/lldb-mi/MICmnStreamStdin.cpp

219

The ctrl-c handler will indeed be called regardless. The problem is that hitting ctrl-c on Windows sets the end of file indicator, so if (::feof(stdin)) on the following line evaluates to True and MI force exits. Modifying this condition would be an alternative solution to avoid the #ifdef, but I'll need to look into that further.

tools/lldb-mi/Platform.cpp

26

Thanks for the comments, I'll try out some alternatives to hardcoding.

[abidh]

tools/lldb-mi/MICmnStreamStdin.cpp

219

In that case, this change is ok. Please rephrase the comments in the code to describe this.

tools/lldb-mi/Platform.cpp

26

I think hardcoding here may be ok.

[EwanCrawford]

Hardcoding the signal seems the best way for now, since the value ends up being compared directly against SIGINT in the driver. It might be easier to tidy up the SIG constants all at once.

I left the GetLastError() check wihout an 'else' condition, I'm not sure what it would include and the purpose of the #ifdef seems clearer this way,

[EwanCrawford]

Could you commit please, thanks. I'm requesting access so I don't need to keep asking you.

[abidh]

In D9248#161965, @EwanCrawford wrote:

Could you commit please, thanks. I'm requesting access so I don't need to keep asking you.

No Problem. I will commit it.

Thanks,
Abid

[abidh]

Committed in 235887.

tools/lldb-mi/MICmnStreamStdin.cpp

 //
 //===----------------------------------------------------------------------===//
 
+// Third Party Headers
+#ifdef _MSC_VER
+#include <Windows.h>
+#endif
+#include <string.h> // For std::strerror()
+
 // In-house headers:
 #include "MICmnStreamStdin.h"
 #include "MICmnStreamStdout.h"

 #include "MICmnLog.h"
 #include "MIDriver.h"
 #include "MIUtilSingletonHelper.h"
-#include <string.h> // For std::strerror()
 
 //++ ------------------------------------------------------------------------------------
 // Details: CMICmnStreamStdin constructor.

     const MIchar *pText = ::fgets(&m_pCmdBuffer[0], m_constBufferSize, stdin);
     if (pText == nullptr)
     {
+#ifdef _MSC_VER
+        // Was Ctrl-C hit?
+        // On Windows, Ctrl-C gives an ERROR_OPERATION_ABORTED as error on the command-line.
+        // The end-of-file indicator is also set, so without this check we will exit next if statement.
+        if (::GetLastError() == ERROR_OPERATION_ABORTED)
+            return nullptr;
+#endif
         if (::feof(stdin))
         {
             const bool bForceExit = true;

tools/lldb-mi/Platform.cpp

 {
     if (_ctrlHandler != NULL)
     {
-        _ctrlHandler(0);
+        _ctrlHandler(SIGINT);
         return TRUE;
     }
     return FALSE;