Skip to content

Commit

Permalink
[lldb] Remove non-stop mode code
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@labath
labath committed Sep 28, 2021
1 parent 6359a4c commit 156cb4c
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 214 deletions.
4 changes: 0 additions & 4 deletions lldb/include/lldb/Target/Target.h
View file
Expand Up @@ -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);
Expand Down
14 changes: 4 additions & 10 deletions lldb/source/Commands/CommandObjectThread.cpp
View file
Expand Up @@ -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();
Expand Down
26 changes: 0 additions & 26 deletions lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
View file
Expand Up @@ -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);
Expand Down
4 changes: 0 additions & 4 deletions lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
View file
Expand Up @@ -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
Expand Down
18 changes: 0 additions & 18 deletions lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
View file
Expand Up @@ -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();
Expand Down
2 changes: 0 additions & 2 deletions lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
View file
Expand Up @@ -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);
Expand Down
204 changes: 71 additions & 133 deletions lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
View file
Expand Up @@ -603,10 +603,6 @@ Status ProcessGDBRemote::DoConnectRemote(llvm::StringRef remote_url) {
if (m_gdb_comm.GetStopReply(response)) {
SetLastStopPacket(response);

// '?' Packets must be handled differently in non-stop mode
if (GetTarget().GetNonStopModeEnabled())
HandleStopReplySequence();

Target &target = GetTarget();
if (!target.GetArchitecture().IsValid()) {
if (m_gdb_comm.GetProcessArchitecture().IsValid()) {
Expand Down Expand Up @@ -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 differently in non-stop mode
if (GetTarget().GetNonStopModeEnabled())
HandleStopReplySequence();

const ArchSpec &process_arch = m_gdb_comm.GetProcessArchitecture();

Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand All @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 &&
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 156cb4c

Please sign in to comment.