Author: labath Date: Fri Jul 29 08:10:02 2016 New Revision: 277139 URL: http://llvm.org/viewvc/llvm-project?rev=277139&view=rev Log: 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 Added: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h lldb/trunk/unittests/Process/ lldb/trunk/unittests/Process/CMakeLists.txt lldb/trunk/unittests/Process/gdb-remote/ lldb/trunk/unittests/Process/gdb-remote/CMakeLists.txt lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp Modified: lldb/trunk/source/Plugins/Process/gdb-remote/CMakeLists.txt lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h lldb/trunk/source/Utility/StringExtractor.cpp lldb/trunk/unittests/CMakeLists.txt Modified: lldb/trunk/source/Plugins/Process/gdb-remote/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/CMakeLists.txt?rev=277139&r1=277138&r2=277139&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/CMakeLists.txt (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/CMakeLists.txt Fri Jul 29 08:10:02 2016 @@ -3,6 +3,7 @@ if (CMAKE_SYSTEM_NAME MATCHES "Darwin") endif() add_lldb_library(lldbPluginProcessGDBRemote + GDBRemoteClientBase.cpp GDBRemoteCommunication.cpp GDBRemoteCommunicationClient.cpp GDBRemoteCommunicationServer.cpp Added: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp?rev=277139&view=auto ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp (added) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp Fri Jul 29 08:10:02 2016 @@ -0,0 +1,378 @@ +//===-- GDBRemoteClientBase.cpp ---------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "GDBRemoteClientBase.h" + +#include "llvm/ADT/StringExtras.h" + +#include "lldb/Target/UnixSignals.h" +#include "lldb/Utility/LLDBAssert.h" + +#include "ProcessGDBRemoteLog.h" + +using namespace lldb; +using namespace lldb_private; +using namespace lldb_private::process_gdb_remote; + +static const std::chrono::seconds kInterruptTimeout(5); + +///////////////////////// +// GDBRemoteClientBase // +///////////////////////// + +GDBRemoteClientBase::ContinueDelegate::~ContinueDelegate() = default; + +GDBRemoteClientBase::GDBRemoteClientBase(const char *comm_name, const char *listener_name) + : GDBRemoteCommunication(comm_name, listener_name), m_async_count(0), m_is_running(false), m_should_stop(false) +{ +} + +StateType +GDBRemoteClientBase::SendContinuePacketAndWaitForResponse(ContinueDelegate &delegate, const UnixSignals &signals, + llvm::StringRef payload, StringExtractorGDBRemote &response) +{ + Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); + response.Clear(); + + m_continue_packet = payload; + ContinueLock cont_lock(*this); + if (!cont_lock) + return eStateInvalid; + OnRunPacketSent(true); + + for (;;) + { + PacketResult read_result = ReadPacket( + response, std::chrono::duration_cast<std::chrono::microseconds>(kInterruptTimeout).count(), false); + switch (read_result) + { + case PacketResult::ErrorReplyTimeout: + { + std::lock_guard<std::mutex> lock(m_mutex); + if (m_async_count == 0) + continue; + if (std::chrono::steady_clock::now() >= m_interrupt_time + kInterruptTimeout) + return eStateInvalid; + } + case PacketResult::Success: + break; + default: + if (log) + log->Printf("GDBRemoteClientBase::%s () ReadPacket(...) => false", __FUNCTION__); + return eStateInvalid; + } + if (response.Empty()) + return eStateInvalid; + + const char stop_type = response.GetChar(); + if (log) + log->Printf("GDBRemoteClientBase::%s () got packet: %s", __FUNCTION__, response.GetStringRef().c_str()); + + switch (stop_type) + { + case 'W': + case 'X': + return eStateExited; + case 'E': + // ERROR + return eStateInvalid; + default: + if (log) + log->Printf("GDBRemoteClientBase::%s () unrecognized async packet", __FUNCTION__); + return eStateInvalid; + case 'O': + { + std::string inferior_stdout; + response.GetHexByteString(inferior_stdout); + delegate.HandleAsyncStdout(inferior_stdout); + break; + } + case 'A': + delegate.HandleAsyncMisc(llvm::StringRef(response.GetStringRef()).substr(1)); + break; + case 'T': + case 'S': + // Do this with the continue lock held. + const bool should_stop = ShouldStop(signals, response); + response.SetFilePos(0); + + // The packet we should resume with. In the future + // we should check our thread list and "do the right thing" + // for new threads that show up while we stop and run async + // packets. Setting the packet to 'c' to continue all threads + // is the right thing to do 99.99% of the time because if a + // thread was single stepping, and we sent an interrupt, we + // will notice above that we didn't stop due to an interrupt + // but stopped due to stepping and we would _not_ continue. + // This packet may get modified by the async actions (e.g. to send a signal). + m_continue_packet = 'c'; + cont_lock.unlock(); + + delegate.HandleStopReply(); + if (should_stop) + return eStateStopped; + + switch (cont_lock.lock()) + { + case ContinueLock::LockResult::Success: + break; + case ContinueLock::LockResult::Failed: + return eStateInvalid; + case ContinueLock::LockResult::Cancelled: + return eStateStopped; + } + OnRunPacketSent(false); + break; + } + } +} + +bool +GDBRemoteClientBase::SendAsyncSignal(int signo) +{ + Lock lock(*this, true); + if (!lock || !lock.DidInterrupt()) + return false; + + m_continue_packet = 'C'; + m_continue_packet += llvm::hexdigit((signo / 16) % 16); + m_continue_packet += llvm::hexdigit(signo % 16); + return true; +} + +bool +GDBRemoteClientBase::Interrupt() +{ + Lock lock(*this, true); + if (!lock.DidInterrupt()) + return false; + m_should_stop = true; + return true; +} +GDBRemoteCommunication::PacketResult +GDBRemoteClientBase::SendPacketAndWaitForResponse(llvm::StringRef payload, StringExtractorGDBRemote &response, + bool send_async) +{ + Lock lock(*this, send_async); + if (!lock) + { + if (Log *log = ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)) + log->Printf("GDBRemoteClientBase::%s failed to get mutex, not sending packet '%.*s' (send_async=%d)", + __FUNCTION__, int(payload.size()), payload.data(), send_async); + return PacketResult::ErrorSendFailed; + } + + return SendPacketAndWaitForResponseNoLock(payload, response); +} + +GDBRemoteCommunication::PacketResult +GDBRemoteClientBase::SendPacketAndWaitForResponseNoLock(llvm::StringRef payload, StringExtractorGDBRemote &response) +{ + PacketResult packet_result = SendPacketNoLock(payload.data(), payload.size()); + if (packet_result != PacketResult::Success) + return packet_result; + + const size_t max_response_retries = 3; + for (size_t i = 0; i < max_response_retries; ++i) + { + packet_result = ReadPacket(response, GetPacketTimeoutInMicroSeconds(), true); + // Make sure we received a response + if (packet_result != PacketResult::Success) + return packet_result; + // Make sure our response is valid for the payload that was sent + if (response.ValidateResponse()) + return packet_result; + // Response says it wasn't valid + Log *log = ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PACKETS); + if (log) + log->Printf("error: packet with payload \"%.*s\" got invalid response \"%s\": %s", int(payload.size()), + payload.data(), response.GetStringRef().c_str(), + (i == (max_response_retries - 1)) ? "using invalid response and giving up" + : "ignoring response and waiting for another"); + } + return packet_result; +} + +bool +GDBRemoteClientBase::SendvContPacket(llvm::StringRef payload, StringExtractorGDBRemote &response) +{ + Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); + if (log) + log->Printf("GDBRemoteCommunicationClient::%s ()", __FUNCTION__); + + // we want to lock down packet sending while we continue + Lock lock(*this, true); + + if (log) + log->Printf("GDBRemoteCommunicationClient::%s () sending vCont packet: %.*s", __FUNCTION__, int(payload.size()), + payload.data()); + + if (SendPacketNoLock(payload.data(), payload.size()) != PacketResult::Success) + return false; + + OnRunPacketSent(true); + + // wait for the response to the vCont + if (ReadPacket(response, UINT32_MAX, false) == PacketResult::Success) + { + if (response.IsOKResponse()) + return true; + } + + return false; +} +bool +GDBRemoteClientBase::ShouldStop(const UnixSignals &signals, StringExtractorGDBRemote &response) +{ + std::lock_guard<std::mutex> lock(m_mutex); + + if (m_async_count == 0) + return true; // We were not interrupted. The process stopped on its own. + + // Older debugserver stubs (before April 2016) can return two + // stop-reply packets in response to a ^C packet. + // Additionally, all debugservers still return two stop replies if + // the inferior stops due to some other reason before the remote + // stub manages to interrupt it. We need to wait for this + // additional packet to make sure the packet sequence does not get + // skewed. + StringExtractorGDBRemote extra_stop_reply_packet; + uint32_t timeout_usec = 100000; // 100ms + ReadPacket(extra_stop_reply_packet, timeout_usec, false); + + // Interrupting is typically done using SIGSTOP or SIGINT, so if + // the process stops with some other signal, we definitely want to + // stop. + const uint8_t signo = response.GetHexU8(UINT8_MAX); + if (signo != signals.GetSignalNumberFromName("SIGSTOP") && signo != signals.GetSignalNumberFromName("SIGINT")) + return true; + + // We probably only stopped to perform some async processing, so continue after that is done. + // TODO: This is not 100% correct, as the process may have been stopped with SIGINT or SIGSTOP + // that was not caused by us (e.g. raise(SIGINT)). This will normally cause a stop, but if it's + // done concurrently with a async interrupt, that stop will get eaten (llvm.org/pr20231). + return false; +} + +void +GDBRemoteClientBase::OnRunPacketSent(bool first) +{ + if (first) + BroadcastEvent(eBroadcastBitRunPacketSent, NULL); +} + +/////////////////////////////////////// +// GDBRemoteClientBase::ContinueLock // +/////////////////////////////////////// + +GDBRemoteClientBase::ContinueLock::ContinueLock(GDBRemoteClientBase &comm) : m_comm(comm), m_acquired(false) +{ + lock(); +} + +GDBRemoteClientBase::ContinueLock::~ContinueLock() +{ + if (m_acquired) + unlock(); +} + +void +GDBRemoteClientBase::ContinueLock::unlock() +{ + lldbassert(m_acquired); + { + std::unique_lock<std::mutex> lock(m_comm.m_mutex); + m_comm.m_is_running = false; + } + m_comm.m_cv.notify_all(); + m_acquired = false; +} + +GDBRemoteClientBase::ContinueLock::LockResult +GDBRemoteClientBase::ContinueLock::lock() +{ + Log *log = ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS); + if (log) + log->Printf("GDBRemoteClientBase::ContinueLock::%s() resuming with %s", __FUNCTION__, + m_comm.m_continue_packet.c_str()); + + lldbassert(!m_acquired); + std::unique_lock<std::mutex> lock(m_comm.m_mutex); + m_comm.m_cv.wait(lock, [this] { return m_comm.m_async_count == 0; }); + if (m_comm.m_should_stop) + { + m_comm.m_should_stop = false; + return LockResult::Cancelled; + } + if (m_comm.SendPacketNoLock(m_comm.m_continue_packet.data(), m_comm.m_continue_packet.size()) != + PacketResult::Success) + return LockResult::Failed; + + lldbassert(!m_comm.m_is_running); + m_comm.m_is_running = true; + m_acquired = true; + return LockResult::Success; +} + +/////////////////////////////// +// GDBRemoteClientBase::Lock // +/////////////////////////////// + +GDBRemoteClientBase::Lock::Lock(GDBRemoteClientBase &comm, bool interrupt) + : m_async_lock(comm.m_async_mutex, std::defer_lock), m_comm(comm), m_acquired(false), m_did_interrupt(false) +{ + SyncWithContinueThread(interrupt); + if (m_acquired) + m_async_lock.lock(); +} + +void +GDBRemoteClientBase::Lock::SyncWithContinueThread(bool interrupt) +{ + Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); + std::unique_lock<std::mutex> lock(m_comm.m_mutex); + if (m_comm.m_is_running && !interrupt) + return; // We were asked to avoid interrupting the sender. Lock is not acquired. + + ++m_comm.m_async_count; + if (m_comm.m_is_running) + { + if (m_comm.m_async_count == 1) + { + // The sender has sent the continue packet and we are the first async packet. Let's interrupt it. + const char ctrl_c = '\x03'; + ConnectionStatus status = eConnectionStatusSuccess; + size_t bytes_written = m_comm.Write(&ctrl_c, 1, status, NULL); + if (bytes_written == 0) + { + --m_comm.m_async_count; + if (log) + log->Printf("GDBRemoteClientBase::Lock::Lock failed to send interrupt packet"); + return; + } + if (log) + log->PutCString("GDBRemoteClientBase::Lock::Lock sent packet: \\x03"); + m_comm.m_interrupt_time = std::chrono::steady_clock::now(); + } + m_comm.m_cv.wait(lock, [this] { return m_comm.m_is_running == false; }); + m_did_interrupt = true; + } + m_acquired = true; +} + +GDBRemoteClientBase::Lock::~Lock() +{ + if (!m_acquired) + return; + { + std::unique_lock<std::mutex> lock(m_comm.m_mutex); + --m_comm.m_async_count; + } + m_comm.m_cv.notify_one(); +} Added: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h?rev=277139&view=auto ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h (added) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h Fri Jul 29 08:10:02 2016 @@ -0,0 +1,153 @@ +//===-- GDBRemoteClientBase.h -----------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef liblldb_GDBRemoteClientBase_h_ +#define liblldb_GDBRemoteClientBase_h_ + +#include "GDBRemoteCommunication.h" + +#include <condition_variable> + +namespace lldb_private +{ +namespace process_gdb_remote +{ + +class GDBRemoteClientBase : public GDBRemoteCommunication +{ +public: + struct ContinueDelegate + { + virtual ~ContinueDelegate(); + virtual void + HandleAsyncStdout(llvm::StringRef out) = 0; + virtual void + HandleAsyncMisc(llvm::StringRef data) = 0; + virtual void + HandleStopReply() = 0; + }; + + GDBRemoteClientBase(const char *comm_name, const char *listener_name); + + bool + SendAsyncSignal(int signo); + + bool + Interrupt(); + + lldb::StateType + SendContinuePacketAndWaitForResponse(ContinueDelegate &delegate, const UnixSignals &signals, + llvm::StringRef payload, StringExtractorGDBRemote &response); + + PacketResult + SendPacketAndWaitForResponse(const char *payload, size_t len, StringExtractorGDBRemote &response, bool send_async) + { + return SendPacketAndWaitForResponse(llvm::StringRef(payload, len), response, send_async); + } + + PacketResult + SendPacketAndWaitForResponse(llvm::StringRef payload, StringExtractorGDBRemote &response, bool send_async); + + bool + SendvContPacket(llvm::StringRef payload, StringExtractorGDBRemote &response); + + class Lock + { + public: + Lock(GDBRemoteClientBase &comm, bool interrupt); + ~Lock(); + + explicit operator bool() { return m_acquired; } + + // Whether we had to interrupt the continue thread to acquire the connection. + bool + DidInterrupt() const + { + return m_did_interrupt; + } + + private: + std::unique_lock<std::recursive_mutex> m_async_lock; + GDBRemoteClientBase &m_comm; + bool m_acquired; + bool m_did_interrupt; + + void + SyncWithContinueThread(bool interrupt); + }; + +protected: + PacketResult + SendPacketAndWaitForResponseNoLock(llvm::StringRef payload, StringExtractorGDBRemote &response); + + virtual void + OnRunPacketSent(bool first); + +private: + // Variables handling synchronization between the Continue thread and any other threads + // wishing to send packets over the connection. Either the continue thread has control over + // the connection (m_is_running == true) or the connection is free for an arbitrary number of + // other senders to take which indicate their interest by incrementing m_async_count. + // Semantics of individual states: + // - m_continue_packet == false, m_async_count == 0: connection is free + // - m_continue_packet == true, m_async_count == 0: only continue thread is present + // - m_continue_packet == true, m_async_count > 0: continue thread has control, async threads + // should interrupt it and wait for it to set m_continue_packet to false + // - m_continue_packet == false, m_async_count > 0: async threads have control, continue + // thread needs to wait for them to finish (m_async_count goes down to 0). + std::mutex m_mutex; + std::condition_variable m_cv; + // Packet with which to resume after an async interrupt. Can be changed by an async thread + // e.g. to inject a signal. + std::string m_continue_packet; + // When was the interrupt packet sent. Used to make sure we time out if the stub does not + // respond to interrupt requests. + std::chrono::time_point<std::chrono::steady_clock> m_interrupt_time; + uint32_t m_async_count; + bool m_is_running; + bool m_should_stop; // Whether we should resume after a stop. + // end of continue thread synchronization block + + // This handles the synchronization between individual async threads. For now they just use a + // simple mutex. + std::recursive_mutex m_async_mutex; + + bool + ShouldStop(const UnixSignals &signals, StringExtractorGDBRemote &response); + + class ContinueLock + { + public: + enum class LockResult + { + Success, + Cancelled, + Failed + }; + + explicit ContinueLock(GDBRemoteClientBase &comm); + ~ContinueLock(); + explicit operator bool() { return m_acquired; } + + LockResult + lock(); + + void + unlock(); + + private: + GDBRemoteClientBase &m_comm; + bool m_acquired; + }; +}; + +} // namespace process_gdb_remote +} // namespace lldb_private + +#endif // liblldb_GDBRemoteCommunicationClient_h_ Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp?rev=277139&r1=277138&r2=277139&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp Fri Jul 29 08:10:02 2016 @@ -161,9 +161,6 @@ GDBRemoteCommunication::GDBRemoteCommuni #endif m_echo_number(0), m_supports_qEcho(eLazyBoolCalculate), - m_sequence_mutex(), - m_public_is_running(false), - m_private_is_running(false), m_history(512), m_send_acks(true), m_compression_type(CompressionType::None), @@ -226,13 +223,6 @@ GDBRemoteCommunication::SendNack () } GDBRemoteCommunication::PacketResult -GDBRemoteCommunication::SendPacket (const char *payload, size_t payload_length) -{ - std::lock_guard<std::recursive_mutex> guard(m_sequence_mutex); - return SendPacketNoLock (payload, payload_length); -} - -GDBRemoteCommunication::PacketResult GDBRemoteCommunication::SendPacketNoLock (const char *payload, size_t payload_length) { if (IsConnected()) @@ -321,22 +311,6 @@ GDBRemoteCommunication::GetAck () return result; } -bool -GDBRemoteCommunication::GetSequenceMutex(std::unique_lock<std::recursive_mutex> &lock, const char *failure_message) -{ - if (IsRunning()) - return (lock = std::unique_lock<std::recursive_mutex>(m_sequence_mutex, std::try_to_lock)).owns_lock(); - - lock = std::unique_lock<std::recursive_mutex>(m_sequence_mutex); - return true; -} - -bool -GDBRemoteCommunication::WaitForNotRunningPrivate(const std::chrono::microseconds &timeout) -{ - return m_private_is_running.WaitForValueEqualTo(false, timeout, NULL); -} - GDBRemoteCommunication::PacketResult GDBRemoteCommunication::ReadPacket (StringExtractorGDBRemote &response, uint32_t timeout_usec, bool sync_on_timeout) { Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h?rev=277139&r1=277138&r2=277139&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h Fri Jul 29 08:10:02 2016 @@ -114,21 +114,12 @@ public: CalculcateChecksum (const char *payload, size_t payload_length); - bool - GetSequenceMutex(std::unique_lock<std::recursive_mutex> &lock, const char *failure_message = nullptr); - PacketType CheckForPacket (const uint8_t *src, size_t src_len, StringExtractorGDBRemote &packet); bool - IsRunning() const - { - return m_public_is_running.GetValue(); - } - - bool GetSendAcks () { return m_send_acks; @@ -285,13 +276,6 @@ protected: uint32_t m_packet_timeout; uint32_t m_echo_number; LazyBool m_supports_qEcho; -#ifdef ENABLE_MUTEX_ERROR_CHECKING -#error TrackingMutex is no longer supported -#else - std::recursive_mutex m_sequence_mutex; // Restrict access to sending/receiving packets to a single thread at a time -#endif - Predicate<bool> m_public_is_running; - Predicate<bool> m_private_is_running; History m_history; bool m_send_acks; bool m_is_platform; // Set to true if this class represents a platform, @@ -301,10 +285,6 @@ protected: CompressionType m_compression_type; PacketResult - SendPacket (const char *payload, - size_t payload_length); - - PacketResult SendPacketNoLock (const char *payload, size_t payload_length); @@ -321,9 +301,6 @@ protected: bool sync_on_timeout); bool - WaitForNotRunningPrivate(const std::chrono::microseconds &timeout); - - bool CompressionIsEnabled () { return m_compression_type != CompressionType::None; Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=277139&r1=277138&r2=277139&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp Fri Jul 29 08:10:02 2016 @@ -19,24 +19,18 @@ #include <numeric> // Other libraries and framework includes -#include "llvm/ADT/STLExtras.h" -#include "llvm/ADT/Triple.h" -#include "lldb/Interpreter/Args.h" #include "lldb/Core/Log.h" #include "lldb/Core/ModuleSpec.h" #include "lldb/Core/State.h" #include "lldb/Core/StreamGDBRemote.h" #include "lldb/Core/StreamString.h" -#include "lldb/Host/ConnectionFileDescriptor.h" -#include "lldb/Host/Endian.h" -#include "lldb/Host/Host.h" #include "lldb/Host/HostInfo.h" #include "lldb/Host/StringConvert.h" #include "lldb/Host/TimeValue.h" +#include "lldb/Interpreter/Args.h" #include "lldb/Symbol/Symbol.h" -#include "lldb/Target/Target.h" #include "lldb/Target/MemoryRegionInfo.h" -#include "lldb/Target/UnixSignals.h" +#include "lldb/Target/Target.h" // Project includes #include "Utility/StringExtractorGDBRemote.h" @@ -56,7 +50,7 @@ using namespace lldb_private::process_gd // GDBRemoteCommunicationClient constructor //---------------------------------------------------------------------- GDBRemoteCommunicationClient::GDBRemoteCommunicationClient() - : GDBRemoteCommunication("gdb-remote.client", "gdb-remote.client.rx_packet"), + : GDBRemoteClientBase("gdb-remote.client", "gdb-remote.client.rx_packet"), m_supports_not_sending_acks(eLazyBoolCalculate), m_supports_thread_suffix(eLazyBoolCalculate), m_supports_threads_in_stop_reply(eLazyBoolCalculate), @@ -88,7 +82,7 @@ GDBRemoteCommunicationClient::GDBRemoteC m_supports_augmented_libraries_svr4_read(eLazyBoolCalculate), m_supports_jThreadExtendedInfo(eLazyBoolCalculate), m_supports_jLoadedDynamicLibrariesInfos(eLazyBoolCalculate), - m_supports_jGetSharedCacheInfo (eLazyBoolCalculate), + m_supports_jGetSharedCacheInfo(eLazyBoolCalculate), m_supports_qProcessInfoPID(true), m_supports_qfProcessInfo(true), m_supports_qUserName(true), @@ -109,14 +103,6 @@ GDBRemoteCommunicationClient::GDBRemoteC m_curr_tid(LLDB_INVALID_THREAD_ID), m_curr_tid_run(LLDB_INVALID_THREAD_ID), m_num_supported_hardware_watchpoints(0), - m_async_mutex(), - m_async_packet_predicate(false), - m_async_packet(), - m_async_result(PacketResult::Success), - m_async_response(), - m_async_signal(-1), - m_interrupt_sent(false), - m_thread_id_to_used_usec_map(), m_host_arch(), m_process_arch(), m_os_version_major(UINT32_MAX), @@ -720,10 +706,8 @@ GDBRemoteCommunicationClient::SendPacket std::string &response_string ) { - std::unique_lock<std::recursive_mutex> lock; - if (!GetSequenceMutex( - lock, - "ProcessGDBRemote::SendPacketsAndConcatenateResponses() failed due to not getting the sequence mutex")) + Lock lock(*this, false); + if (!lock) { Log *log (ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet (GDBR_LOG_PROCESS | GDBR_LOG_PACKETS)); if (log) @@ -745,9 +729,7 @@ GDBRemoteCommunicationClient::SendPacket // Construct payload char sizeDescriptor[128]; snprintf(sizeDescriptor, sizeof(sizeDescriptor), "%x,%x", offset, response_size); - PacketResult result = SendPacketAndWaitForResponse((payload_prefix_str + sizeDescriptor).c_str(), - this_response, - /*send_async=*/false); + PacketResult result = SendPacketAndWaitForResponseNoLock(payload_prefix_str + sizeDescriptor, this_response); if (result != PacketResult::Success) return result; @@ -767,722 +749,6 @@ GDBRemoteCommunicationClient::SendPacket } } -GDBRemoteCommunicationClient::PacketResult -GDBRemoteCommunicationClient::SendPacketAndWaitForResponse -( - const char *payload, - StringExtractorGDBRemote &response, - bool send_async -) -{ - return SendPacketAndWaitForResponse (payload, - ::strlen (payload), - response, - send_async); -} - -GDBRemoteCommunicationClient::PacketResult -GDBRemoteCommunicationClient::SendPacketAndWaitForResponseNoLock (const char *payload, - size_t payload_length, - StringExtractorGDBRemote &response) -{ - PacketResult packet_result = SendPacketNoLock(payload, payload_length); - if (packet_result == PacketResult::Success) - { - const size_t max_response_retries = 3; - for (size_t i=0; i<max_response_retries; ++i) - { - packet_result = ReadPacket(response, GetPacketTimeoutInMicroSeconds (), true); - // Make sure we received a response - if (packet_result != PacketResult::Success) - return packet_result; - // Make sure our response is valid for the payload that was sent - if (response.ValidateResponse()) - return packet_result; - // Response says it wasn't valid - Log *log = ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PACKETS); - if (log) - log->Printf("error: packet with payload \"%*s\" got invalid response \"%s\": %s", - (int)payload_length, - payload, - response.GetStringRef().c_str(), - (i == (max_response_retries - 1)) ? "using invalid response and giving up" : "ignoring response and waiting for another"); - } - } - return packet_result; -} - -GDBRemoteCommunicationClient::PacketResult -GDBRemoteCommunicationClient::SendPacketAndWaitForResponse -( - const char *payload, - size_t payload_length, - StringExtractorGDBRemote &response, - bool send_async -) -{ - PacketResult packet_result = PacketResult::ErrorSendFailed; - std::unique_lock<std::recursive_mutex> lock; - Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PROCESS)); - - // In order to stop async notifications from being processed in the middle of the - // send/receive sequence Hijack the broadcast. Then rebroadcast any events when we are done. - static ListenerSP hijack_listener_sp(Listener::MakeListener("lldb.NotifyHijacker")); - HijackBroadcaster(hijack_listener_sp, eBroadcastBitGdbReadThreadGotNotify); - - if (GetSequenceMutex(lock)) - { - packet_result = SendPacketAndWaitForResponseNoLock (payload, payload_length, response); - } - else - { - if (send_async) - { - if (IsRunning()) - { - std::lock_guard<std::recursive_mutex> guard(m_async_mutex); - m_async_packet.assign(payload, payload_length); - m_async_response.CopyResponseValidator(response); - m_async_packet_predicate.SetValue (true, eBroadcastNever); - - if (log) - log->Printf ("async: async packet = %s", m_async_packet.c_str()); - - bool timed_out = false; - if (SendInterrupt(lock, 2, timed_out)) - { - if (m_interrupt_sent) - { - m_interrupt_sent = false; - - std::chrono::time_point<std::chrono::system_clock> until; - until = std::chrono::system_clock::now() + std::chrono::seconds(m_packet_timeout); - - if (log) - log->Printf ("async: sent interrupt"); - - if (m_async_packet_predicate.WaitForValueEqualTo( - false, std::chrono::duration_cast<std::chrono::microseconds>( - until - std::chrono::system_clock::now()), - &timed_out)) - { - if (log) - log->Printf ("async: got response"); - - // Swap the response buffer to avoid malloc and string copy - response.GetStringRef().swap (m_async_response.GetStringRef()); - packet_result = m_async_result; - } - else - { - if (log) - log->Printf ("async: timed out waiting for response"); - } - - // Make sure we wait until the continue packet has been sent again... - if (m_private_is_running.WaitForValueEqualTo( - true, std::chrono::duration_cast<std::chrono::microseconds>( - until - std::chrono::system_clock::now()), - &timed_out)) - { - if (log) - { - if (timed_out) - log->Printf ("async: timed out waiting for process to resume, but process was resumed"); - else - log->Printf ("async: async packet sent"); - } - } - else - { - if (log) - log->Printf ("async: timed out waiting for process to resume"); - } - } - else - { - // We had a racy condition where we went to send the interrupt - // yet we were able to get the lock, so the process must have - // just stopped? - if (log) - log->Printf ("async: got lock without sending interrupt"); - // Send the packet normally since we got the lock - packet_result = SendPacketAndWaitForResponseNoLock (payload, payload_length, response); - } - } - else - { - if (log) - log->Printf ("async: failed to interrupt"); - } - - m_async_response.SetResponseValidator(nullptr, nullptr); - - } - else - { - if (log) - log->Printf ("async: not running, async is ignored"); - } - } - else - { - if (log) - log->Printf("error: failed to get packet sequence mutex, not sending packet '%*s'", (int) payload_length, payload); - } - } - - // Remove our Hijacking listener from the broadcast. - RestoreBroadcaster(); - - // If a notification event occurred, rebroadcast since it can now be processed safely. - EventSP event_sp; - if (hijack_listener_sp->GetNextEvent(event_sp)) - BroadcastEvent(event_sp); - - return packet_result; -} - -static const char *end_delimiter = "--end--;"; -static const int end_delimiter_len = 8; - -std::string -GDBRemoteCommunicationClient::HarmonizeThreadIdsForProfileData -( ProcessGDBRemote *process, - StringExtractorGDBRemote& profileDataExtractor -) -{ - std::map<uint64_t, uint32_t> new_thread_id_to_used_usec_map; - std::stringstream final_output; - std::string name, value; - - // Going to assuming thread_used_usec comes first, else bail out. - while (profileDataExtractor.GetNameColonValue(name, value)) - { - if (name.compare("thread_used_id") == 0) - { - StringExtractor threadIDHexExtractor(value.c_str()); - uint64_t thread_id = threadIDHexExtractor.GetHexMaxU64(false, 0); - - bool has_used_usec = false; - uint32_t curr_used_usec = 0; - std::string usec_name, usec_value; - uint32_t input_file_pos = profileDataExtractor.GetFilePos(); - if (profileDataExtractor.GetNameColonValue(usec_name, usec_value)) - { - if (usec_name.compare("thread_used_usec") == 0) - { - has_used_usec = true; - curr_used_usec = strtoull(usec_value.c_str(), NULL, 0); - } - else - { - // We didn't find what we want, it is probably - // an older version. Bail out. - profileDataExtractor.SetFilePos(input_file_pos); - } - } - - if (has_used_usec) - { - uint32_t prev_used_usec = 0; - std::map<uint64_t, uint32_t>::iterator iterator = m_thread_id_to_used_usec_map.find(thread_id); - if (iterator != m_thread_id_to_used_usec_map.end()) - { - prev_used_usec = m_thread_id_to_used_usec_map[thread_id]; - } - - uint32_t real_used_usec = curr_used_usec - prev_used_usec; - // A good first time record is one that runs for at least 0.25 sec - bool good_first_time = (prev_used_usec == 0) && (real_used_usec > 250000); - bool good_subsequent_time = (prev_used_usec > 0) && - ((real_used_usec > 0) || (process->HasAssignedIndexIDToThread(thread_id))); - - if (good_first_time || good_subsequent_time) - { - // We try to avoid doing too many index id reservation, - // resulting in fast increase of index ids. - - final_output << name << ":"; - int32_t index_id = process->AssignIndexIDToThread(thread_id); - final_output << index_id << ";"; - - final_output << usec_name << ":" << usec_value << ";"; - } - else - { - // Skip past 'thread_used_name'. - std::string local_name, local_value; - profileDataExtractor.GetNameColonValue(local_name, local_value); - } - - // Store current time as previous time so that they can be compared later. - new_thread_id_to_used_usec_map[thread_id] = curr_used_usec; - } - else - { - // Bail out and use old string. - final_output << name << ":" << value << ";"; - } - } - else - { - final_output << name << ":" << value << ";"; - } - } - final_output << end_delimiter; - m_thread_id_to_used_usec_map = new_thread_id_to_used_usec_map; - - return final_output.str(); -} - -bool -GDBRemoteCommunicationClient::SendvContPacket -( - ProcessGDBRemote *process, - const char *payload, - size_t packet_length, - StringExtractorGDBRemote &response -) -{ - - m_curr_tid = LLDB_INVALID_THREAD_ID; - Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); - if (log) - log->Printf("GDBRemoteCommunicationClient::%s ()", __FUNCTION__); - - // we want to lock down packet sending while we continue - std::lock_guard<std::recursive_mutex> guard(m_sequence_mutex); - - // here we broadcast this before we even send the packet!! - // this signals doContinue() to exit - BroadcastEvent(eBroadcastBitRunPacketSent, NULL); - - // set the public state to running - m_public_is_running.SetValue(true, eBroadcastNever); - - // Set the starting continue packet into "continue_packet". This packet - // may change if we are interrupted and we continue after an async packet... - std::string continue_packet(payload, packet_length); - - if (log) - log->Printf("GDBRemoteCommunicationClient::%s () sending vCont packet: %s", __FUNCTION__, continue_packet.c_str()); - - if (SendPacketNoLock(continue_packet.c_str(), continue_packet.size()) != PacketResult::Success) - return false; - - // set the private state to running and broadcast this - m_private_is_running.SetValue(true, eBroadcastAlways); - - if (log) - log->Printf("GDBRemoteCommunicationClient::%s () ReadPacket(%s)", __FUNCTION__, continue_packet.c_str()); - - // wait for the response to the vCont - if (ReadPacket(response, UINT32_MAX, false) == PacketResult::Success) - { - if (response.IsOKResponse()) - return true; - } - - return false; -} - -StateType -GDBRemoteCommunicationClient::SendContinuePacketAndWaitForResponse -( - ProcessGDBRemote *process, - const char *payload, - size_t packet_length, - StringExtractorGDBRemote &response -) -{ - m_curr_tid = LLDB_INVALID_THREAD_ID; - Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PROCESS)); - if (log) - log->Printf ("GDBRemoteCommunicationClient::%s ()", __FUNCTION__); - - std::lock_guard<std::recursive_mutex> guard(m_sequence_mutex); - StateType state = eStateRunning; - - m_public_is_running.SetValue (true, eBroadcastNever); - // Set the starting continue packet into "continue_packet". This packet - // may change if we are interrupted and we continue after an async packet... - std::string continue_packet(payload, packet_length); - - const auto sigstop_signo = process->GetUnixSignals()->GetSignalNumberFromName("SIGSTOP"); - const auto sigint_signo = process->GetUnixSignals()->GetSignalNumberFromName("SIGINT"); - - bool got_async_packet = false; - bool broadcast_sent = false; - - while (state == eStateRunning) - { - if (!got_async_packet) - { - if (log) - log->Printf ("GDBRemoteCommunicationClient::%s () sending continue packet: %s", __FUNCTION__, continue_packet.c_str()); - if (SendPacketNoLock(continue_packet.c_str(), continue_packet.size()) != PacketResult::Success) - state = eStateInvalid; - else - m_interrupt_sent = false; - - if (! broadcast_sent) - { - BroadcastEvent(eBroadcastBitRunPacketSent, NULL); - broadcast_sent = true; - } - - m_private_is_running.SetValue (true, eBroadcastAlways); - } - - got_async_packet = false; - - if (log) - log->Printf ("GDBRemoteCommunicationClient::%s () ReadPacket(%s)", __FUNCTION__, continue_packet.c_str()); - - if (ReadPacket(response, UINT32_MAX, false) == PacketResult::Success) - { - if (response.Empty()) - state = eStateInvalid; - else - { - const char stop_type = response.GetChar(); - if (log) - log->Printf ("GDBRemoteCommunicationClient::%s () got packet: %s", __FUNCTION__, response.GetStringRef().c_str()); - switch (stop_type) - { - case 'T': - case 'S': - { - if (process->GetStopID() == 0) - { - if (process->GetID() == LLDB_INVALID_PROCESS_ID) - { - lldb::pid_t pid = GetCurrentProcessID (); - if (pid != LLDB_INVALID_PROCESS_ID) - process->SetID (pid); - } - process->BuildDynamicRegisterInfo (true); - } - - // Privately notify any internal threads that we have stopped - // in case we wanted to interrupt our process, yet we might - // send a packet and continue without returning control to the - // user. - m_private_is_running.SetValue (false, eBroadcastAlways); - - const uint8_t signo = response.GetHexU8 (UINT8_MAX); - - bool continue_after_async = m_async_signal != -1 || m_async_packet_predicate.GetValue(); - if (continue_after_async || m_interrupt_sent) - { - // We sent an interrupt packet to stop the inferior process - // for an async signal or to send an async packet while running - // but we might have been single stepping and received the - // stop packet for the step instead of for the interrupt packet. - // Typically when an interrupt is sent a SIGINT or SIGSTOP - // is used, so if we get anything else, we need to try and - // get another stop reply packet that may have been sent - // due to sending the interrupt when the target is stopped - // which will just re-send a copy of the last stop reply - // packet. If we don't do this, then the reply for our - // async packet will be the repeat stop reply packet and cause - // a lot of trouble for us! We also have some debugserver - // binaries that would send two stop replies anytime the process - // was interrupted, so we need to also check for an extra - // stop reply packet if we interrupted the process - const bool received_nonstop_signal = signo != sigint_signo && signo != sigstop_signo; - if (m_interrupt_sent || received_nonstop_signal) - { - if (received_nonstop_signal) - continue_after_async = false; - - // Try for a very brief time (0.1s) to get another stop reply - // packet to make sure it doesn't get in the way - StringExtractorGDBRemote extra_stop_reply_packet; - uint32_t timeout_usec = 100000; - if (ReadPacket (extra_stop_reply_packet, timeout_usec, false) == PacketResult::Success) - { - switch (extra_stop_reply_packet.GetChar()) - { - case 'T': - case 'S': - // We did get an extra stop reply, which means - // our interrupt didn't stop the target so we - // shouldn't continue after the async signal - // or packet is sent... - continue_after_async = false; - break; - } - } - } - } - - if (m_async_signal != -1) - { - if (log) - log->Printf ("async: send signo = %s", Host::GetSignalAsCString (m_async_signal)); - - // Save off the async signal we are supposed to send - const int async_signal = m_async_signal; - // Clear the async signal member so we don't end up - // sending the signal multiple times... - m_async_signal = -1; - // Check which signal we stopped with - if (signo == async_signal) - { - if (log) - log->Printf ("async: stopped with signal %s, we are done running", Host::GetSignalAsCString (signo)); - - // We already stopped with a signal that we wanted - // to stop with, so we are done - } - else - { - // We stopped with a different signal that the one - // we wanted to stop with, so now we must resume - // with the signal we want - char signal_packet[32]; - int signal_packet_len = 0; - signal_packet_len = ::snprintf (signal_packet, - sizeof (signal_packet), - "C%2.2x", - async_signal); - - if (log) - log->Printf ("async: stopped with signal %s, resume with %s", - Host::GetSignalAsCString (signo), - Host::GetSignalAsCString (async_signal)); - - // Set the continue packet to resume even if the - // interrupt didn't cause our stop (ignore continue_after_async) - continue_packet.assign(signal_packet, signal_packet_len); - continue; - } - } - else if (m_async_packet_predicate.GetValue()) - { - Log * packet_log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PACKETS)); - - // We are supposed to send an asynchronous packet while - // we are running. - m_async_response.Clear(); - if (m_async_packet.empty()) - { - m_async_result = PacketResult::ErrorSendFailed; - if (packet_log) - packet_log->Printf ("async: error: empty async packet"); - - } - else - { - if (packet_log) - packet_log->Printf ("async: sending packet"); - - m_async_result = SendPacketAndWaitForResponse (&m_async_packet[0], - m_async_packet.size(), - m_async_response, - false); - } - // Let the other thread that was trying to send the async - // packet know that the packet has been sent and response is - // ready... - m_async_packet_predicate.SetValue(false, eBroadcastAlways); - - if (packet_log) - packet_log->Printf ("async: sent packet, continue_after_async = %i", continue_after_async); - - // Set the continue packet to resume if our interrupt - // for the async packet did cause the stop - if (continue_after_async) - { - // Reverting this for now as it is causing deadlocks - // in programs (<rdar://problem/11529853>). In the future - // we should check our thread list and "do the right thing" - // for new threads that show up while we stop and run async - // packets. Setting the packet to 'c' to continue all threads - // is the right thing to do 99.99% of the time because if a - // thread was single stepping, and we sent an interrupt, we - // will notice above that we didn't stop due to an interrupt - // but stopped due to stepping and we would _not_ continue. - continue_packet.assign (1, 'c'); - continue; - } - } - // Stop with signal and thread info - state = eStateStopped; - } - break; - - case 'W': - case 'X': - // process exited - state = eStateExited; - break; - - case 'O': - // STDOUT - { - got_async_packet = true; - std::string inferior_stdout; - inferior_stdout.reserve(response.GetBytesLeft () / 2); - - uint8_t ch; - while (response.GetHexU8Ex(ch)) - { - if (ch != 0) - inferior_stdout.append(1, (char)ch); - } - process->AppendSTDOUT (inferior_stdout.c_str(), inferior_stdout.size()); - } - break; - - case 'A': - // Async miscellaneous reply. Right now, only profile data is coming through this channel. - { - got_async_packet = true; - std::string input = response.GetStringRef().substr(1); // '1' to move beyond 'A' - if (m_partial_profile_data.length() > 0) - { - m_partial_profile_data.append(input); - input = m_partial_profile_data; - m_partial_profile_data.clear(); - } - - size_t found, pos = 0, len = input.length(); - while ((found = input.find(end_delimiter, pos)) != std::string::npos) - { - StringExtractorGDBRemote profileDataExtractor(input.substr(pos, found).c_str()); - std::string profile_data = HarmonizeThreadIdsForProfileData(process, profileDataExtractor); - process->BroadcastAsyncProfileData (profile_data); - - pos = found + end_delimiter_len; - } - - if (pos < len) - { - // Last incomplete chunk. - m_partial_profile_data = input.substr(pos); - } - } - break; - - case 'E': - // ERROR - state = eStateInvalid; - break; - - default: - if (log) - log->Printf ("GDBRemoteCommunicationClient::%s () unrecognized async packet", __FUNCTION__); - state = eStateInvalid; - break; - } - } - } - else - { - if (log) - log->Printf ("GDBRemoteCommunicationClient::%s () ReadPacket(...) => false", __FUNCTION__); - state = eStateInvalid; - } - } - if (log) - log->Printf ("GDBRemoteCommunicationClient::%s () => %s", __FUNCTION__, StateAsCString(state)); - response.SetFilePos(0); - m_private_is_running.SetValue (false, eBroadcastAlways); - m_public_is_running.SetValue (false, eBroadcastAlways); - return state; -} - -bool -GDBRemoteCommunicationClient::SendAsyncSignal (int signo) -{ - std::lock_guard<std::recursive_mutex> guard(m_async_mutex); - m_async_signal = signo; - bool timed_out = false; - std::unique_lock<std::recursive_mutex> lock; - if (SendInterrupt(lock, 1, timed_out)) - return true; - m_async_signal = -1; - return false; -} - -// This function takes a mutex locker as a parameter in case the GetSequenceMutex -// actually succeeds. If it doesn't succeed in acquiring the sequence mutex -// (the expected result), then it will send the halt packet. If it does succeed -// then the caller that requested the interrupt will want to keep the sequence -// locked down so that no one else can send packets while the caller has control. -// This function usually gets called when we are running and need to stop the -// target. It can also be used when we are running and we need to do something -// else (like read/write memory), so we need to interrupt the running process -// (gdb remote protocol requires this), and do what we need to do, then resume. - -bool -GDBRemoteCommunicationClient::SendInterrupt(std::unique_lock<std::recursive_mutex> &lock, - uint32_t seconds_to_wait_for_stop, bool &timed_out) -{ - timed_out = false; - Log *log (ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet (GDBR_LOG_PROCESS | GDBR_LOG_PACKETS)); - - if (IsRunning()) - { - // Only send an interrupt if our debugserver is running... - if (GetSequenceMutex(lock)) - { - if (log) - log->Printf ("SendInterrupt () - got sequence mutex without having to interrupt"); - } - else - { - // Someone has the mutex locked waiting for a response or for the - // inferior to stop, so send the interrupt on the down low... - char ctrl_c = '\x03'; - ConnectionStatus status = eConnectionStatusSuccess; - size_t bytes_written = Write (&ctrl_c, 1, status, NULL); - if (log) - log->PutCString("send packet: \\x03"); - if (bytes_written > 0) - { - m_interrupt_sent = true; - if (seconds_to_wait_for_stop) - { - if (m_private_is_running.WaitForValueEqualTo(false, std::chrono::seconds(seconds_to_wait_for_stop), - &timed_out)) - { - if (log) - log->PutCString ("SendInterrupt () - sent interrupt, private state stopped"); - return true; - } - else - { - if (log) - log->Printf ("SendInterrupt () - sent interrupt, timed out wating for async thread resume"); - } - } - else - { - if (log) - log->Printf ("SendInterrupt () - sent interrupt, not waiting for stop..."); - return true; - } - } - else - { - if (log) - log->Printf ("SendInterrupt () - failed to write interrupt"); - } - return false; - } - } - else - { - if (log) - log->Printf ("SendInterrupt () - not running"); - } - return true; -} - lldb::pid_t GDBRemoteCommunicationClient::GetCurrentProcessID (bool allow_lazy) { @@ -3651,18 +2917,18 @@ size_t GDBRemoteCommunicationClient::GetCurrentThreadIDs (std::vector<lldb::tid_t> &thread_ids, bool &sequence_mutex_unavailable) { - std::unique_lock<std::recursive_mutex> lock; thread_ids.clear(); - if (GetSequenceMutex(lock, "ProcessGDBRemote::UpdateThreadList() failed due to not getting the sequence mutex")) + Lock lock(*this, false); + if (lock) { sequence_mutex_unavailable = false; StringExtractorGDBRemote response; PacketResult packet_result; - for (packet_result = SendPacketAndWaitForResponseNoLock ("qfThreadInfo", strlen("qfThreadInfo"), response); + for (packet_result = SendPacketAndWaitForResponseNoLock("qfThreadInfo", response); packet_result == PacketResult::Success && response.IsNormalResponse(); - packet_result = SendPacketAndWaitForResponseNoLock ("qsThreadInfo", strlen("qsThreadInfo"), response)) + packet_result = SendPacketAndWaitForResponseNoLock("qsThreadInfo", response)) { char ch = response.GetChar(); if (ch == 'l') @@ -3710,7 +2976,8 @@ GDBRemoteCommunicationClient::GetCurrent lldb::addr_t GDBRemoteCommunicationClient::GetShlibInfoAddr() { - if (!IsRunning()) + Lock lock(*this, false); + if (lock) { StringExtractorGDBRemote response; if (SendPacketAndWaitForResponse("qShlibInfoAddr", ::strlen ("qShlibInfoAddr"), response, false) == PacketResult::Success) @@ -3719,6 +2986,11 @@ GDBRemoteCommunicationClient::GetShlibIn return response.GetHexMaxU64(false, LLDB_INVALID_ADDRESS); } } + else if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS)) + { + log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for qShlibInfoAddr packet.", + __FUNCTION__); + } return LLDB_INVALID_ADDRESS; } @@ -4201,8 +3473,8 @@ GDBRemoteCommunicationClient::AvoidGPack bool GDBRemoteCommunicationClient::ReadRegister(lldb::tid_t tid, uint32_t reg, StringExtractorGDBRemote &response) { - std::unique_lock<std::recursive_mutex> lock; - if (GetSequenceMutex(lock, "Didn't get sequence mutex for p packet.")) + Lock lock(*this, false); + if (lock) { const bool thread_suffix_supported = GetThreadSuffixSupported(); @@ -4218,6 +3490,10 @@ GDBRemoteCommunicationClient::ReadRegist return SendPacketAndWaitForResponse(packet, response, false) == PacketResult::Success; } } + else if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS)) + { + log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for p packet.", __FUNCTION__); + } return false; } @@ -4226,8 +3502,8 @@ GDBRemoteCommunicationClient::ReadRegist bool GDBRemoteCommunicationClient::ReadAllRegisters (lldb::tid_t tid, StringExtractorGDBRemote &response) { - std::unique_lock<std::recursive_mutex> lock; - if (GetSequenceMutex(lock, "Didn't get sequence mutex for g packet.")) + Lock lock(*this, false); + if (lock) { const bool thread_suffix_supported = GetThreadSuffixSupported(); @@ -4244,6 +3520,10 @@ GDBRemoteCommunicationClient::ReadAllReg return SendPacketAndWaitForResponse(packet, response, false) == PacketResult::Success; } } + else if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS)) + { + log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for g packet.", __FUNCTION__); + } return false; } bool @@ -4254,8 +3534,8 @@ GDBRemoteCommunicationClient::SaveRegist return false; m_supports_QSaveRegisterState = eLazyBoolYes; - std::unique_lock<std::recursive_mutex> lock; - if (GetSequenceMutex(lock, "Didn't get sequence mutex for QSaveRegisterState.")) + Lock lock(*this, false); + if (lock) { const bool thread_suffix_supported = GetThreadSuffixSupported(); if (thread_suffix_supported || SetCurrentThread(tid)) @@ -4285,6 +3565,11 @@ GDBRemoteCommunicationClient::SaveRegist } } } + else if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS)) + { + log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for QSaveRegisterState packet.", + __FUNCTION__); + } return false; } @@ -4297,8 +3582,8 @@ GDBRemoteCommunicationClient::RestoreReg if (m_supports_QSaveRegisterState == eLazyBoolNo) return false; - std::unique_lock<std::recursive_mutex> lock; - if (GetSequenceMutex(lock, "Didn't get sequence mutex for QRestoreRegisterState.")) + Lock lock(*this, false); + if (lock) { const bool thread_suffix_supported = GetThreadSuffixSupported(); if (thread_suffix_supported || SetCurrentThread(tid)) @@ -4326,6 +3611,11 @@ GDBRemoteCommunicationClient::RestoreReg } } } + else if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS)) + { + log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for QRestoreRegisterState packet.", + __FUNCTION__); + } return false; } @@ -4528,15 +3818,13 @@ GDBRemoteCommunicationClient::ServeSymbo if (m_supports_qSymbol && m_qSymbol_requests_done == false) { - std::unique_lock<std::recursive_mutex> lock; - if (GetSequenceMutex( - lock, - "GDBRemoteCommunicationClient::ServeSymbolLookups() failed due to not getting the sequence mutex")) + Lock lock(*this, false); + if (lock) { StreamString packet; packet.PutCString ("qSymbol::"); StringExtractorGDBRemote response; - while (SendPacketAndWaitForResponseNoLock(packet.GetData(), packet.GetSize(), response) == PacketResult::Success) + while (SendPacketAndWaitForResponseNoLock(packet.GetString(), response) == PacketResult::Success) { if (response.IsOKResponse()) { @@ -4648,6 +3936,18 @@ GDBRemoteCommunicationClient::ServeSymbo return; } + else if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS)) + { + log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex.", __FUNCTION__); + } } } +void +GDBRemoteCommunicationClient::OnRunPacketSent(bool first) +{ + GDBRemoteClientBase::OnRunPacketSent(first); + if (first) + BroadcastEvent(eBroadcastBitRunPacketSent, NULL); + m_curr_tid = LLDB_INVALID_THREAD_ID; +} Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h?rev=277139&r1=277138&r2=277139&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h Fri Jul 29 08:10:02 2016 @@ -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); }; Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp?rev=277139&r1=277138&r2=277139&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp Fri Jul 29 08:10:02 2016 @@ -372,12 +372,6 @@ GDBRemoteRegisterContext::WriteRegisterB return false; GDBRemoteCommunicationClient &gdb_comm (((ProcessGDBRemote *)process)->GetGDBRemote()); -// FIXME: This check isn't right because IsRunning checks the Public state, but this -// is work you need to do - for instance in ShouldStop & friends - before the public -// state has been changed. -// if (gdb_comm.IsRunning()) -// return false; - #if defined (LLDB_CONFIGURATION_DEBUG) assert (m_reg_data.GetByteSize() >= reg_info->byte_offset + reg_info->byte_size); @@ -401,8 +395,8 @@ GDBRemoteRegisterContext::WriteRegisterB reg_info->byte_size, // dst length m_reg_data.GetByteOrder())) // dst byte order { - std::unique_lock<std::recursive_mutex> lock; - if (gdb_comm.GetSequenceMutex(lock, "Didn't get sequence mutex for write register.")) + GDBRemoteClientBase::Lock lock(gdb_comm, false); + if (lock) { const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported(); ProcessSP process_sp (m_thread.GetProcess()); @@ -570,8 +564,8 @@ GDBRemoteRegisterContext::ReadAllRegiste const bool use_g_packet = gdb_comm.AvoidGPackets ((ProcessGDBRemote *)process) == false; - std::unique_lock<std::recursive_mutex> lock; - if (gdb_comm.GetSequenceMutex(lock, "Didn't get sequence mutex for read all registers.")) + GDBRemoteClientBase::Lock lock(gdb_comm, false); + if (lock) { SyncThreadState(process); @@ -679,8 +673,8 @@ GDBRemoteRegisterContext::WriteAllRegist const bool use_g_packet = gdb_comm.AvoidGPackets ((ProcessGDBRemote *)process) == false; StringExtractorGDBRemote response; - std::unique_lock<std::recursive_mutex> lock; - if (gdb_comm.GetSequenceMutex(lock, "Didn't get sequence mutex for write all registers.")) + GDBRemoteClientBase::Lock lock(gdb_comm, false); + if (lock) { const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported(); ProcessSP process_sp (m_thread.GetProcess()); Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=277139&r1=277138&r2=277139&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Fri Jul 29 08:10:02 2016 @@ -24,6 +24,7 @@ #include <algorithm> #include <map> #include <mutex> +#include <sstream> #include "lldb/Breakpoint/Watchpoint.h" #include "lldb/Interpreter/Args.h" @@ -2713,9 +2714,6 @@ ProcessGDBRemote::DoHalt (bool &caused_s { Error error; - bool timed_out = false; - std::unique_lock<std::recursive_mutex> lock; - if (m_public_state.GetValue() == eStateAttaching) { // We are being asked to halt during an attach. We need to just close @@ -2723,17 +2721,7 @@ ProcessGDBRemote::DoHalt (bool &caused_s m_gdb_comm.Disconnect(); } else - { - if (!m_gdb_comm.SendInterrupt(lock, 2, timed_out)) - { - if (timed_out) - error.SetErrorString("timed out sending interrupt packet"); - else - error.SetErrorString("unknown error sending interrupt packet"); - } - - caused_stop = m_gdb_comm.GetInterruptWasSent (); - } + caused_stop = m_gdb_comm.Interrupt(); return error; } @@ -3886,7 +3874,8 @@ ProcessGDBRemote::AsyncThread (void *arg if (process->GetTarget().GetNonStopModeEnabled()) { // send the vCont packet - if (!process->GetGDBRemote().SendvContPacket(process, continue_cstr, continue_cstr_len, response)) + if (!process->GetGDBRemote().SendvContPacket( + llvm::StringRef(continue_cstr, continue_cstr_len), response)) { // Something went wrong done = true; @@ -3896,7 +3885,9 @@ ProcessGDBRemote::AsyncThread (void *arg // If in All-Stop-Mode else { - StateType stop_state = process->GetGDBRemote().SendContinuePacketAndWaitForResponse (process, continue_cstr, continue_cstr_len, response); + StateType stop_state = process->GetGDBRemote().SendContinuePacketAndWaitForResponse( + *process, *process->GetUnixSignals(), + llvm::StringRef(continue_cstr, continue_cstr_len), response); // We need to immediately clear the thread ID list so we are sure to get a valid list of threads. // The thread ID list might be contained within the "response", or the stop reply packet that @@ -5038,6 +5029,144 @@ ProcessGDBRemote::ModulesDidLoad (Module m_gdb_comm.ServeSymbolLookups(this); } +void +ProcessGDBRemote::HandleAsyncStdout(llvm::StringRef out) +{ + AppendSTDOUT(out.data(), out.size()); +} + +static const char *end_delimiter = "--end--;"; +static const int end_delimiter_len = 8; + +void +ProcessGDBRemote::HandleAsyncMisc(llvm::StringRef data) +{ + std::string input = data.str(); // '1' to move beyond 'A' + if (m_partial_profile_data.length() > 0) + { + m_partial_profile_data.append(input); + input = m_partial_profile_data; + m_partial_profile_data.clear(); + } + + size_t found, pos = 0, len = input.length(); + while ((found = input.find(end_delimiter, pos)) != std::string::npos) + { + StringExtractorGDBRemote profileDataExtractor(input.substr(pos, found).c_str()); + std::string profile_data = HarmonizeThreadIdsForProfileData(profileDataExtractor); + BroadcastAsyncProfileData(profile_data); + + pos = found + end_delimiter_len; + } + + if (pos < len) + { + // Last incomplete chunk. + m_partial_profile_data = input.substr(pos); + } +} + +std::string +ProcessGDBRemote::HarmonizeThreadIdsForProfileData(StringExtractorGDBRemote &profileDataExtractor) +{ + std::map<uint64_t, uint32_t> new_thread_id_to_used_usec_map; + std::stringstream final_output; + std::string name, value; + + // Going to assuming thread_used_usec comes first, else bail out. + while (profileDataExtractor.GetNameColonValue(name, value)) + { + if (name.compare("thread_used_id") == 0) + { + StringExtractor threadIDHexExtractor(value.c_str()); + uint64_t thread_id = threadIDHexExtractor.GetHexMaxU64(false, 0); + + bool has_used_usec = false; + uint32_t curr_used_usec = 0; + std::string usec_name, usec_value; + uint32_t input_file_pos = profileDataExtractor.GetFilePos(); + if (profileDataExtractor.GetNameColonValue(usec_name, usec_value)) + { + if (usec_name.compare("thread_used_usec") == 0) + { + has_used_usec = true; + curr_used_usec = strtoull(usec_value.c_str(), NULL, 0); + } + else + { + // We didn't find what we want, it is probably + // an older version. Bail out. + profileDataExtractor.SetFilePos(input_file_pos); + } + } + + if (has_used_usec) + { + uint32_t prev_used_usec = 0; + std::map<uint64_t, uint32_t>::iterator iterator = m_thread_id_to_used_usec_map.find(thread_id); + if (iterator != m_thread_id_to_used_usec_map.end()) + { + prev_used_usec = m_thread_id_to_used_usec_map[thread_id]; + } + + uint32_t real_used_usec = curr_used_usec - prev_used_usec; + // A good first time record is one that runs for at least 0.25 sec + bool good_first_time = (prev_used_usec == 0) && (real_used_usec > 250000); + bool good_subsequent_time = + (prev_used_usec > 0) && ((real_used_usec > 0) || (HasAssignedIndexIDToThread(thread_id))); + + if (good_first_time || good_subsequent_time) + { + // We try to avoid doing too many index id reservation, + // resulting in fast increase of index ids. + + final_output << name << ":"; + int32_t index_id = AssignIndexIDToThread(thread_id); + final_output << index_id << ";"; + + final_output << usec_name << ":" << usec_value << ";"; + } + else + { + // Skip past 'thread_used_name'. + std::string local_name, local_value; + profileDataExtractor.GetNameColonValue(local_name, local_value); + } + + // Store current time as previous time so that they can be compared later. + new_thread_id_to_used_usec_map[thread_id] = curr_used_usec; + } + else + { + // Bail out and use old string. + final_output << name << ":" << value << ";"; + } + } + else + { + final_output << name << ":" << value << ";"; + } + } + final_output << end_delimiter; + m_thread_id_to_used_usec_map = new_thread_id_to_used_usec_map; + + return final_output.str(); +} + +void +ProcessGDBRemote::HandleStopReply() +{ + if (GetStopID() != 0) + return; + + if (GetID() == LLDB_INVALID_PROCESS_ID) + { + lldb::pid_t pid = m_gdb_comm.GetCurrentProcessID(); + if (pid != LLDB_INVALID_PROCESS_ID) + SetID(pid); + } + BuildDynamicRegisterInfo(true); +} class CommandObjectProcessGDBRemoteSpeedTest: public CommandObjectParsed { @@ -5245,7 +5374,7 @@ public: if (strstr(packet_cstr, "qGetProfileData") != NULL) { - response_str = process->GetGDBRemote().HarmonizeThreadIdsForProfileData(process, response); + response_str = process->HarmonizeThreadIdsForProfileData(response); } if (response_str.empty()) Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h?rev=277139&r1=277138&r2=277139&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h Fri Jul 29 08:10:02 2016 @@ -43,7 +43,7 @@ namespace process_gdb_remote { class ThreadGDBRemote; -class ProcessGDBRemote : public Process +class ProcessGDBRemote : public Process, private GDBRemoteClientBase::ContinueDelegate { public: ProcessGDBRemote(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp); @@ -273,6 +273,9 @@ public: StructuredData::ObjectSP GetSharedCacheInfo () override; + std::string + HarmonizeThreadIdsForProfileData(StringExtractorGDBRemote &inputStringExtractor); + protected: friend class ThreadGDBRemote; friend class GDBRemoteCommunicationClient; @@ -485,12 +488,25 @@ private: //------------------------------------------------------------------ // For ProcessGDBRemote only //------------------------------------------------------------------ + std::string m_partial_profile_data; + std::map<uint64_t, uint32_t> m_thread_id_to_used_usec_map; + static bool NewThreadNotifyBreakpointHit (void *baton, StoppointCallbackContext *context, lldb::user_id_t break_id, lldb::user_id_t break_loc_id); + //------------------------------------------------------------------ + // ContinueDelegate interface + //------------------------------------------------------------------ + void + HandleAsyncStdout(llvm::StringRef out) override; + void + HandleAsyncMisc(llvm::StringRef data) override; + void + HandleStopReply() override; + DISALLOW_COPY_AND_ASSIGN (ProcessGDBRemote); }; Modified: lldb/trunk/source/Utility/StringExtractor.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/StringExtractor.cpp?rev=277139&r1=277138&r2=277139&view=diff ============================================================================== --- lldb/trunk/source/Utility/StringExtractor.cpp (original) +++ lldb/trunk/source/Utility/StringExtractor.cpp Fri Jul 29 08:10:02 2016 @@ -433,6 +433,7 @@ size_t StringExtractor::GetHexByteString (std::string &str) { str.clear(); + str.reserve(GetBytesLeft() / 2); char ch; while ((ch = GetHexU8()) != '\0') str.append(1, ch); Modified: lldb/trunk/unittests/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/CMakeLists.txt?rev=277139&r1=277138&r2=277139&view=diff ============================================================================== --- lldb/trunk/unittests/CMakeLists.txt (original) +++ lldb/trunk/unittests/CMakeLists.txt Fri Jul 29 08:10:02 2016 @@ -43,6 +43,7 @@ add_subdirectory(Editline) add_subdirectory(Expression) add_subdirectory(Host) add_subdirectory(Interpreter) +add_subdirectory(Process) add_subdirectory(ScriptInterpreter) add_subdirectory(Symbol) add_subdirectory(SymbolFile) Added: lldb/trunk/unittests/Process/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/CMakeLists.txt?rev=277139&view=auto ============================================================================== --- lldb/trunk/unittests/Process/CMakeLists.txt (added) +++ lldb/trunk/unittests/Process/CMakeLists.txt Fri Jul 29 08:10:02 2016 @@ -0,0 +1 @@ +add_subdirectory(gdb-remote) Added: lldb/trunk/unittests/Process/gdb-remote/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/gdb-remote/CMakeLists.txt?rev=277139&view=auto ============================================================================== --- lldb/trunk/unittests/Process/gdb-remote/CMakeLists.txt (added) +++ lldb/trunk/unittests/Process/gdb-remote/CMakeLists.txt Fri Jul 29 08:10:02 2016 @@ -0,0 +1,3 @@ +add_lldb_unittest(ProcessGdbRemoteTests + GDBRemoteClientBaseTest.cpp + ) Added: lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp?rev=277139&view=auto ============================================================================== --- lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp (added) +++ lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp Fri Jul 29 08:10:02 2016 @@ -0,0 +1,396 @@ +//===-- GDBRemoteClientBaseTest.cpp -----------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#if defined(_MSC_VER) && (_HAS_EXCEPTIONS == 0) +// Workaround for MSVC standard library bug, which fails to include <thread> when +// exceptions are disabled. +#include <eh.h> +#endif +#include <future> + +#include "gtest/gtest.h" + +#include "Plugins/Process/Utility/LinuxSignals.h" +#include "Plugins/Process/gdb-remote/GDBRemoteClientBase.h" +#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h" + +#include "lldb/Host/common/TCPSocket.h" +#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h" + +#include "llvm/ADT/STLExtras.h" + +using namespace lldb_private::process_gdb_remote; +using namespace lldb_private; +using namespace lldb; +typedef GDBRemoteCommunication::PacketResult PacketResult; + +namespace +{ + +struct MockDelegate : public GDBRemoteClientBase::ContinueDelegate +{ + std::string output; + std::string misc_data; + unsigned stop_reply_called = 0; + + void + HandleAsyncStdout(llvm::StringRef out) + { + output += out; + } + void + HandleAsyncMisc(llvm::StringRef data) + { + misc_data += data; + } + void + HandleStopReply() + { + ++stop_reply_called; + } +}; + +struct MockServer : public GDBRemoteCommunicationServer +{ + MockServer() : GDBRemoteCommunicationServer("mock-server", "mock-server.listener") { m_send_acks = false; } + + bool + GetThreadSuffixSupported() override + { + return false; + } + + PacketResult + SendPacket(llvm::StringRef payload) + { + return GDBRemoteCommunicationServer::SendPacketNoLock(payload.data(), payload.size()); + } + + PacketResult + GetPacket(StringExtractorGDBRemote &response) + { + const unsigned timeout_usec = 1000000; // 1s + const bool sync_on_timeout = false; + return WaitForPacketWithTimeoutMicroSecondsNoLock(response, timeout_usec, sync_on_timeout); + } +}; + +struct TestClient : public GDBRemoteClientBase +{ + TestClient() : GDBRemoteClientBase("test.client", "test.client.listener") { m_send_acks = false; } + + bool + GetThreadSuffixSupported() override + { + return false; + } +}; + +struct ContinueFixture +{ + MockDelegate delegate; + TestClient client; + MockServer server; + ListenerSP listener_sp; + + ContinueFixture(); + + StateType + SendCPacket(StringExtractorGDBRemote &response) + { + return client.SendContinuePacketAndWaitForResponse(delegate, LinuxSignals(), "c", response); + } + + void + WaitForRunEvent() + { + EventSP event_sp; + listener_sp->WaitForEventForBroadcasterWithType(std::chrono::microseconds(0), &client, + TestClient::eBroadcastBitRunPacketSent, event_sp); + } +}; + +ContinueFixture::ContinueFixture() : listener_sp(Listener::MakeListener("listener")) +{ + bool child_processes_inherit = false; + Error error; + TCPSocket listen_socket(child_processes_inherit, error); + EXPECT_FALSE(error.Fail()); + error = listen_socket.Listen("127.0.0.1:0", 5); + EXPECT_FALSE(error.Fail()); + + Socket *accept_socket; + std::future<Error> accept_error = std::async(std::launch::async, [&] { + return listen_socket.Accept("127.0.0.1:0", child_processes_inherit, accept_socket); + }); + + char connect_remote_address[64]; + snprintf(connect_remote_address, sizeof(connect_remote_address), "connect://localhost:%u", + listen_socket.GetLocalPortNumber()); + + std::unique_ptr<ConnectionFileDescriptor> conn_ap(new ConnectionFileDescriptor()); + EXPECT_EQ(conn_ap->Connect(connect_remote_address, nullptr), lldb::eConnectionStatusSuccess); + + client.SetConnection(conn_ap.release()); + EXPECT_TRUE(accept_error.get().Success()); + server.SetConnection(new ConnectionFileDescriptor(accept_socket)); + + listener_sp->StartListeningForEvents(&client, TestClient::eBroadcastBitRunPacketSent); +} + +} // end anonymous namespace + +class GDBRemoteClientBaseTest : public testing::Test +{ +public: + static void + SetUpTestCase() + { +#if defined(_MSC_VER) + WSADATA data; + ::WSAStartup(MAKEWORD(2, 2), &data); +#endif + } + + static void + TearDownTestCase() + { +#if defined(_MSC_VER) + ::WSACleanup(); +#endif + } +}; + +TEST(GDBRemoteClientBaseTest, SendContinueAndWait) +{ + StringExtractorGDBRemote response; + ContinueFixture fix; + if (HasFailure()) + return; + + // Continue. The inferior will stop with a signal. + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T01")); + ASSERT_EQ(eStateStopped, fix.SendCPacket(response)); + ASSERT_EQ("T01", response.GetStringRef()); + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("c", response.GetStringRef()); + + // Continue. The inferior will exit. + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("W01")); + ASSERT_EQ(eStateExited, fix.SendCPacket(response)); + ASSERT_EQ("W01", response.GetStringRef()); + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("c", response.GetStringRef()); + + // Continue. The inferior will get killed. + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("X01")); + ASSERT_EQ(eStateExited, fix.SendCPacket(response)); + ASSERT_EQ("X01", response.GetStringRef()); + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("c", response.GetStringRef()); +} + +TEST(GDBRemoteClientBaseTest, SendContinueAndAsyncSignal) +{ + StringExtractorGDBRemote continue_response, response; + ContinueFixture fix; + if (HasFailure()) + return; + + // SendAsyncSignal should do nothing when we are not running. + ASSERT_FALSE(fix.client.SendAsyncSignal(0x47)); + + // Continue. After the run packet is sent, send an async signal. + std::future<StateType> continue_state = + std::async(std::launch::async, [&] { return fix.SendCPacket(continue_response); }); + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("c", response.GetStringRef()); + fix.WaitForRunEvent(); + + std::future<bool> async_result = std::async(std::launch::async, [&] { return fix.client.SendAsyncSignal(0x47); }); + + // First we'll get interrupted. + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("\x03", response.GetStringRef()); + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T13")); + + // Then we get the signal packet. + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("C47", response.GetStringRef()); + ASSERT_TRUE(async_result.get()); + + // And we report back a signal stop. + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T47")); + ASSERT_EQ(eStateStopped, continue_state.get()); + ASSERT_EQ("T47", continue_response.GetStringRef()); +} + +TEST(GDBRemoteClientBaseTest, SendContinueAndAsyncPacket) +{ + StringExtractorGDBRemote continue_response, async_response, response; + const bool send_async = true; + ContinueFixture fix; + if (HasFailure()) + return; + + // Continue. After the run packet is sent, send an async packet. + std::future<StateType> continue_state = + std::async(std::launch::async, [&] { return fix.SendCPacket(continue_response); }); + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("c", response.GetStringRef()); + fix.WaitForRunEvent(); + + // Sending without async enabled should fail. + ASSERT_EQ(PacketResult::ErrorSendFailed, fix.client.SendPacketAndWaitForResponse("qTest1", response, !send_async)); + + std::future<PacketResult> async_result = std::async(std::launch::async, [&] { + return fix.client.SendPacketAndWaitForResponse("qTest2", async_response, send_async); + }); + + // First we'll get interrupted. + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("\x03", response.GetStringRef()); + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T13")); + + // Then we get the async packet. + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("qTest2", response.GetStringRef()); + + // Send the response and receive it. + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("QTest2")); + ASSERT_EQ(PacketResult::Success, async_result.get()); + ASSERT_EQ("QTest2", async_response.GetStringRef()); + + // And we get resumed again. + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("c", response.GetStringRef()); + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T01")); + ASSERT_EQ(eStateStopped, continue_state.get()); + ASSERT_EQ("T01", continue_response.GetStringRef()); +} + +TEST(GDBRemoteClientBaseTest, SendContinueAndInterrupt) +{ + StringExtractorGDBRemote continue_response, response; + ContinueFixture fix; + if (HasFailure()) + return; + + // Interrupt should do nothing when we're not running. + ASSERT_FALSE(fix.client.Interrupt()); + + // Continue. After the run packet is sent, send an interrupt. + std::future<StateType> continue_state = + std::async(std::launch::async, [&] { return fix.SendCPacket(continue_response); }); + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("c", response.GetStringRef()); + fix.WaitForRunEvent(); + + std::future<bool> async_result = std::async(std::launch::async, [&] { return fix.client.Interrupt(); }); + + // We get interrupted. + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("\x03", response.GetStringRef()); + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T13")); + + // And that's it. + ASSERT_EQ(eStateStopped, continue_state.get()); + ASSERT_EQ("T13", continue_response.GetStringRef()); + ASSERT_TRUE(async_result.get()); +} + +TEST(GDBRemoteClientBaseTest, SendContinueAndInterrupt2PacketBug) +{ + StringExtractorGDBRemote continue_response, async_response, response; + const bool send_async = true; + ContinueFixture fix; + if (HasFailure()) + return; + + // Interrupt should do nothing when we're not running. + ASSERT_FALSE(fix.client.Interrupt()); + + // Continue. After the run packet is sent, send an async signal. + std::future<StateType> continue_state = + std::async(std::launch::async, [&] { return fix.SendCPacket(continue_response); }); + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("c", response.GetStringRef()); + fix.WaitForRunEvent(); + + std::future<bool> interrupt_result = std::async(std::launch::async, [&] { return fix.client.Interrupt(); }); + + // We get interrupted. We'll send two packets to simulate a buggy stub. + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("\x03", response.GetStringRef()); + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T13")); + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T13")); + + // We should stop. + ASSERT_EQ(eStateStopped, continue_state.get()); + ASSERT_EQ("T13", continue_response.GetStringRef()); + ASSERT_TRUE(interrupt_result.get()); + + // Packet stream should remain synchronized. + std::future<PacketResult> send_result = std::async(std::launch::async, [&] { + return fix.client.SendPacketAndWaitForResponse("qTest", async_response, !send_async); + }); + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("qTest", response.GetStringRef()); + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("QTest")); + ASSERT_EQ(PacketResult::Success, send_result.get()); + ASSERT_EQ("QTest", async_response.GetStringRef()); +} + +TEST(GDBRemoteClientBaseTest, SendContinueDelegateInterface) +{ + StringExtractorGDBRemote response; + ContinueFixture fix; + if (HasFailure()) + return; + + // Continue. We'll have the server send a bunch of async packets before it stops. + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("O4142")); + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("Apro")); + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("O4344")); + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("Afile")); + ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T01")); + ASSERT_EQ(eStateStopped, fix.SendCPacket(response)); + ASSERT_EQ("T01", response.GetStringRef()); + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("c", response.GetStringRef()); + + EXPECT_EQ("ABCD", fix.delegate.output); + EXPECT_EQ("profile", fix.delegate.misc_data); + EXPECT_EQ(1u, fix.delegate.stop_reply_called); +} + +TEST(GDBRemoteClientBaseTest, InterruptNoResponse) +{ + StringExtractorGDBRemote continue_response, response; + ContinueFixture fix; + if (HasFailure()) + return; + + // Continue. After the run packet is sent, send an interrupt. + std::future<StateType> continue_state = + std::async(std::launch::async, [&] { return fix.SendCPacket(continue_response); }); + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("c", response.GetStringRef()); + fix.WaitForRunEvent(); + + std::future<bool> async_result = std::async(std::launch::async, [&] { return fix.client.Interrupt(); }); + + // We get interrupted, but we don't send a stop packet. + ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response)); + ASSERT_EQ("\x03", response.GetStringRef()); + + // The functions should still terminate (after a timeout). + ASSERT_TRUE(async_result.get()); + ASSERT_EQ(eStateInvalid, continue_state.get()); +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
