[lldb] [llgs] Fix signo sent with fork/vfork/vforkdone events

[mgorny]

Summary

Fix ThreadStopInfo struct to include the signal number for all events.
Since signo was not included in the details for fork, vfork
and vforkdone stops, the code incidentally referenced the wrong union
member, resulting in wrong signo being sent.

Sponsored by: The FreeBSD Foundation

[labath]

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

697

It seems this struct is only used in the lldb-server. Maybe we could make this a member function instead? Or move the signo field out of the union?

[mgorny]

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

697

I would lean towards the latter, I guess. Lemme look about updating the patch.

[mgorny]

Update to make signo an unconditional member of the struct, and set it for fork/vfork/vforkdone events explicitly.

[labath]

cool

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

static const char *GetStopReasonString(StopReason stop_reason) {
   case eStopReasonPlanComplete:
   case eStopReasonThreadExiting:
   case eStopReasonNone:
     break; // ignored
   }
   return nullptr;
 }
 
+static int GetSignalForStopReason(const struct ThreadStopInfo &tid_stop_info) {
+  if (tid_stop_info.reason == eStopReasonException)
+    return SIGTRAP; // TODO: does this make sense?
+  if (tid_stop_info.reason == eStopReasonFork ||
+      tid_stop_info.reason == eStopReasonVFork ||
+      tid_stop_info.reason == eStopReasonVForkDone)
+    return SIGTRAP;
+  return tid_stop_info.details.signal.signo;
+}
+
 static llvm::Expected<json::Array>
 GetJSONThreadsInfo(NativeProcessProtocol &process, bool abridged) {
   Log *log = GetLog(LLDBLog::Process | LLDBLog::Thread);
 
   json::Array threads_array;
 
   // Ensure we can get info on the given thread.
   uint32_t thread_idx = 0;
   for (NativeThreadProtocol *thread;
        (thread = process.GetThreadAtIndex(thread_idx)) != nullptr;
        ++thread_idx) {
 
     lldb::tid_t tid = thread->GetID();
 
     // Grab the reason this thread stopped.
     struct ThreadStopInfo tid_stop_info;
     std::string description;
     if (!thread->GetStopReason(tid_stop_info, description))
       return llvm::make_error<llvm::StringError>(
           "failed to get stop reason", llvm::inconvertibleErrorCode());
 
-    const int signum = tid_stop_info.details.signal.signo;
+    const int signum = GetSignalForStopReason(tid_stop_info);
     if (log) {
       LLDB_LOGF(log,
                 "GDBRemoteCommunicationServerLLGS::%s pid %" PRIu64
                 " tid %" PRIu64
                 " got signal signo = %d, reason = %d, exc_type = %" PRIu64,
                 __FUNCTION__, process.GetID(), tid, signum,
                 tid_stop_info.reason, tid_stop_info.details.exception.type);
     }
GDBRemoteCommunicationServerLLGS::SendStopReplyPacketForThread(
   // if (tid_stop_info.reason == eStopReasonExec) {
   //     const bool force = true;
   //     InitializeRegisters(force);
   // }
 
   StreamString response;
   // Output the T packet with the thread
   response.PutChar('T');
-  int signum = tid_stop_info.details.signal.signo;
+  int signum = GetSignalForStopReason(tid_stop_info);
   LLDB_LOG(
       log,
       "pid {0}, tid {1}, got signal signo = {2}, reason = {3}, exc_type = {4}",
       m_current_process->GetID(), tid, signum, int(tid_stop_info.reason),
       tid_stop_info.details.exception.type);
 
   // Print the signal number.
   response.PutHex8(signum & 0xff);

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

def test_fork_multithreaded(self):
         self.build()
         self.prep_debug_monitor_and_inferior(inferior_args=["thread:new"]*2 + ["fork"])
         self.add_qSupported_packets(["multiprocess+", "fork-events+"])
         ret = self.expect_gdbremote_sequence()
         self.assertIn("fork-events+", ret["qSupported_response"])
         self.reset_test_sequence()
 
         # continue and expect fork
-        fork_regex = "[$]T.*;fork:p([0-9a-f]+)[.]([0-9a-f]+).*"
+        fork_regex = "[$]T05.*;fork:p([0-9a-f]+)[.]([0-9a-f]+).*"
         self.test_sequence.add_log_lines([
             "read packet: $c#00",
             {"direction": "send", "regex": fork_regex,
              "capture": {1: "pid", 2: "tid"}},
         ], True)
         ret = self.expect_gdbremote_sequence()
         pid = int(ret["pid"], 16)
         self.reset_test_sequence()
def fork_and_detach_test(self, variant):
         self.prep_debug_monitor_and_inferior(inferior_args=[variant])
         self.add_qSupported_packets(["multiprocess+",
                                      "{}-events+".format(variant)])
         ret = self.expect_gdbremote_sequence()
         self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
         self.reset_test_sequence()
 
         # continue and expect fork
-        fork_regex = "[$]T.*;{}:p([0-9a-f]+)[.]([0-9a-f]+).*".format(variant)
+        fork_regex = "[$]T05.*;{}:p([0-9a-f]+)[.]([0-9a-f]+).*".format(variant)
         self.test_sequence.add_log_lines([
             "read packet: $c#00",
             {"direction": "send", "regex": fork_regex,
              "capture": {1: "pid", 2: "tid"}},
         ], True)
         ret = self.expect_gdbremote_sequence()
         pid = int(ret["pid"], 16)
         self.reset_test_sequence()
class TestGdbRemoteFork(gdbremote_testcase.GdbRemoteTestCaseBase):
 
     @add_test_categories(["fork"])
     def test_vfork(self):
         self.fork_and_detach_test("vfork")
 
         # resume the parent
         self.test_sequence.add_log_lines([
             "read packet: $c#00",
-            {"direction": "send", "regex": r"[$]T.*vforkdone.*"},
+            {"direction": "send", "regex": r"[$]T05.*vforkdone.*"},
             "read packet: $c#00",
             {"direction": "send", "regex": r"[$]W00;process:[0-9a-f]+#.*"},
         ], True)
         self.expect_gdbremote_sequence()
 
     def fork_and_follow_test(self, variant):
         self.build()
         self.prep_debug_monitor_and_inferior(inferior_args=[variant])