[lldb] [llgs] Support "t" vCont action

[mgorny]

Summary

Implement support for the "t" action that is used to stop a thread.
Normally this action is used only in non-stop mode. However, there's
no technical reason why it couldn't be also used in all-stop mode,
e.g. to express "resume all threads except ..." (t:...;c).

While at it, add a more complete test for vCont correctly resuming
a subset of program's threads.

Sponsored by: The FreeBSD Foundation

[mib]

Hey @mgorny,

This patch is causing a test to fail on the macOS bot:
https://green.lab.llvm.org/green/job/lldb-cmake/44921/consoleText

Feel free to skip it or let me know if you need help reproducing the issue.

FAIL: lldb-api :: tools/lldb-server/TestGdbRemote_vCont.py (734 of 1734)
******************** TEST 'lldb-api :: tools/lldb-server/TestGdbRemote_vCont.py' FAILED ********************
FAIL: LLDB (/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/clang-x86_64) :: test_vCont_supports_t_debugserver (TestGdbRemote_vCont.TestGdbRemote_vCont)
UNSUPPORTED: LLDB (/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/clang-x86_64) :: test_vCont_supports_t_llgs (TestGdbRemote_vCont.TestGdbRemote_vCont) (test case does not fall in any category of interest for this run) 
Restore dir to: /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/tools/lldb/test
======================================================================
FAIL: test_vCont_supports_t_debugserver (TestGdbRemote_vCont.TestGdbRemote_vCont)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py", line 52, in test_method
    return attrvalue(self)
  File "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/API/tools/lldb-server/TestGdbRemote_vCont.py", line 44, in test_vCont_supports_t
    self.vCont_supports_mode("t")
  File "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/API/tools/lldb-server/TestGdbRemote_vCont.py", line 23, in vCont_supports_mode
    self.assertIn(mode, supported_vCont_modes)
AssertionError: 't' not found in {'c': 1, 'C': 1, 's': 1, 'S': 1}
Config=x86_64-/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/clang
----------------------------------------------------------------------
Ran 14 tests in 17.353s

RESULT: FAILED (6 passes, 1 failures, 0 errors, 7 skipped, 0 expected failures, 0 unexpected successes)

[mgorny]

Thanks, I've pushed a skip. I'm sorry, I keep forgetting to mark these new tests llgs-only :-(.

[labath]

Some flakyness here: https://lab.llvm.org/buildbot/#/builders/68/builds/35111/steps/6/logs/stdio

[mgorny]

In D126983#3624800, @labath wrote:

Some flakyness here: https://lab.llvm.org/buildbot/#/builders/68/builds/35111/steps/6/logs/stdio

That's so rare it's going to be hard figuring out if any changes actually fix it. My only idea is to either increase the sleep within continue_and_get_threads_running() or make output inside the process more frequent — though the latter probably won't help if some threads don't get run because of loaded system.

[labath]

In D126983#3624901, @mgorny wrote:

In D126983#3624800, @labath wrote:

Some flakyness here: https://lab.llvm.org/buildbot/#/builders/68/builds/35111/steps/6/logs/stdio

That's so rare it's going to be hard figuring out if any changes actually fix it. My only idea is to either increase the sleep within continue_and_get_threads_running() or make output inside the process more frequent — though the latter probably won't help if some threads don't get run because of loaded system.

Don't we have the ability to wait for a specific output from the process? Could we make use of that to await for the actual "thread is running" message? If needed, we can put some barriers between the print calls, and then we can even make sure each thread prints its message exactly once.

[mgorny]

I don't think we can reliably check whether additional threads aren't run then.

I have another idea that could work better if we could assume that the scheduler runs threads of the same process somehow evenly — or well, I suppose it'd be better in any case. Rather than stopping the process from LLDB, I could add a counter that issues SIGSTOP from inside the process after doing N (say, 5) prints. I suppose that should give it enough time for all threads to output something.

Just need to figure out a simple synchronization method to ensure that SIGSTOP is emitted only once.

[mgorny]

In D126983#3624915, @labath wrote:

Don't we have the ability to wait for a specific output from the process?

Hmm, actually thinking about it, I suppose we could wait for m*n messages from m threads (say, n=5) and then we should have a pretty good chance that if an extra thread is running, it would fit somewhere between them.

[mgorny]

Unfortunately, switching to output_match makes the test even more flaky. I think it doesn't handle it well if two output lines get sent as a single packet.

That said, the other tests are probably flaky the other way — they rely on all signals being reported in a single output packet.

I'm going to continue trying to improve the test.

lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp

case eStateStepping: {
       const int signo = action->signal;
       ResumeThread(static_cast<NativeThreadLinux &>(*thread), action->state,
                    signo);
       break;
     }
 
     case eStateSuspended:
     case eStateStopped:
-      llvm_unreachable("Unexpected state");
+      break;
 
     default:
       return Status("NativeProcessLinux::%s (): unexpected state %s specified "
                     "for pid %" PRIu64 ", tid %" PRIu64,
                     __FUNCTION__, StateAsCString(action->state), GetID(),
                     thread->GetID());
     }
   }

lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp

for (uint32_t i = 0; i < m_threads.size(); ++i) {
                    "Trying to resume thread at index {0}, but failed with "
                    "error {1}.",
                    i, result);
         }
         break;
       }
       case eStateSuspended:
       case eStateStopped:
-        llvm_unreachable("Unexpected state");
+        break;
 
       default:
         return Status(
             "NativeProcessWindows::%s (): unexpected state %s specified "
             "for pid %" PRIu64 ", tid %" PRIu64,
             __FUNCTION__, StateAsCString(action->state), GetID(),
             thread->GetID());
       }

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

GDBRemoteCommunicationServerLLGS::Handle_c(StringExtractorGDBRemote &packet) {
 
   return SendContinueSuccessResponse();
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::Handle_vCont_actions(
     StringExtractorGDBRemote &packet) {
   StreamString response;
-  response.Printf("vCont;c;C;s;S");
+  response.Printf("vCont;c;C;s;S;t");
 
   return SendPacketNoLock(response.GetString());
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::Handle_vCont(
     StringExtractorGDBRemote &packet) {
   Log *log = GetLog(LLDBLog::Process);
case 'S':
             packet, "Could not parse signal in vCont packet S action");
       LLVM_FALLTHROUGH;
 
     case 's':
       // Step
       thread_action.state = eStateStepping;
       break;
 
+    case 't':
+      // Stop
+      thread_action.state = eStateSuspended;
+      break;
+
     default:
       return SendIllFormedResponse(packet, "Unsupported vCont action");
       break;
     }
 
     lldb::pid_t pid = StringExtractorGDBRemote::AllProcesses;
     lldb::tid_t tid = StringExtractorGDBRemote::AllThreads;
 

lldb/test/API/tools/lldb-server/TestGdbRemote_vCont.py

class TestGdbRemote_vCont(gdbremote_testcase.GdbRemoteTestCaseBase):
     def test_vCont_supports_s(self):
         self.build()
         self.vCont_supports_mode("s")
 
     def test_vCont_supports_S(self):
         self.build()
         self.vCont_supports_mode("S")
 
+    def test_vCont_supports_t(self):
+        self.build()
+        self.vCont_supports_mode("t")
+
     @skipIfWindows # No pty support to test O* & I* notification packets.
     @skipIf(triple='^mips')
     def test_single_step_only_steps_one_instruction_with_Hc_vCont_s(self):
         self.build()
         self.set_inferior_startup_launch()
         self.single_step_only_steps_one_instruction(
             use_Hc_packet=True, step_instruction="vCont;s")
 
     @skipIfWindows # No pty support to test O* & I* notification packets.
     @skipIf(triple='^mips')
     def test_single_step_only_steps_one_instruction_with_vCont_s_thread(self):
         self.build()
         self.set_inferior_startup_launch()
         self.single_step_only_steps_one_instruction(
             use_Hc_packet=False, step_instruction="vCont;s:{thread}")

lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py

 import json
 import re
+import time
 
 import gdbremote_testcase
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 class TestGdbRemote_vContThreads(gdbremote_testcase.GdbRemoteTestCaseBase):
 
     def start_threads(self, num):
         procs = self.prep_debug_monitor_and_inferior(inferior_args=[str(num)])
         # start the process and wait for output
         self.test_sequence.add_log_lines([
             "read packet: $c#63",
-            {"type": "output_match", "regex": self.maybe_strict_output_regex(
-                r"@started\r\n")},
+            {"type": "output_match", "regex": r".*@started\r\n.*"},
         ], True)
         # then interrupt it
         self.add_interrupt_packets()
         self.add_threadinfo_collection_packets()
 
         context = self.expect_gdbremote_sequence()
         self.assertIsNotNone(context)
         threads = self.parse_threadinfo_packets(context)
         self.assertIsNotNone(threads)
         self.assertEqual(len(threads), num + 1)
 
         self.reset_test_sequence()
         return threads
 
     def send_and_check_signal(self, vCont_data, threads):
         self.test_sequence.add_log_lines([
             "read packet: $vCont;{0}#00".format(vCont_data),
             {"type": "output_match",
-             "regex": self.maybe_strict_output_regex(
-                 len(threads) *
-                 r"received SIGUSR1 on thread id: ([0-9a-f]+)\r\n"),
+             "regex": len(threads) *
+                      r".*received SIGUSR1 on thread id: ([0-9a-f]+)\r\n.*",
              "capture": dict((i, "tid{0}".format(i)) for i
                              in range(1, len(threads)+1)),
              },
         ], True)
 
         context = self.expect_gdbremote_sequence()
         self.assertIsNotNone(context)
         tids = sorted(int(context["tid{0}".format(x)], 16)
def test_signal_two_signals(self):
             "read packet: $vCont;C{0:x}:{1:x};C{2:x}:{3:x}#00".format(
                 lldbutil.get_signal_number('SIGUSR1'), threads[0],
                 lldbutil.get_signal_number('SIGUSR2'), threads[1]),
             {"direction": "send", "regex": r"^\$E1e#db$"},
         ], True)
 
         context = self.expect_gdbremote_sequence()
         self.assertIsNotNone(context)
+
+    THREAD_MATCH_RE = re.compile(r"thread ([0-9a-f]+) running")
+
+    def continue_and_get_threads_running(self, continue_packet):
+        self.test_sequence.add_log_lines(
+            ["read packet: ${}#00".format(continue_packet),
+             ], True)
+        self.expect_gdbremote_sequence()
+        self.reset_test_sequence()
+        time.sleep(1)
+        self.add_interrupt_packets()
+        exp = self.expect_gdbremote_sequence()
+        found = set()
+        for line in exp["O_content"].decode().splitlines():
+            m = self.THREAD_MATCH_RE.match(line)
+            if m is not None:
+                found.add(int(m.group(1), 16))
+        return found
+
+    @add_test_categories(["llgs"])
+    def test_vCont_run_subset_of_threads(self):
+        self.build()
+        self.set_inferior_startup_launch()
+
+        threads = set(self.start_threads(3))
+        all_subthreads = self.continue_and_get_threads_running("c")
+        all_subthreads_list = list(all_subthreads)
+        self.assertEqual(len(all_subthreads), 3)
+        self.assertEqual(threads & all_subthreads, all_subthreads)
+
+        # resume two threads explicitly, stop the third one implicitly
+        self.assertEqual(
+            self.continue_and_get_threads_running(
+                "vCont;c:{:x};c:{:x}".format(*all_subthreads_list[:2])),
+            set(all_subthreads_list[:2]))
+
+        # resume two threads explicitly, stop others explicitly
+        self.assertEqual(
+            self.continue_and_get_threads_running(
+                "vCont;c:{:x};c:{:x};t".format(*all_subthreads_list[:2])),
+            set(all_subthreads_list[:2]))
+
+        # stop one thread explicitly, resume others
+        self.assertEqual(
+            self.continue_and_get_threads_running(
+                "vCont;t:{:x};c".format(all_subthreads_list[-1])),
+            set(all_subthreads_list[:2]))
+
+        # resume one thread explicitly, stop one explicitly,
+        # resume others
+        self.assertEqual(
+            self.continue_and_get_threads_running(
+                "vCont;c:{:x};t:{:x};c".format(*all_subthreads_list[-2:])),
+            set(all_subthreads_list[:2]))
+
+        # resume one thread explicitly, stop one explicitly,
+        # stop others implicitly
+        self.assertEqual(
+            self.continue_and_get_threads_running(
+                "vCont;t:{:x};c:{:x}".format(*all_subthreads_list[:2])),
+            set(all_subthreads_list[1:2]))
+
+        # resume one thread explicitly, stop one explicitly,
+        # stop others explicitly
+        self.assertEqual(
+            self.continue_and_get_threads_running(
+                "vCont;t:{:x};c:{:x};t".format(*all_subthreads_list[:2])),
+            set(all_subthreads_list[1:2]))

lldb/test/API/tools/lldb-server/vCont-threads/main.cpp

static void sigusr1_handler(int signo) {
   std::snprintf(buf, sizeof(buf),
                 "received SIGUSR1 on thread id: %" PRIx64 "\n",
                 get_thread_id());
   write(STDOUT_FILENO, buf, strlen(buf));
 }
 
 static void thread_func() {
   pseudo_barrier_wait(barrier);
-  std::this_thread::sleep_for(std::chrono::minutes(1));
+  for (int i = 0; i < 300; ++i) {
+    std::printf("thread %" PRIx64 " running\n", get_thread_id());
+    std::this_thread::sleep_for(std::chrono::milliseconds(200));
+  }
 }
 
 int main(int argc, char **argv) {
   int num = atoi(argv[1]);
 
   pseudo_barrier_init(barrier, num + 1);
 
   signal(SIGUSR1, sigusr1_handler);