[Lldb-commits] [PATCH] D90525: [lldb] [Process/FreeBSDRemote] Fix attaching via lldb-server

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: krytarowski, emaste, labath.
Herald added a subscriber: arichardson.
mgorny requested review of this revision.

Fix two bugs that caused attaching to a process in a pre-connected
lldb-server to fail.  These are:

1. Prematurely reporting status in NativeProcessFreeBSD::Attach(). The 
SetState() call defaulted to notify the process, and LLGS tried to send the 
stopped packet before the process instance was assigned to it.  While at it, 
add an assert for that in LLGS.

2. Duplicate call to ReinitializeThreads() (via SetupTrace()) that overwrote 
the stopped status in threads.  Now SetupTrace() is called directly by 
NativeProcessFreeBSD::Attach() (not the Factory) in place of 
ReinitializeThreads().

This fixes at least commands/process/attach/TestProcessAttach.py
and python_api/hello_world/TestHelloWorld.py.


https://reviews.llvm.org/D90525

Files:
  lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp


Index: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1728,6 +1728,7 @@
   case eStateSuspended:
   case eStateStopped:
   case eStateCrashed: {
+    assert(m_debugged_process_up != nullptr);
     lldb::tid_t tid = m_debugged_process_up->GetCurrentThreadID();
     // Make sure we set the current thread so g and p packets return the data
     // the gdb will expect.
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
===================================================================
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -125,10 +125,6 @@
   if (!status.Success())
     return status.ToError();
 
-  status = process_up->SetupTrace();
-  if (status.Fail())
-    return status.ToError();
-
   return std::move(process_up);
 }
 
@@ -698,8 +694,9 @@
       0)
     return Status(errno, eErrorTypePOSIX);
 
-  /* Initialize threads */
-  status = ReinitializeThreads();
+  // Initialize threads and tracing status
+  // NB: this needs to be called before we set thread state
+  status = SetupTrace();
   if (status.Fail())
     return status;
 
@@ -707,7 +704,8 @@
     static_cast<NativeThreadFreeBSD &>(*thread).SetStoppedBySignal(SIGSTOP);
 
   // Let our process instance know the thread has stopped.
-  SetState(StateType::eStateStopped);
+  SetCurrentThreadID(m_threads.front()->GetID());
+  SetState(StateType::eStateStopped, false);
   return Status();
 }
 



Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1728,6 +1728,7 @@
   case eStateSuspended:
   case eStateStopped:
   case eStateCrashed: {
+    assert(m_debugged_process_up != nullptr);
     lldb::tid_t tid = m_debugged_process_up->GetCurrentThreadID();
     // Make sure we set the current thread so g and p packets return the data
     // the gdb will expect.
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
===================================================================
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -125,10 +125,6 @@
   if (!status.Success())
     return status.ToError();
 
-  status = process_up->SetupTrace();
-  if (status.Fail())
-    return status.ToError();
-
   return std::move(process_up);
 }
 
@@ -698,8 +694,9 @@
       0)
     return Status(errno, eErrorTypePOSIX);
 
-  /* Initialize threads */
-  status = ReinitializeThreads();
+  // Initialize threads and tracing status
+  // NB: this needs to be called before we set thread state
+  status = SetupTrace();
   if (status.Fail())
     return status;
 
@@ -707,7 +704,8 @@
     static_cast<NativeThreadFreeBSD &>(*thread).SetStoppedBySignal(SIGSTOP);
 
   // Let our process instance know the thread has stopped.
-  SetState(StateType::eStateStopped);
+  SetCurrentThreadID(m_threads.front()->GetID());
+  SetState(StateType::eStateStopped, false);
   return Status();
 }
 

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits