Rewrite gdb-remote's SendContinuePacketAndWaitForResponse

SendContinuePacketAndWaitForResponse was huge function with very complex interactions with
several other functions (SendAsyncSignal, SendInterrupt, SendPacket). This meant that making any
changes to how packet sending functions and threads interact was very difficult and error-prone.

This change does not add any functionality yet, it merely paves the way for future changes. In a
follow-up, I plan to add the ability to have multiple query packets in flight (i.e.,
request,request,response,response instead of the usual request,response sequences) and use that
to speed up qModuleInfo packet processing.

Here, I introduce two special kinds of locks: ContinueLock, which is used by the continue thread,
and Lock, which is used by everyone else. ContinueLock (atomically) sends a continue packet, and
blocks any other async threads from accessing the connection. Other threads create an instance of
the Lock object when they want to access the connection. This object, while in scope prevents the
continue from being send. Optionally, it can also interrupt the process to gain access to the
connection for async processing.

Most of the syncrhonization logic is encapsulated within these two classes. Some of it still
had to bleed over into the SendContinuePacketAndWaitForResponse, but the function is still much
more manageable than before -- partly because of most of the work is done in the ContinueLock
class, and partly because I have factored out a lot of the packet processing code separate
functions (this also makes the functionality more easily testable). Most importantly, there is
none of syncrhonization code in the async thread users -- as far as they are concerned, they just
need to declare a Lock object, and they are good to go (SendPacketAndWaitForResponse is now a
very thin wrapper around the NoLock version of the function, whereas previously it had over 100
lines of synchronization code).  This will make my follow up changes there easy.

I have written a number of unit tests for the new code and I have ran the test suite on linux and
osx with no regressions.

Subscribers: tberghammer

Differential Revision: https://reviews.llvm.org/D22629

llvm-svn: 277139

1 files changed, 6 insertions, 57 deletions

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index a603fcf..424a949 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -12,6 +12,7 @@
 
 // C Includes
 // C++ Includes
+#include <chrono>
 #include <map>
 #include <mutex>
 #include <string>
@@ -23,12 +24,12 @@
 #include "lldb/Core/StructuredData.h"
 #include "lldb/Target/Process.h"
 
-#include "GDBRemoteCommunication.h"
+#include "GDBRemoteClientBase.h"
 
 namespace lldb_private {
 namespace process_gdb_remote {
 
-class GDBRemoteCommunicationClient : public GDBRemoteCommunication
+class GDBRemoteCommunicationClient : public GDBRemoteClientBase
 {
 public:
     GDBRemoteCommunicationClient();
@@ -42,17 +43,6 @@ public:
     bool
     HandshakeWithServer (Error *error_ptr);
 
-    PacketResult
-    SendPacketAndWaitForResponse (const char *send_payload,
-                                  StringExtractorGDBRemote &response,
-                                  bool send_async);
-
-    PacketResult
-    SendPacketAndWaitForResponse (const char *send_payload,
-                                  size_t send_length,
-                                  StringExtractorGDBRemote &response,
-                                  bool send_async);
-
     // For packets which specify a range of output to be returned,
     // return all of the output via a series of request packets of the form
     // <prefix>0,<size>
@@ -74,18 +64,6 @@ public:
     SendPacketsAndConcatenateResponses (const char *send_payload_prefix,
                                         std::string &response_string);
 
-    lldb::StateType
-    SendContinuePacketAndWaitForResponse (ProcessGDBRemote *process,
-                                          const char *packet_payload,
-                                          size_t packet_length,
-                                          StringExtractorGDBRemote &response);
-
-    bool
-    SendvContPacket (ProcessGDBRemote *process,
-                     const char *payload,
-                     size_t packet_length,
-                     StringExtractorGDBRemote &response);
-
     bool
     GetThreadSuffixSupported () override;
 
@@ -101,12 +79,6 @@ public:
     void
     GetListThreadsInStopReplySupported ();
 
-    bool
-    SendAsyncSignal (int signo);
-
-    bool
-    SendInterrupt(std::unique_lock<std::recursive_mutex> &lock, uint32_t seconds_to_wait_for_stop, bool &timed_out);
-
     lldb::pid_t
     GetCurrentProcessID (bool allow_lazy = true);
 
@@ -461,12 +433,6 @@ public:
     GetCurrentThreadIDs (std::vector<lldb::tid_t> &thread_ids,
                          bool &sequence_mutex_unavailable);
     
-    bool
-    GetInterruptWasSent () const
-    {
-        return m_interrupt_sent;
-    }
-    
     lldb::user_id_t
     OpenFile (const FileSpec& file_spec, uint32_t flags, mode_t mode, Error &error);
     
@@ -520,9 +486,6 @@ public:
     bool
     CalculateMD5 (const FileSpec& file_spec, uint64_t &high, uint64_t &low);
     
-    std::string
-    HarmonizeThreadIdsForProfileData (ProcessGDBRemote *process,
-                                      StringExtractorGDBRemote &inputStringExtractor);
 
     bool
     ReadRegister(lldb::tid_t tid,
@@ -632,18 +595,6 @@ protected:
 
     uint32_t m_num_supported_hardware_watchpoints;
 
-    // If we need to send a packet while the target is running, the m_async_XXX
-    // member variables take care of making this happen.
-    std::recursive_mutex m_async_mutex;
-    Predicate<bool> m_async_packet_predicate;
-    std::string m_async_packet;
-    PacketResult m_async_result;
-    StringExtractorGDBRemote m_async_response;
-    int m_async_signal; // We were asked to deliver a signal to the inferior process.
-    bool m_interrupt_sent;
-    std::string m_partial_profile_data;
-    std::map<uint64_t, uint32_t> m_thread_id_to_used_usec_map;
-    
     ArchSpec m_host_arch;
     ArchSpec m_process_arch;
     uint32_t m_os_version_major;
@@ -657,11 +608,6 @@ protected:
     uint32_t m_default_packet_timeout;
     uint64_t m_max_packet_size;  // as returned by qSupported
 
-    PacketResult
-    SendPacketAndWaitForResponseNoLock (const char *payload,
-                                        size_t payload_length,
-                                        StringExtractorGDBRemote &response);
-
     bool
     GetCurrentProcessInfo (bool allow_lazy_pid = true);
 
@@ -677,6 +623,9 @@ protected:
     DecodeProcessInfoResponse (StringExtractorGDBRemote &response, 
                                ProcessInstanceInfo &process_info);
 
+    void
+    OnRunPacketSent(bool first) override;
+
 private:
     DISALLOW_COPY_AND_ASSIGN (GDBRemoteCommunicationClient);
 };