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

mgorny · mgorny · commit d6b3de72566f · 2022-06-21T19:47:30.000+02:00
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 Differential Revision: https://reviews.llvm.org/D127193
diff --git a/lldb/include/lldb/Host/Debug.h b/lldb/include/lldb/Host/Debug.h
@@ -130,12 +130,8 @@ class ResumeActionList {
 
 struct ThreadStopInfo {
   lldb::StopReason reason;
+  uint32_t signo;
   union {
-    // eStopReasonSignal
-    struct {
-      uint32_t signo;
-    } signal;
-
     // eStopReasonException
     struct {
       uint64_t type;
diff --git a/lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp b/lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp
@@ -81,7 +81,7 @@ void NativeThreadFreeBSD::SetStoppedBySignal(uint32_t signo,
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonSignal;
-  m_stop_info.details.signal.signo = signo;
+  m_stop_info.signo = signo;
 
   m_stop_description.clear();
   if (info) {
@@ -100,19 +100,19 @@ void NativeThreadFreeBSD::SetStoppedBySignal(uint32_t signo,
 void NativeThreadFreeBSD::SetStoppedByBreakpoint() {
   SetStopped();
   m_stop_info.reason = StopReason::eStopReasonBreakpoint;
-  m_stop_info.details.signal.signo = SIGTRAP;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadFreeBSD::SetStoppedByTrace() {
   SetStopped();
   m_stop_info.reason = StopReason::eStopReasonTrace;
-  m_stop_info.details.signal.signo = SIGTRAP;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadFreeBSD::SetStoppedByExec() {
   SetStopped();
   m_stop_info.reason = StopReason::eStopReasonExec;
-  m_stop_info.details.signal.signo = SIGTRAP;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadFreeBSD::SetStoppedByWatchpoint(uint32_t wp_index) {
@@ -127,14 +127,15 @@ void NativeThreadFreeBSD::SetStoppedByWatchpoint(uint32_t wp_index) {
   SetStopped();
   m_stop_description = ostr.str();
   m_stop_info.reason = StopReason::eStopReasonWatchpoint;
-  m_stop_info.details.signal.signo = SIGTRAP;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadFreeBSD::SetStoppedByFork(lldb::pid_t child_pid,
                                            lldb::tid_t child_tid) {
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonFork;
+  m_stop_info.signo = SIGTRAP;
   m_stop_info.details.fork.child_pid = child_pid;
   m_stop_info.details.fork.child_tid = child_tid;
 }
@@ -144,6 +145,7 @@ void NativeThreadFreeBSD::SetStoppedByVFork(lldb::pid_t child_pid,
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonVFork;
+  m_stop_info.signo = SIGTRAP;
   m_stop_info.details.fork.child_pid = child_pid;
   m_stop_info.details.fork.child_tid = child_tid;
 }
@@ -152,13 +154,14 @@ void NativeThreadFreeBSD::SetStoppedByVForkDone() {
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonVForkDone;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadFreeBSD::SetStoppedWithNoReason() {
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonNone;
-  m_stop_info.details.signal.signo = 0;
+  m_stop_info.signo = 0;
 }
 
 void NativeThreadFreeBSD::SetStopped() {
diff --git a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -48,27 +48,27 @@ void LogThreadStopInfo(Log &log, const ThreadStopInfo &stop_info,
     return;
   case eStopReasonTrace:
     log.Printf("%s: %s trace, stopping signal 0x%" PRIx32, __FUNCTION__, header,
-               stop_info.details.signal.signo);
+               stop_info.signo);
     return;
   case eStopReasonBreakpoint:
     log.Printf("%s: %s breakpoint, stopping signal 0x%" PRIx32, __FUNCTION__,
-               header, stop_info.details.signal.signo);
+               header, stop_info.signo);
     return;
   case eStopReasonWatchpoint:
     log.Printf("%s: %s watchpoint, stopping signal 0x%" PRIx32, __FUNCTION__,
-               header, stop_info.details.signal.signo);
+               header, stop_info.signo);
     return;
   case eStopReasonSignal:
     log.Printf("%s: %s signal 0x%02" PRIx32, __FUNCTION__, header,
-               stop_info.details.signal.signo);
+               stop_info.signo);
     return;
   case eStopReasonException:
     log.Printf("%s: %s exception type 0x%02" PRIx64, __FUNCTION__, header,
                stop_info.details.exception.type);
     return;
   case eStopReasonExec:
     log.Printf("%s: %s exec, stopping signal 0x%" PRIx32, __FUNCTION__, header,
-               stop_info.details.signal.signo);
+               stop_info.signo);
     return;
   case eStopReasonPlanComplete:
     log.Printf("%s: %s plan complete", __FUNCTION__, header);
@@ -285,7 +285,7 @@ void NativeThreadLinux::SetStoppedBySignal(uint32_t signo,
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonSignal;
-  m_stop_info.details.signal.signo = signo;
+  m_stop_info.signo = signo;
 
   m_stop_description.clear();
   if (info) {
@@ -371,7 +371,7 @@ bool NativeThreadLinux::IsStopped(int *signo) {
   // If we are stopped by a signal, return the signo.
   if (signo && m_state == StateType::eStateStopped &&
       m_stop_info.reason == StopReason::eStopReasonSignal) {
-    *signo = m_stop_info.details.signal.signo;
+    *signo = m_stop_info.signo;
   }
 
   // Regardless, we are stopped.
@@ -398,14 +398,14 @@ void NativeThreadLinux::SetStoppedByExec() {
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonExec;
-  m_stop_info.details.signal.signo = SIGSTOP;
+  m_stop_info.signo = SIGSTOP;
 }
 
 void NativeThreadLinux::SetStoppedByBreakpoint() {
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonBreakpoint;
-  m_stop_info.details.signal.signo = SIGTRAP;
+  m_stop_info.signo = SIGTRAP;
   m_stop_description.clear();
 }
 
@@ -434,7 +434,7 @@ void NativeThreadLinux::SetStoppedByWatchpoint(uint32_t wp_index) {
   m_stop_description = ostr.str();
 
   m_stop_info.reason = StopReason::eStopReasonWatchpoint;
-  m_stop_info.details.signal.signo = SIGTRAP;
+  m_stop_info.signo = SIGTRAP;
 }
 
 bool NativeThreadLinux::IsStoppedAtBreakpoint() {
@@ -451,14 +451,15 @@ void NativeThreadLinux::SetStoppedByTrace() {
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonTrace;
-  m_stop_info.details.signal.signo = SIGTRAP;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadLinux::SetStoppedByFork(bool is_vfork, lldb::pid_t child_pid) {
   SetStopped();
 
   m_stop_info.reason =
       is_vfork ? StopReason::eStopReasonVFork : StopReason::eStopReasonFork;
+  m_stop_info.signo = SIGTRAP;
   m_stop_info.details.fork.child_pid = child_pid;
   m_stop_info.details.fork.child_tid = child_pid;
 }
@@ -467,21 +468,22 @@ void NativeThreadLinux::SetStoppedByVForkDone() {
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonVForkDone;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadLinux::SetStoppedWithNoReason() {
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonNone;
-  m_stop_info.details.signal.signo = 0;
+  m_stop_info.signo = 0;
 }
 
 void NativeThreadLinux::SetStoppedByProcessorTrace(
     llvm::StringRef description) {
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonProcessorTrace;
-  m_stop_info.details.signal.signo = 0;
+  m_stop_info.signo = 0;
   m_stop_description = description.str();
 }
 
diff --git a/lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp b/lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
@@ -81,7 +81,7 @@ void NativeThreadNetBSD::SetStoppedBySignal(uint32_t signo,
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonSignal;
-  m_stop_info.details.signal.signo = signo;
+  m_stop_info.signo = signo;
 
   m_stop_description.clear();
   if (info) {
@@ -100,19 +100,19 @@ void NativeThreadNetBSD::SetStoppedBySignal(uint32_t signo,
 void NativeThreadNetBSD::SetStoppedByBreakpoint() {
   SetStopped();
   m_stop_info.reason = StopReason::eStopReasonBreakpoint;
-  m_stop_info.details.signal.signo = SIGTRAP;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadNetBSD::SetStoppedByTrace() {
   SetStopped();
   m_stop_info.reason = StopReason::eStopReasonTrace;
-  m_stop_info.details.signal.signo = SIGTRAP;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadNetBSD::SetStoppedByExec() {
   SetStopped();
   m_stop_info.reason = StopReason::eStopReasonExec;
-  m_stop_info.details.signal.signo = SIGTRAP;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadNetBSD::SetStoppedByWatchpoint(uint32_t wp_index) {
@@ -127,14 +127,15 @@ void NativeThreadNetBSD::SetStoppedByWatchpoint(uint32_t wp_index) {
   SetStopped();
   m_stop_description = ostr.str();
   m_stop_info.reason = StopReason::eStopReasonWatchpoint;
-  m_stop_info.details.signal.signo = SIGTRAP;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadNetBSD::SetStoppedByFork(lldb::pid_t child_pid,
                                            lldb::tid_t child_tid) {
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonFork;
+  m_stop_info.signo = SIGTRAP;
   m_stop_info.details.fork.child_pid = child_pid;
   m_stop_info.details.fork.child_tid = child_tid;
 }
@@ -144,6 +145,7 @@ void NativeThreadNetBSD::SetStoppedByVFork(lldb::pid_t child_pid,
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonVFork;
+  m_stop_info.signo = SIGTRAP;
   m_stop_info.details.fork.child_pid = child_pid;
   m_stop_info.details.fork.child_tid = child_tid;
 }
@@ -152,13 +154,14 @@ void NativeThreadNetBSD::SetStoppedByVForkDone() {
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonVForkDone;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadNetBSD::SetStoppedWithNoReason() {
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonNone;
-  m_stop_info.details.signal.signo = 0;
+  m_stop_info.signo = 0;
 }
 
 void NativeThreadNetBSD::SetStopped() {
diff --git a/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
@@ -253,13 +253,12 @@ void NativeProcessWindows::SetStopReasonForThread(NativeThreadWindows &thread,
 
   ThreadStopInfo stop_info;
   stop_info.reason = reason;
-
   // No signal support on Windows but required to provide a 'valid' signum.
+  stop_info.signo = SIGTRAP;
+
   if (reason == StopReason::eStopReasonException) {
     stop_info.details.exception.type = 0;
     stop_info.details.exception.data_count = 0;
-  } else {
-    stop_info.details.signal.signo = SIGTRAP;
   }
 
   thread.SetStopReason(stop_info, description);
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -728,7 +728,7 @@ GetJSONThreadsInfo(NativeProcessProtocol &process, bool abridged) {
       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 = tid_stop_info.signo;
     if (log) {
       LLDB_LOGF(log,
                 "GDBRemoteCommunicationServerLLGS::%s pid %" PRIu64
@@ -804,7 +804,7 @@ GDBRemoteCommunicationServerLLGS::PrepareStopReplyPacketForThread(
 
   // Output the T packet with the thread
   response.PutChar('T');
-  int signum = tid_stop_info.details.signal.signo;
+  int signum = tid_stop_info.signo;
   LLDB_LOG(
       log,
       "pid {0}, tid {1}, got signal signo = {2}, reason = {3}, exc_type = {4}",
diff --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -15,7 +15,7 @@ def test_fork_multithreaded(self):
         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,
@@ -49,7 +49,7 @@ def fork_and_detach_test(self, variant):
         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,
@@ -85,7 +85,7 @@ def test_vfork(self):
         # 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)
diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -2704,7 +2704,7 @@ rnb_err_t RNBRemote::SendStopReplyPacketForThread(nub_thread_t tid) {
     std::ostringstream ostrm;
     // Output the T packet with the thread
     ostrm << 'T';
-    int signum = tid_stop_info.details.signal.signo;
+    int signum = tid_stop_info.signo;
     DNBLogThreadedIf(
         LOG_RNB_PROC, "%8d %s got signal signo = %u, exc_type = %u",
         (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true), __FUNCTION__,
@@ -5450,9 +5450,9 @@ RNBRemote::GetJSONThreadsInfo(bool threads_with_valid_stop_info_only) {
           break;
 
         case eStopTypeSignal:
-          if (tid_stop_info.details.signal.signo != 0) {
+          if (tid_stop_info.signo != 0) {
             thread_dict_sp->AddIntegerItem("signal",
-                                           tid_stop_info.details.signal.signo);
+                                           tid_stop_info.signo);
             reason_value = "signal";
           }
           break;
