[lldb-mi] Re-implement MI -exec-next command.

[polyakov.alex]

Summary

Now -exec-next command uses SB API for stepping over.

[aprantl]

lit/tools/lldb-mi/exec/exec-next.test

2 ↗

(On Diff #150035)

@stella.stemanova: Would be interesting to understand why (/if ?) this doesn't work on windows

[stella.stamenova]

lit/tools/lldb-mi/exec/exec-next.test

2 ↗

(On Diff #150035)

I can test it out after it's committed.

[labath]

tools/lldb-mi/MICmdCmdExec.cpp

384 ↗

(On Diff #150035)

It looks like this branch is not covered by the test. Also, do we care about what happens when the client specifies an invalid thread id?
(I'm mainly pointing these out because these seem like they would be hard to test in the current framework, the thread ids being unpredictable and all.)

[polyakov.alex]

Added some checks for invalid thread id.

[polyakov.alex]

tools/lldb-mi/MICmdCmdExec.cpp

384 ↗

(On Diff #150137)

We can't test this branch until we have a way to get thread ID and then store it in some variable.

[aprantl]

lit/tools/lldb-mi/exec/exec-next.test

19 ↗

(On Diff #150137)

0 feels like it might actually be a valid thread id on some systems.. perhaps use really high number instead?

[labath]

lit/tools/lldb-mi/exec/exec-next.test

19 ↗

(On Diff #150137)

I was surprised by that as well, so I tried a some experiments. I don't know how or why, but lldb-mi seems to use it's own notion of thread-ids, which are independent of os-level ids and always start with one. I guess they are just indexes into the list of threads. I don't know if that is intentional or what.

tools/lldb-mi/MICmdCmdExec.cpp

384 ↗

(On Diff #150137)

Yes, that's the issue I was alluding to. I am not going to block this patch over it or anything, but I want to make sure you are aware that you're hitting the limitations of the FileCheck test approach already.

That said, if what I said above about the thread-id's always being numbered starting from one is true, then the thread ids may be predictable enough to test using -exec-next --thread 1 and avoid this problem for now.

[polyakov.alex]

Changes for using thread ids in a gdb way. Also added case of specified thread into test case.

[labath]

looks good to me

lldb/trunk/lit/tools/lldb-mi/exec/exec-next.test

+# XFAIL: windows
+# -> llvm.org/pr24452
+#
+# RUN: %cc -o %t %p/inputs/main.c -g
+# RUN: %lldbmi %t < %s | FileCheck %s
+
+# Test lldb-mi -exec-next command.
+
+# Check that we have a valid target created via '%lldbmi %t'.
+# CHECK: ^done
+
+-break-insert main
+# CHECK: ^done,bkpt={number="1"
+
+-exec-run
+# CHECK: ^running
+# CHECK: *stopped,reason="breakpoint-hit"
+
+-exec-next --thread 0
+# Check that exec-next can process the case of invalid thread ID.
+# CHECK: ^error,msg="Command 'exec-next'. Thread ID invalid"
+
+-exec-next --thread 1
+# CHECK: ^running
+# CHECK: *stopped,reason="end-stepping-range"
+
+-exec-next
+# Check that exec-next can step over in a selected thread.
+# CHECK: ^running
+# CHECK: *stopped,reason="end-stepping-range"

lldb/trunk/lit/tools/lldb-mi/exec/inputs/main.c

+int main(void) {
+  int x = 0;
+  return x;
+}

lldb/trunk/tools/lldb-mi/MICmdCmdExec.cpp

 //              CMICmdCmdExecInterrupt          implementation.
 //              CMICmdCmdExecArguments          implementation.
 //              CMICmdCmdExecAbort              implementation.
 
 // Third Party Headers:
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/API/SBThread.h"
 #include "lldb/lldb-enumerations.h"
 
 // In-house headers:
 #include "MICmdArgValListOfN.h"
 #include "MICmdArgValNumber.h"
 #include "MICmdArgValOptionLong.h"
 #include "MICmdArgValOptionShort.h"
 #include "MICmdArgValString.h"
if (pArgThread->GetFound() &&
     SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_THREAD_INVALID),
                                    m_cmdData.strMiCmd.c_str(),
                                    m_constStrArgThread.c_str()));
     return MIstatus::failure;
   }
 
   CMICmnLLDBDebugSessionInfo &rSessionInfo(
       CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBDebugger &rDebugger = rSessionInfo.GetDebugger();
-  CMIUtilString strCmd("thread step-over");
-  if (nThreadId != UINT64_MAX)
-    strCmd += CMIUtilString::Format(" %llu", nThreadId);
-  rDebugger.GetCommandInterpreter().HandleCommand(strCmd.c_str(), m_lldbResult,
-                                                  false);
+
+  if (nThreadId != UINT64_MAX) {
+    lldb::SBThread sbThread = rSessionInfo.GetProcess().GetThreadByIndexID(nThreadId);
+    if (!sbThread.IsValid()) {
+      SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_THREAD_INVALID),
+                                     m_cmdData.strMiCmd.c_str(),
+                                     m_constStrArgThread.c_str()));
+      return MIstatus::failure;
+    }
+    sbThread.StepOver();
+  } else rSessionInfo.GetProcess().GetSelectedThread().StepOver();
 
   return MIstatus::success;
 }
 
 //++
 //------------------------------------------------------------------------------------
 // Details: The invoker requires this function. The command prepares a MI Record
 // Result