[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

710

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

710

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/include/lldb/Host/Debug.h

 
 protected:
   std::vector<ResumeAction> m_actions;
   mutable std::vector<bool> m_signal_handled;
 };
 
 struct ThreadStopInfo {
   lldb::StopReason reason;
-  union {
-    // eStopReasonSignal
-    struct {
-      uint32_t signo;
-    } signal;
-
+  union {
     // eStopReasonException
     struct {
       uint64_t type;
       uint32_t data_count;
       lldb::addr_t data[8];
     } exception;
 
     // eStopReasonFork / eStopReasonVFork

lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp

 void NativeThreadFreeBSD::SetStoppedBySignal(uint32_t signo,
                                              const siginfo_t *info) {
   Log *log = GetLog(POSIXLog::Thread);
   LLDB_LOG(log, "tid = {0} in called with signal {1}", GetID(), 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) {
     switch (signo) {
     case SIGSEGV:
     case SIGBUS:
     case SIGFPE:
     case SIGILL:
       const auto reason = GetCrashReason(*info);
       m_stop_description = GetCrashReasonString(reason, *info);
       break;
     }
   }
 }
 
 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) {
   lldbassert(wp_index != LLDB_INVALID_INDEX32 && "wp_index cannot be invalid");
 
   std::ostringstream ostr;
   ostr << GetRegisterContext().GetWatchpointAddress(wp_index) << " ";
   ostr << wp_index;
 
   ostr << " " << GetRegisterContext().GetWatchpointHitAddress(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;
 }
 
 void NativeThreadFreeBSD::SetStoppedByVFork(lldb::pid_t child_pid,
                                             lldb::tid_t child_tid) {
   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;
 }
 
 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() {
   const StateType new_state = StateType::eStateStopped;
   m_state = new_state;
   m_stop_description.clear();
 }
 

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

 void LogThreadStopInfo(Log &log, const ThreadStopInfo &stop_info,
                        const char *const header) {
   switch (stop_info.reason) {
   case eStopReasonNone:
     log.Printf("%s: %s no stop reason", __FUNCTION__, header);
     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);
     return;
   case eStopReasonThreadExiting:
     log.Printf("%s: %s thread exiting", __FUNCTION__, header);
     return;
   case eStopReasonInstrumentation:
void NativeThreadLinux::SetStoppedBySignal(uint32_t signo,
                                            const siginfo_t *info) {
   Log *log = GetLog(LLDBLog::Thread);
   LLDB_LOGF(log, "NativeThreadLinux::%s called with signal 0x%02" PRIx32,
             __FUNCTION__, 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) {
     switch (signo) {
     case SIGSEGV:
     case SIGBUS:
     case SIGFPE:
     case SIGILL:

 
 bool NativeThreadLinux::IsStopped(int *signo) {
   if (!StateIsStoppedState(m_state, false))
     return false;
 
   // 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.
   return true;
 }
 
 void NativeThreadLinux::SetStopped() {
   if (m_state == StateType::eStateStepping)

 
 void NativeThreadLinux::SetStoppedByExec() {
   Log *log = GetLog(LLDBLog::Thread);
   LLDB_LOGF(log, "NativeThreadLinux::%s()", __FUNCTION__);
 
   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();
 }
 
 void NativeThreadLinux::SetStoppedByWatchpoint(uint32_t wp_index) {
   SetStopped();
 
   lldbassert(wp_index != LLDB_INVALID_INDEX32 && "wp_index cannot be invalid");
 
void NativeThreadLinux::SetStoppedByWatchpoint(uint32_t wp_index) {
    * stop-info
    * packet.
   */
   ostr << " " << m_reg_context_up->GetWatchpointHitAddress(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() {
   return GetState() == StateType::eStateStopped &&
          m_stop_info.reason == StopReason::eStopReasonBreakpoint;
 }
 
 bool NativeThreadLinux::IsStoppedAtWatchpoint() {
   return GetState() == StateType::eStateStopped &&
          m_stop_info.reason == StopReason::eStopReasonWatchpoint;
 }
 
 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;
 }
 
 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();
 }
 
 void NativeThreadLinux::SetExited() {
   const StateType new_state = StateType::eStateExited;
   MaybeLogStateChange(new_state);
   m_state = new_state;
 

lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp

 void NativeThreadNetBSD::SetStoppedBySignal(uint32_t signo,
                                             const siginfo_t *info) {
   Log *log = GetLog(POSIXLog::Thread);
   LLDB_LOG(log, "tid = {0} in called with signal {1}", GetID(), 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) {
     switch (signo) {
     case SIGSEGV:
     case SIGBUS:
     case SIGFPE:
     case SIGILL:
       const auto reason = GetCrashReason(*info);
       m_stop_description = GetCrashReasonString(reason, *info);
       break;
     }
   }
 }
 
 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) {
   lldbassert(wp_index != LLDB_INVALID_INDEX32 && "wp_index cannot be invalid");
 
   std::ostringstream ostr;
   ostr << GetRegisterContext().GetWatchpointAddress(wp_index) << " ";
   ostr << wp_index;
 
   ostr << " " << GetRegisterContext().GetWatchpointHitAddress(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;
 }
 
 void NativeThreadNetBSD::SetStoppedByVFork(lldb::pid_t child_pid,
                                             lldb::tid_t child_tid) {
   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;
 }
 
 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() {
   const StateType new_state = StateType::eStateStopped;
   m_state = new_state;
   m_stop_description.clear();
 }
 

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

 
 void NativeProcessWindows::SetStopReasonForThread(NativeThreadWindows &thread,
                                                   lldb::StopReason reason,
                                                   std::string description) {
   SetCurrentThreadID(thread.GetID());
 
   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);
 }
 
 void NativeProcessWindows::StopThread(lldb::tid_t thread_id,
                                       lldb::StopReason reason,
                                       std::string description) {

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 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 = tid_stop_info.signo;
     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::PrepareStopReplyPacketForThread(
   // FIXME implement register handling for exec'd inferiors.
   // if (tid_stop_info.reason == eStopReasonExec) {
   //     const bool force = true;
   //     InitializeRegisters(force);
   // }
 
   // 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}",
       process.GetID(), thread.GetID(), 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])

lldb/tools/debugserver/source/RNBRemote.cpp

if (did_exec) {
       // Reset any symbols that need resetting when we exec
       m_dispatch_queue_offsets_addr = INVALID_NUB_ADDRESS;
       m_dispatch_queue_offsets.Clear();
     }
 
     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__,
         signum, tid_stop_info.details.exception.type);
 
     // Translate any mach exceptions to gdb versions, unless they are
     // common exceptions like a breakpoint or a soft signal.
     switch (tid_stop_info.details.exception.type) {
for (nub_size_t i = 0; i < numthreads; ++i) {
       std::string reason_value("none");
 
       if (stop_info_valid) {
         switch (tid_stop_info.reason) {
         case eStopTypeInvalid:
           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;
 
         case eStopTypeException:
           if (tid_stop_info.details.exception.type != 0) {
             reason_value = "exception";
             thread_dict_sp->AddIntegerItem(