Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lldb/include/lldb/Host/common/NativeProcessProtocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,9 @@ class NativeProcessProtocol {
memory_tagging = (1u << 6),
savecore = (1u << 7),
siginfo_read = (1u << 8),
resume_without_disabling_breakpoints = (1u << 9),

LLVM_MARK_AS_BITMASK_ENUM(siginfo_read)
LLVM_MARK_AS_BITMASK_ENUM(resume_without_disabling_breakpoints)
};

class Manager {
Expand Down
2 changes: 2 additions & 0 deletions lldb/include/lldb/Target/Process.h
Original file line number Diff line number Diff line change
Expand Up @@ -3076,6 +3076,8 @@ void PruneThreadPlans();
/// false otherwise.
virtual bool SupportsMemoryTagging() { return false; }

virtual bool SupportsResumeWithoutDisablingBreakpoints() { return false; }

/// Does the final operation to read memory tags. E.g. sending a GDB packet.
/// It assumes that ReadMemoryTags has checked that memory tagging is enabled
/// and has expanded the memory range as needed.
Expand Down
11 changes: 11 additions & 0 deletions lldb/include/lldb/Target/ThreadList.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,17 @@ class ThreadList : public ThreadCollection {
///
bool WillResume(lldb::RunDirection &direction);

/// Set up a step-over breakpoint plan before resuming if needed..
///
/// \param[in] thread_to_run
/// An optional thread that will have the run priority in the resume.
///
/// \param[in] direction
/// The direction to run in.
void
SetUpStepOverBreakpointBeforeResumeIfNeeded(lldb::ThreadSP thread_to_run,
lldb::RunDirection &direction);

void DidResume();

void DidStop();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
m_supports_reverse_continue = eLazyBoolNo;
m_supports_reverse_step = eLazyBoolNo;
m_supports_multi_mem_read = eLazyBoolNo;
m_supports_resume_without_disabling_breakpoints = eLazyBoolNo;

m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
// not, we assume no limit
Expand Down Expand Up @@ -434,6 +435,8 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
m_supports_reverse_step = eLazyBoolYes;
else if (x == "MultiMemRead+")
m_supports_multi_mem_read = eLazyBoolYes;
else if (x == "resume-without-disabling-breakpoints+")
m_supports_resume_without_disabling_breakpoints = eLazyBoolYes;
// Look for a list of compressions in the features list e.g.
// qXfer:features:read+;PacketSize=20000;qEcho+;SupportedCompressions=zlib-
// deflate,lzma
Expand Down Expand Up @@ -686,6 +689,13 @@ bool GDBRemoteCommunicationClient::GetMemoryTaggingSupported() {
return m_supports_memory_tagging == eLazyBoolYes;
}

bool GDBRemoteCommunicationClient::
GetResumeWithoutDisablingBreakpointsSupported() {
if (m_supports_resume_without_disabling_breakpoints == eLazyBoolCalculate)
GetRemoteQSupported();
return m_supports_resume_without_disabling_breakpoints == eLazyBoolYes;
}

DataBufferSP GDBRemoteCommunicationClient::ReadMemoryTags(lldb::addr_t addr,
size_t len,
int32_t type) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,8 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {

bool GetMemoryTaggingSupported();

bool GetResumeWithoutDisablingBreakpointsSupported();

bool UsesNativeSignals();

lldb::DataBufferSP ReadMemoryTags(lldb::addr_t addr, size_t len,
Expand Down Expand Up @@ -577,6 +579,7 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
LazyBool m_supports_reverse_continue = eLazyBoolCalculate;
LazyBool m_supports_reverse_step = eLazyBoolCalculate;
LazyBool m_supports_multi_mem_read = eLazyBoolCalculate;
LazyBool m_supports_resume_without_disabling_breakpoints = eLazyBoolCalculate;

bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
m_supports_qUserName : 1, m_supports_qGroupName : 1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4326,6 +4326,8 @@ std::vector<std::string> GDBRemoteCommunicationServerLLGS::HandleFeatures(
ret.push_back("memory-tagging+");
if (bool(plugin_features & Extension::savecore))
ret.push_back("qSaveCore+");
if (bool(plugin_features & Extension::resume_without_disabling_breakpoints))
ret.push_back("resume-without-disabling-breakpoints+");

// check for client features
m_extensions_supported = {};
Expand Down
4 changes: 4 additions & 0 deletions lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2869,6 +2869,10 @@ bool ProcessGDBRemote::SupportsMemoryTagging() {
return m_gdb_comm.GetMemoryTaggingSupported();
}

bool ProcessGDBRemote::SupportsResumeWithoutDisablingBreakpoints() {
return m_gdb_comm.GetResumeWithoutDisablingBreakpointsSupported();
}

llvm::Expected<std::vector<uint8_t>>
ProcessGDBRemote::DoReadMemoryTags(lldb::addr_t addr, size_t len,
int32_t type) {
Expand Down
4 changes: 4 additions & 0 deletions lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,10 @@ class ProcessGDBRemote : public Process,

bool SupportsMemoryTagging() override;

/// \returns true if the process doesn't need the client to disable
/// breakpoints before issuing a resume operation.
bool SupportsResumeWithoutDisablingBreakpoints() override;

/// Broadcaster event bits definitions.
enum {
eBroadcastBitAsyncContinue = (1 << 0),
Expand Down
84 changes: 44 additions & 40 deletions lldb/source/Target/ThreadList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,49 @@ void ThreadList::DiscardThreadPlans() {
(*pos)->DiscardThreadPlans(true);
}

void ThreadList::SetUpStepOverBreakpointBeforeResumeIfNeeded(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jimingham , I ended up removing some comments that seems not to reflect exactly the source code here. Let me know what you think

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's better. It still doesn't stop someone from adding so code below this that isn't breakpoint related, which will then work on every platform but yours because they didn't notice this return. If the early return & breakpoint handling were in a separate method, then that mistake wouldn't be possible.
Not terribly important, however.

ThreadSP thread_to_run, RunDirection &direction) {
// If the process doesn't need the client to disable breakpoints before
// issuing a resume operation, we can skip the step-over breakpoint plan
// setup.
if (!m_process.SupportsResumeWithoutDisablingBreakpoints())
return;

// Give all the threads that are likely to run a chance to set up their
// step over breakpoint behavior.
if (thread_to_run != nullptr) {
if (thread_to_run->SetupToStepOverBreakpointIfNeeded(direction)) {
// We only need to step over breakpoints when running forward, and the
// step-over-breakpoint plan itself wants to run forward, so this
// keeps our desired direction.
assert(thread_to_run->GetCurrentPlan()->GetDirection() == direction);
}
} else {
collection::iterator end = m_threads.end();
for (auto pos = m_threads.begin(); pos != end; ++pos) {
ThreadSP thread_sp(*pos);
if (thread_sp->GetResumeState() != eStateSuspended) {
if (thread_sp->IsOperatingSystemPluginThread() &&
!thread_sp->GetBackingThread())
continue;
if (thread_sp->SetupToStepOverBreakpointIfNeeded(direction)) {
// We only need to step over breakpoints when running forward, and the
// step-over-breakpoint plan itself wants to run forward, so this
// keeps our desired direction.
assert(thread_sp->GetCurrentPlan()->GetDirection() == direction);
// You can't say "stop others" and also want yourself to be suspended.
assert(thread_sp->GetCurrentPlan()->RunState() != eStateSuspended);
thread_to_run = thread_sp;
if (thread_sp->ShouldRunBeforePublicStop()) {
// This takes precedence, so if we find one of these, service it:
break;
}
}
}
}
}
}

bool ThreadList::WillResume(RunDirection &direction) {
// Run through the threads and perform their momentary actions. But we only
// do this for threads that are running, user suspended threads stay where
Expand Down Expand Up @@ -558,46 +601,7 @@ bool ThreadList::WillResume(RunDirection &direction) {
direction = m_process.GetBaseDirection();
}

// Give all the threads that are likely to run a last chance to set up their
// state before we negotiate who is actually going to get a chance to run...
// Don't set to resume suspended threads, and if any thread wanted to stop
// others, only call setup on the threads that request StopOthers...
if (thread_to_run != nullptr) {
// See if any thread wants to run stopping others. If it does, then we
// won't setup the other threads for resume, since they aren't going to get
// a chance to run. This is necessary because the SetupForResume might add
// "StopOthers" plans which would then get to be part of the who-gets-to-run
// negotiation, but they're coming in after the fact, and the threads that
// are already set up should take priority.
if (thread_to_run->SetupToStepOverBreakpointIfNeeded(direction)) {
// We only need to step over breakpoints when running forward, and the
// step-over-breakpoint plan itself wants to run forward, so this
// keeps our desired direction.
assert(thread_to_run->GetCurrentPlan()->GetDirection() == direction);
}
} else {
for (pos = m_threads.begin(); pos != end; ++pos) {
ThreadSP thread_sp(*pos);
if (thread_sp->GetResumeState() != eStateSuspended) {
if (thread_sp->IsOperatingSystemPluginThread() &&
!thread_sp->GetBackingThread())
continue;
if (thread_sp->SetupToStepOverBreakpointIfNeeded(direction)) {
// We only need to step over breakpoints when running forward, and the
// step-over-breakpoint plan itself wants to run forward, so this
// keeps our desired direction.
assert(thread_sp->GetCurrentPlan()->GetDirection() == direction);
// You can't say "stop others" and also want yourself to be suspended.
assert(thread_sp->GetCurrentPlan()->RunState() != eStateSuspended);
thread_to_run = thread_sp;
if (thread_sp->ShouldRunBeforePublicStop()) {
// This takes precedence, so if we find one of these, service it:
break;
}
}
}
}
}
SetUpStepOverBreakpointBeforeResumeIfNeeded(thread_to_run, direction);

if (thread_to_run != nullptr) {
Log *log = GetLog(LLDBLog::Step);
Expand Down
Loading