Fix a problem where lldb-mi would not stop the debuggee after -exec-interrupt command.

[abidh]

Summary

This revision fixes a problem where lldb-mi would not stop the execution after exec-interrupt call.
On Linux, SIGSTOP is used to stop the debuggee process. LLDB stopped the debuggee alright. But when
lldb-mi received the notification of stopping with reason as SIGSTOP, it would resume the process.
This was heppening in CMICmnLLDBDebuggerHandleEvents::HandleProcessEventStopSignal. This function aslo
used hard coded numbers for signal istead of symbolic names.

This revision changes code to treat SIGSTOP reason as SIGINT. Also used symbolic names for signals
instead of numbers.

[ki.stfu]

Can you provide test for it?

tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp

884

What is it? SIGSTOP?

[ki.stfu]

In D7783#127295, @ki.stfu wrote:

Can you provide test for it?

I'll add this test in D7762.

[abidh]

In D7783#127420, @ki.stfu wrote:

In D7783#127295, @ki.stfu wrote:

Can you provide test for it?

I'll add this test in D7762.

Thanks. I will wait for the test.

tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp

884

Yes it is SIGSTOP.

[zturner]

Does this work on windows? I see reference to some signal constants?

[ki.stfu]

In D7783#127464, @zturner wrote:

Does this work on windows? I see reference to some signal constants?

It can be done like this (right?):

#ifdef _WIN32
#define SIGINT 2
#define SIGSTOP 19
...
#endif // _WIN32

[abidh]

In D7783#127464, @zturner wrote:

Does this work on windows? I see reference to some signal constants?

I dont think this is a problem for Windows as I think SIGSTOP is not used to stop the process there. OTOH, I see that SIGSTOP is defined to a 2 different values within lldb.
#define SIGSTOP 20 win32.h
#define SIGSTOP 23 Platform.h

So probably this needs to be corrected.

[zturner]

As long as it compiles it's probably ok for now. But yes, fixing the signal
values would be nice.

[ki.stfu]

In D7783#127438, @abidh wrote:

In D7783#127420, @ki.stfu wrote:

In D7783#127295, @ki.stfu wrote:

Can you provide test for it?

I'll add this test in D7762.

Thanks. I will wait for the test.

On OS X 2 tests failed: test_lldbmi_stopped_when_stopatentry_local / test_lldbmi_stopped_when_stopatentry_remote. I did some fix for it:

• Add thread-id and stopped-threads in SIGINT message:

@@ -852,11 +853,18 @@
             bOk = bOk && MiHelpGetCurrentThreadFrame(miValueTuple);
             const CMICmnMIValueResult miValueResult5("frame", miValueTuple);
             bOk = bOk && miOutOfBandRecord.Add(miValueResult5);
+            const CMIUtilString strThreadId(CMIUtilString::Format("%d", sbProcess.GetSelectedThread().GetIndexID()));
+            const CMICmnMIValueConst miValueConst6(strThreadId);
+            const CMICmnMIValueResult miValueResult6("thread-id", miValueConst6);
+            bOk = bOk && miOutOfBandRecord.Add(miValueResult6);
+            const CMICmnMIValueConst miValueConst7("all");
+            const CMICmnMIValueResult miValueResult7("stopped-threads", miValueConst7);
+            bOk = bOk && miOutOfBandRecord.Add(miValueResult7);
             bOk = bOk && MiOutOfBandRecordToStdout(miOutOfBandRecord);
             bOk = bOk && TextToStdout("(gdb)");
         }
         break;

• Fix signal tests:

Index: test/tools/lldb-mi/signal/TestMiSignal.py
===================================================================
--- test/tools/lldb-mi/signal/TestMiSignal.py   (revision 230022)
+++ test/tools/lldb-mi/signal/TestMiSignal.py   (working copy)
@@ -64,10 +64,7 @@
         self.expect("\^done")

         # Test that *stopped is printed
-        # Note that message is different in Darwin and Linux:
-        # Darwin: "*stopped,reason=\"signal-received\",signal=\"17\",thread-id=\"1\",stopped-threads=\"all\""
-        # Linux:  "*stopped,reason=\"end-stepping-range\",frame={addr=\"0x[0-9a-f]+\",func=\"??\",args=\[\],file=\"??\",fullname=\"??\",line=\"-1\"},thread-id=\"1\",stopped-threads=\"all\"
-        self.expect("\*stopped,reason=\"(signal-received|end-stepping-range)\",.+,thread-id=\"1\",stopped-threads=\"all\"")
+        self.expect("\*stopped,reason=\"signal-received\",signal-name=\"SIGINT\",signal-meaning=\"Interrupt\",.*thread-id=\"1\",stopped-threads=\"all\"")

         # Run to main to make sure we have not exited the application
         self.runCmd("-break-insert -f main")
@@ -112,7 +109,7 @@
             self.expect("\^done")

             # Test that *stopped is printed
-            self.expect("\*stopped,reason=\"signal-received\",signal=\"17\",thread-id=\"1\",stopped-threads=\"all\"")
+            self.expect("\*stopped,reason=\"signal-received\",signal-name=\"SIGINT\",signal-meaning=\"Interrupt\",.*thread-id=\"1\",stopped-threads=\"all\"")

             # Exit
             self.runCmd("-gdb-exit")

[clayborg]

It is ok to rely on signals here because CMICmnLLDBDebuggerHandleEvents::HandleProcessEventStateStopped() already determined that a signal was reported as the stop reason and windows just can avoid saying it stopped due to a signal, so no work needs to be done to work around this.

You do need to ask the process about its signals and you can't use hard coded values, you need to look them up by name through the SBUnixSignals object that you can get from a live SBProcess instance. See inlined comments above.

tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp

839–840

You need to ask the process about its unix signal and you can't use hard coded values. You can make some member variables in CMICmnLLDBDebuggerHandleEvents and then initialize them when you have a process. The process must be up and running, you can't ask the process before it is stopped at its first stop. So when you get a eStateStopped event, see if it is the first one and then initialize your ivars with something like:

void
CMICmnLLDBDebuggerHandleEvents::InitializeSignals()
{
    SBUnixSignals unix_signals = sbProcess.GetUnixSignals();
    m_SIGINT = unix_signals.GetSignalNumberFromName("SIGINT");
    m_SIGSTOP = unix_signals.GetSignalNumberFromName("SIGSTOP");
    m_SIGSEGV = unix_signals.GetSignalNumberFromName("SIGSEGV");
    m_SIGTRAP = unix_signals.GetSignalNumberFromName("SIGTRAP");
}

[zturner]

That's fine, I was mostly just concerned that it wouldn't compile. Windows
has only a very small subset of SIG_XXX constants defined in its system
headers. lldb and lldb-mi define a few more for Windows that it needs, I
just didn't have code in front of me so I wanted to make sure that the
#defines being used here are defined somewhere on Windows so that it would
compile.

[abidh]

Obtained values of the signal from Process instead of using hard-coded values as suggested in review.
Patches looks bigger due to indentation change after replacing switch with if.

All tests pass on Linux and it build ok on Windows.

[ki.stfu]

Looks good. I'll check it on OS X tomorrow.

Don't wait me. I'll fix tests if something is wrong.

[clayborg]

Looks good.

tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp

 CMICmnLLDBDebuggerHandleEvents::HandleProcessEventStopSignal(bool &vwrbShouldBrk)
 {
     bool bOk = MIstatus::success;
 
     lldb::SBProcess sbProcess = CMICmnLLDBDebugSessionInfo::Instance().GetProcess();
     const MIuint64 nStopReason = sbProcess.GetSelectedThread().GetStopReasonDataAtIndex(0);
     switch (nStopReason)
     {
-        case 2: // Terminal interrupt signal. SIGINT
+        case SIGINT: // Terminal interrupt signal. SIGINT
+        case SIGSTOP:
         {
             // MI print "*stopped,reason=\"signal-received\",signal-name=\"SIGNINT\",signal-meaning=\"Interrupt\",frame={%s}"
             const CMICmnMIValueConst miValueConst("signal-received");
             const CMICmnMIValueResult miValueResult("reason", miValueConst);
             CMICmnMIOutOfBandRecord miOutOfBandRecord(CMICmnMIOutOfBandRecord::eOutOfBand_Stopped, miValueResult);
             const CMICmnMIValueConst miValueConst2("SIGINT");
             const CMICmnMIValueResult miValueResult2("signal-name", miValueConst2);
             bOk = miOutOfBandRecord.Add(miValueResult2);
             const CMICmnMIValueConst miValueConst3("Interrupt");
             const CMICmnMIValueResult miValueResult3("signal-meaning", miValueConst3);
             bOk = bOk && miOutOfBandRecord.Add(miValueResult3);
             CMICmnMIValueTuple miValueTuple;
             bOk = bOk && MiHelpGetCurrentThreadFrame(miValueTuple);
             const CMICmnMIValueResult miValueResult5("frame", miValueTuple);
             bOk = bOk && miOutOfBandRecord.Add(miValueResult5);
             bOk = bOk && MiOutOfBandRecordToStdout(miOutOfBandRecord);
             bOk = bOk && TextToStdout("(gdb)");
         }
         break;
-        case 11: // Invalid memory reference. SIGSEGV
+        case SIGSEGV: // Invalid memory reference. SIGSEGV
         {
             // MI print "*stopped,reason=\"signal-received\",signal-name=\"SIGSEGV\",signal-meaning=\"Segmentation
             // fault\",thread-id=\"%d\",frame={%s}"
             const CMICmnMIValueConst miValueConst("signal-received");
             const CMICmnMIValueResult miValueResult("reason", miValueConst);
             CMICmnMIOutOfBandRecord miOutOfBandRecord(CMICmnMIOutOfBandRecord::eOutOfBand_Stopped, miValueResult);
             const CMICmnMIValueConst miValueConst2("SIGSEGV");
             const CMICmnMIValueResult miValueResult2("signal-name", miValueConst2);
             bOk = miOutOfBandRecord.Add(miValueResult2);
             const CMICmnMIValueConst miValueConst3("Segmentation fault");
             const CMICmnMIValueResult miValueResult3("signal-meaning", miValueConst3);
             bOk = bOk && miOutOfBandRecord.Add(miValueResult3);
             const CMIUtilString strThreadId(CMIUtilString::Format("%d", sbProcess.GetSelectedThread().GetIndexID()));
             const CMICmnMIValueConst miValueConst4(strThreadId);
             const CMICmnMIValueResult miValueResult4("thread-id", miValueConst4);
             bOk = bOk && miOutOfBandRecord.Add(miValueResult4);
             CMICmnMIValueTuple miValueTuple;
             bOk = bOk && MiHelpGetCurrentThreadFrame(miValueTuple);
             const CMICmnMIValueResult miValueResult5("frame", miValueTuple);
             bOk = bOk && miOutOfBandRecord.Add(miValueResult5);
             bOk = bOk && MiOutOfBandRecordToStdout(miOutOfBandRecord);
             // Note no "(gdb)" output here
         }
         break;
-        case 19:
-            if (sbProcess.IsValid())
-                sbProcess.Continue();
-            break;
-        case 5: //  Trace/breakpoint trap. SIGTRAP
+        case SIGTRAP: //  Trace/breakpoint trap. SIGTRAP
         {
             lldb::SBThread thread = sbProcess.GetSelectedThread();
             const MIuint nFrames = thread.GetNumFrames();
             if (nFrames > 0)
             {
                 lldb::SBFrame frame = thread.GetFrameAtIndex(0);
                 const char *pFnName = frame.GetFunctionName();
                 if (pFnName != nullptr)