[Lldb-commits] [lldb] 156cb4c - [lldb] Remove non-stop mode code
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Tue Sep 28 05:14:11 PDT 2021
Author: Pavel Labath
Date: 2021-09-28T14:13:50+02:00
New Revision: 156cb4cc64bec2b72aad9848f8181b338ff19ebc
URL: https://github.com/llvm/llvm-project/commit/156cb4cc64bec2b72aad9848f8181b338ff19ebc
DIFF: https://github.com/llvm/llvm-project/commit/156cb4cc64bec2b72aad9848f8181b338ff19ebc.diff
LOG: [lldb] Remove non-stop mode code
We added some support for this mode back in 2015, but the feature was
never productionized. It is completely untested, and there are known
major structural lldb issues that need to be resolved before this
feature can really be supported.
It also complicates making further changes to stop reply packet
handling, which is what I am about to do.
Differential Revision: https://reviews.llvm.org/D110553
Added:
Modified:
lldb/include/lldb/Target/Target.h
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td
Removed:
################################################################################
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index fc9dd97515aa9..4ddaf3fe2fca6 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -206,10 +206,6 @@ class TargetProperties : public Properties {
void SetUserSpecifiedTrapHandlerNames(const Args &args);
- bool GetNonStopModeEnabled() const;
-
- void SetNonStopModeEnabled(bool b);
-
bool GetDisplayRuntimeSupportValues() const;
void SetDisplayRuntimeSupportValues(bool b);
diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp
index 7247601b292b0..1534959989d3e 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -292,16 +292,10 @@ class ThreadStepScopeOptionGroup : public OptionGroup {
// Check if we are in Non-Stop mode
TargetSP target_sp =
execution_context ? execution_context->GetTargetSP() : TargetSP();
- if (target_sp && target_sp->GetNonStopModeEnabled()) {
- // NonStopMode runs all threads by definition, so when it is on we don't
- // need to check the process setting for runs all threads.
- m_run_mode = eOnlyThisThread;
- } else {
- ProcessSP process_sp =
- execution_context ? execution_context->GetProcessSP() : ProcessSP();
- if (process_sp && process_sp->GetSteppingRunsAllThreads())
- m_run_mode = eAllThreads;
- }
+ ProcessSP process_sp =
+ execution_context ? execution_context->GetProcessSP() : ProcessSP();
+ if (process_sp && process_sp->GetSteppingRunsAllThreads())
+ m_run_mode = eAllThreads;
m_avoid_regexp.clear();
m_step_in_target.clear();
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
index a4c71e864a767..803e5842cd7de 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
@@ -249,32 +249,6 @@ GDBRemoteClientBase::SendPacketAndWaitForResponseNoLock(
return packet_result;
}
-bool GDBRemoteClientBase::SendvContPacket(
- llvm::StringRef payload, std::chrono::seconds interrupt_timeout,
- StringExtractorGDBRemote &response) {
- Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
- LLDB_LOGF(log, "GDBRemoteCommunicationClient::%s ()", __FUNCTION__);
-
- // we want to lock down packet sending while we continue
- Lock lock(*this, interrupt_timeout);
-
- LLDB_LOGF(log,
- "GDBRemoteCommunicationClient::%s () sending vCont packet: %.*s",
- __FUNCTION__, int(payload.size()), payload.data());
-
- if (SendPacketNoLock(payload) != PacketResult::Success)
- return false;
-
- OnRunPacketSent(true);
-
- // wait for the response to the vCont
- if (ReadPacket(response, llvm::None, 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);
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
index 518b81318b6cc..43a5313eae6ad 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
@@ -59,10 +59,6 @@ class GDBRemoteClientBase : public GDBRemoteCommunication {
std::chrono::seconds interrupt_timeout,
llvm::function_ref<void(llvm::StringRef)> output_callback);
- bool SendvContPacket(llvm::StringRef payload,
- std::chrono::seconds interrupt_timeout,
- StringExtractorGDBRemote &response);
-
class Lock {
public:
// If interrupt_timeout == 0 seconds, only take the lock if the target is
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 805a836942d6b..a44e666427899 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2426,24 +2426,6 @@ bool GDBRemoteCommunicationClient::GetGroupName(uint32_t gid,
return false;
}
-bool GDBRemoteCommunicationClient::SetNonStopMode(const bool enable) {
- // Form non-stop packet request
- char packet[32];
- const int packet_len =
- ::snprintf(packet, sizeof(packet), "QNonStop:%1d", (int)enable);
- assert(packet_len < (int)sizeof(packet));
- UNUSED_IF_ASSERT_DISABLED(packet_len);
-
- StringExtractorGDBRemote response;
- // Send to target
- if (SendPacketAndWaitForResponse(packet, response) == PacketResult::Success)
- if (response.IsOKResponse())
- return true;
-
- // Failed or not supported
- return false;
-}
-
static void MakeSpeedTestPacket(StreamString &packet, uint32_t send_size,
uint32_t recv_size) {
packet.Clear();
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index 376adfeb95d0f..fd3fe1ce29969 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -315,8 +315,6 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
uint32_t length, // Byte Size of breakpoint or watchpoint
std::chrono::seconds interrupt_timeout); // Time to wait for an interrupt
- bool SetNonStopMode(const bool enable);
-
void TestPacketSpeed(const uint32_t num_packets, uint32_t max_send,
uint32_t max_recv, uint64_t recv_amount, bool json,
Stream &strm);
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index fdedb9b83416e..903c132dbf0ca 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -603,10 +603,6 @@ Status ProcessGDBRemote::DoConnectRemote(llvm::StringRef remote_url) {
if (m_gdb_comm.GetStopReply(response)) {
SetLastStopPacket(response);
- // '?' Packets must be handled
diff erently in non-stop mode
- if (GetTarget().GetNonStopModeEnabled())
- HandleStopReplySequence();
-
Target &target = GetTarget();
if (!target.GetArchitecture().IsValid()) {
if (m_gdb_comm.GetProcessArchitecture().IsValid()) {
@@ -846,9 +842,6 @@ Status ProcessGDBRemote::DoLaunch(lldb_private::Module *exe_module,
StringExtractorGDBRemote response;
if (m_gdb_comm.GetStopReply(response)) {
SetLastStopPacket(response);
- // '?' Packets must be handled
diff erently in non-stop mode
- if (GetTarget().GetNonStopModeEnabled())
- HandleStopReplySequence();
const ArchSpec &process_arch = m_gdb_comm.GetProcessArchitecture();
@@ -919,11 +912,6 @@ Status ProcessGDBRemote::ConnectToDebugserver(llvm::StringRef connect_url) {
return error;
}
- // Start the communications read thread so all incoming data can be parsed
- // into packets and queued as they arrive.
- if (GetTarget().GetNonStopModeEnabled())
- m_gdb_comm.StartReadThread();
-
// We always seem to be able to open a connection to a local port so we need
// to make sure we can then send data to it. If we can't then we aren't
// actually connected to anything, so try and do the handshake with the
@@ -935,10 +923,6 @@ Status ProcessGDBRemote::ConnectToDebugserver(llvm::StringRef connect_url) {
return error;
}
- // Send $QNonStop:1 packet on startup if required
- if (GetTarget().GetNonStopModeEnabled())
- GetTarget().SetNonStopModeEnabled(m_gdb_comm.SetNonStopMode(true));
-
m_gdb_comm.GetEchoSupported();
m_gdb_comm.GetThreadSuffixSupported();
m_gdb_comm.GetListThreadsInStopReplySupported();
@@ -947,10 +931,6 @@ Status ProcessGDBRemote::ConnectToDebugserver(llvm::StringRef connect_url) {
m_gdb_comm.GetVAttachOrWaitSupported();
m_gdb_comm.EnableErrorStringInPacket();
- // Ask the remote server for the default thread id
- if (GetTarget().GetNonStopModeEnabled())
- m_gdb_comm.GetDefaultThreadId(m_initial_tid);
-
size_t num_cmds = GetExtraStartupCommands().GetArgumentCount();
for (size_t idx = 0; idx < num_cmds; idx++) {
StringExtractorGDBRemote response;
@@ -1210,10 +1190,9 @@ Status ProcessGDBRemote::DoResume() {
StreamString continue_packet;
bool continue_packet_error = false;
if (m_gdb_comm.HasAnyVContSupport()) {
- if (!GetTarget().GetNonStopModeEnabled() &&
- (m_continue_c_tids.size() == num_threads ||
- (m_continue_c_tids.empty() && m_continue_C_tids.empty() &&
- m_continue_s_tids.empty() && m_continue_S_tids.empty()))) {
+ if (m_continue_c_tids.size() == num_threads ||
+ (m_continue_c_tids.empty() && m_continue_C_tids.empty() &&
+ m_continue_s_tids.empty() && m_continue_S_tids.empty())) {
// All threads are continuing, just send a "c" packet
continue_packet.PutCString("c");
} else {
@@ -1331,14 +1310,7 @@ Status ProcessGDBRemote::DoResume() {
// All threads are resuming...
m_gdb_comm.SetCurrentThreadForRun(-1);
- // If in Non-Stop-Mode use vCont when stepping
- if (GetTarget().GetNonStopModeEnabled()) {
- if (m_gdb_comm.GetVContSupported('s'))
- continue_packet.PutCString("vCont;s");
- else
- continue_packet.PutChar('s');
- } else
- continue_packet.PutChar('s');
+ continue_packet.PutChar('s');
continue_packet_error = false;
} else if (num_continue_c_tids == 0 && num_continue_C_tids == 0 &&
@@ -1495,12 +1467,9 @@ bool ProcessGDBRemote::UpdateThreadIDList() {
std::unique_lock<std::recursive_mutex> stop_stack_lock(
m_last_stop_packet_mutex, std::defer_lock);
if (stop_stack_lock.try_lock()) {
- // Get the number of stop packets on the stack
- int nItems = m_stop_packet_stack.size();
- // Iterate over them
- for (int i = 0; i < nItems; i++) {
+ if (m_last_stop_packet) {
// Get the thread stop info
- StringExtractorGDBRemote &stop_info = m_stop_packet_stack[i];
+ StringExtractorGDBRemote &stop_info = *m_last_stop_packet;
const std::string &stop_info_str =
std::string(stop_info.GetStringRef());
@@ -2352,17 +2321,9 @@ void ProcessGDBRemote::RefreshStateAfterStop() {
{
// Lock the thread stack while we access it
std::lock_guard<std::recursive_mutex> guard(m_last_stop_packet_mutex);
- // Get the number of stop packets on the stack
- int nItems = m_stop_packet_stack.size();
- // Iterate over them
- for (int i = 0; i < nItems; i++) {
- // Get the thread stop info
- StringExtractorGDBRemote stop_info = m_stop_packet_stack[i];
- // Process thread stop info
- SetThreadStopInfo(stop_info);
- }
- // Clear the thread stop stack
- m_stop_packet_stack.clear();
+ if (m_last_stop_packet)
+ SetThreadStopInfo(*m_last_stop_packet);
+ m_last_stop_packet.reset();
}
// If we have queried for a default thread id
@@ -2615,14 +2576,7 @@ void ProcessGDBRemote::SetLastStopPacket(
// Lock the thread stack while we access it
std::lock_guard<std::recursive_mutex> guard(m_last_stop_packet_mutex);
- // We are are not using non-stop mode, there can only be one last stop
- // reply packet, so clear the list.
- if (!GetTarget().GetNonStopModeEnabled())
- m_stop_packet_stack.clear();
-
- // Add this stop packet to the stop packet stack This stack will get popped
- // and examined when we switch to the Stopped state
- m_stop_packet_stack.push_back(response);
+ m_last_stop_packet = response;
}
}
@@ -3740,88 +3694,72 @@ thread_result_t ProcessGDBRemote::AsyncThread(void *arg) {
process->SetPrivateState(eStateRunning);
StringExtractorGDBRemote response;
- // If in Non-Stop-Mode
- if (process->GetTarget().GetNonStopModeEnabled()) {
- // send the vCont packet
- if (!process->GetGDBRemote().SendvContPacket(
- llvm::StringRef(continue_cstr, continue_cstr_len),
- process->GetInterruptTimeout(), response)) {
- // Something went wrong
- done = true;
- break;
- }
- }
- // If in All-Stop-Mode
- else {
- StateType stop_state =
- process->GetGDBRemote().SendContinuePacketAndWaitForResponse(
- *process, *process->GetUnixSignals(),
- llvm::StringRef(continue_cstr, continue_cstr_len),
- process->GetInterruptTimeout(),
- 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
- // caused the stop. So clear it now before we give the stop reply
- // packet to the process using the
- // process->SetLastStopPacket()...
- process->ClearThreadIDList();
+ StateType stop_state =
+ process->GetGDBRemote().SendContinuePacketAndWaitForResponse(
+ *process, *process->GetUnixSignals(),
+ llvm::StringRef(continue_cstr, continue_cstr_len),
+ process->GetInterruptTimeout(), 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
+ // caused the stop. So clear it now before we give the stop reply
+ // packet to the process using the
+ // process->SetLastStopPacket()...
+ process->ClearThreadIDList();
+
+ switch (stop_state) {
+ case eStateStopped:
+ case eStateCrashed:
+ case eStateSuspended:
+ process->SetLastStopPacket(response);
+ process->SetPrivateState(stop_state);
+ break;
- switch (stop_state) {
- case eStateStopped:
- case eStateCrashed:
- case eStateSuspended:
- process->SetLastStopPacket(response);
- process->SetPrivateState(stop_state);
- break;
-
- case eStateExited: {
- process->SetLastStopPacket(response);
- process->ClearThreadIDList();
- response.SetFilePos(1);
-
- int exit_status = response.GetHexU8();
- std::string desc_string;
- if (response.GetBytesLeft() > 0 &&
- response.GetChar('-') == ';') {
- llvm::StringRef desc_str;
- llvm::StringRef desc_token;
- while (response.GetNameColonValue(desc_token, desc_str)) {
- if (desc_token != "description")
- continue;
- StringExtractor extractor(desc_str);
- extractor.GetHexByteString(desc_string);
- }
+ case eStateExited: {
+ process->SetLastStopPacket(response);
+ process->ClearThreadIDList();
+ response.SetFilePos(1);
+
+ int exit_status = response.GetHexU8();
+ std::string desc_string;
+ if (response.GetBytesLeft() > 0 && response.GetChar('-') == ';') {
+ llvm::StringRef desc_str;
+ llvm::StringRef desc_token;
+ while (response.GetNameColonValue(desc_token, desc_str)) {
+ if (desc_token != "description")
+ continue;
+ StringExtractor extractor(desc_str);
+ extractor.GetHexByteString(desc_string);
}
- process->SetExitStatus(exit_status, desc_string.c_str());
- done = true;
- break;
}
- case eStateInvalid: {
- // Check to see if we were trying to attach and if we got back
- // the "E87" error code from debugserver -- this indicates that
- // the process is not debuggable. Return a slightly more
- // helpful error message about why the attach failed.
- if (::strstr(continue_cstr, "vAttach") != nullptr &&
- response.GetError() == 0x87) {
- process->SetExitStatus(-1, "cannot attach to process due to "
- "System Integrity Protection");
- } else if (::strstr(continue_cstr, "vAttach") != nullptr &&
- response.GetStatus().Fail()) {
- process->SetExitStatus(-1, response.GetStatus().AsCString());
- } else {
- process->SetExitStatus(-1, "lost connection");
- }
- done = true;
- break;
+ process->SetExitStatus(exit_status, desc_string.c_str());
+ done = true;
+ break;
+ }
+ case eStateInvalid: {
+ // Check to see if we were trying to attach and if we got back
+ // the "E87" error code from debugserver -- this indicates that
+ // the process is not debuggable. Return a slightly more
+ // helpful error message about why the attach failed.
+ if (::strstr(continue_cstr, "vAttach") != nullptr &&
+ response.GetError() == 0x87) {
+ process->SetExitStatus(-1, "cannot attach to process due to "
+ "System Integrity Protection");
+ } else if (::strstr(continue_cstr, "vAttach") != nullptr &&
+ response.GetStatus().Fail()) {
+ process->SetExitStatus(-1, response.GetStatus().AsCString());
+ } else {
+ process->SetExitStatus(-1, "lost connection");
}
+ done = true;
+ break;
+ }
- default:
- process->SetPrivateState(stop_state);
- break;
- } // switch(stop_state)
- } // else // if in All-stop-mode
+ default:
+ process->SetPrivateState(stop_state);
+ break;
+ } // switch(stop_state)
} // if (continue_packet)
} // case eBroadcastBitAsyncContinue
break;
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
index b4eb8e5a6b982..962bb807d9641 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -269,11 +269,10 @@ class ProcessGDBRemote : public Process,
GDBRemoteCommunicationClient m_gdb_comm;
GDBRemoteCommunicationReplayServer m_gdb_replay_server;
std::atomic<lldb::pid_t> m_debugserver_pid;
- std::vector<StringExtractorGDBRemote> m_stop_packet_stack; // The stop packet
- // stack replaces
- // the last stop
- // packet variable
+
+ llvm::Optional<StringExtractorGDBRemote> m_last_stop_packet;
std::recursive_mutex m_last_stop_packet_mutex;
+
GDBRemoteDynamicRegisterInfoSP m_register_info_sp;
Broadcaster m_async_broadcaster;
lldb::ListenerSP m_async_listener_sp;
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index e24aa58fb9409..801906d020ce9 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -4288,16 +4288,6 @@ void TargetProperties::SetDisplayRecognizedArguments(bool b) {
m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
}
-bool TargetProperties::GetNonStopModeEnabled() const {
- const uint32_t idx = ePropertyNonStopModeEnabled;
- return m_collection_sp->GetPropertyAtIndexAsBoolean(nullptr, idx, false);
-}
-
-void TargetProperties::SetNonStopModeEnabled(bool b) {
- const uint32_t idx = ePropertyNonStopModeEnabled;
- m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
-}
-
const ProcessLaunchInfo &TargetProperties::GetProcessLaunchInfo() const {
return m_launch_info;
}
diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td
index 23f18fcf0c5e4..063ba0a6c25af 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -163,9 +163,6 @@ let Definition = "target" in {
def DisplayRecognizedArguments: Property<"display-recognized-arguments", "Boolean">,
DefaultFalse,
Desc<"Show recognized arguments in variable listings by default.">;
- def NonStopModeEnabled: Property<"non-stop-mode", "Boolean">,
- DefaultFalse,
- Desc<"Disable lock-step debugging, instead control threads independently.">;
def RequireHardwareBreakpoints: Property<"require-hardware-breakpoint", "Boolean">,
DefaultFalse,
Desc<"Require all breakpoints to be hardware breakpoints.">;
More information about the lldb-commits
mailing list