Skip to content

[lldb] Rework how we pass the execution context to the statusline #159887

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 23, 2025
Merged

[lldb] Rework how we pass the execution context to the statusline #159887

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4fe7308
[lldb] Rework how we pass the execution context to the statusline
JDevlieghere Sep 19, 2025
afdf9a7
Address Jim's feedback
JDevlieghere Sep 22, 2025
06817bb
Add workaround for issue #160216
JDevlieghere Sep 23, 2025
8966b2c
Add comment about having a frame in the StoppedExecutionContext
JDevlieghere Sep 23, 2025
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
13 changes: 10 additions & 3 deletions lldb/include/lldb/Core/Debugger.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,14 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
return m_target_list.GetSelectedTarget();
}

/// Get the execution context for the selected target.
ExecutionContext GetSelectedExecutionContext();

/// Similar to GetSelectedExecutionContext but returns a
/// ExecutionContextRef, and will hold the dummy target if no target is
/// currently selected.
ExecutionContextRef GetSelectedExecutionContextRef();

/// Get accessor for the target list.
///
/// The target list is part of the global debugger object. This the single
Expand Down Expand Up @@ -419,7 +426,7 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
void CancelInterruptRequest();

/// Redraw the statusline if enabled.
void RedrawStatusline(bool update = true);
void RedrawStatusline(std::optional<ExecutionContextRef> exe_ctx_ref);

/// This is the correct way to query the state of Interruption.
/// If you are on the RunCommandInterpreter thread, it will check the
Expand Down Expand Up @@ -701,9 +708,9 @@ class Debugger : public std::enable_shared_from_this<Debugger>,

void HandleBreakpointEvent(const lldb::EventSP &event_sp);

void HandleProcessEvent(const lldb::EventSP &event_sp);
lldb::ProcessSP HandleProcessEvent(const lldb::EventSP &event_sp);

void HandleThreadEvent(const lldb::EventSP &event_sp);
lldb::ThreadSP HandleThreadEvent(const lldb::EventSP &event_sp);

void HandleProgressEvent(const lldb::EventSP &event_sp);

Expand Down
17 changes: 12 additions & 5 deletions lldb/include/lldb/Core/Statusline.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#ifndef LLDB_CORE_STATUSLINE_H
#define LLDB_CORE_STATUSLINE_H

#include "lldb/Symbol/SymbolContext.h"
#include "lldb/Target/ExecutionContext.h"
#include "lldb/lldb-forward.h"
#include <cstdint>
#include <string>
Expand All @@ -19,15 +21,16 @@ class Statusline {
Statusline(Debugger &debugger);
~Statusline();

using Context = std::pair<ExecutionContextRef, SymbolContext>;

/// Reduce the scroll window and draw the statusline.
void Enable();
void Enable(std::optional<ExecutionContextRef> exe_ctx_ref);

/// Hide the statusline and extend the scroll window.
void Disable();

/// Redraw the statusline. If update is false, this will redraw the last
/// string.
void Redraw(bool update = true);
/// Redraw the statusline.
void Redraw(std::optional<ExecutionContextRef> exe_ctx_ref);

/// Inform the statusline that the terminal dimensions have changed.
void TerminalSizeChanged();
Expand All @@ -46,7 +49,11 @@ class Statusline {
void UpdateScrollWindow(ScrollWindowMode mode);

Debugger &m_debugger;
std::string m_last_str;

/// Cached copy of the execution context that allows us to redraw the
/// statusline.
ExecutionContextRef m_exe_ctx_ref;

uint64_t m_terminal_width = 0;
uint64_t m_terminal_height = 0;
};
Expand Down
56 changes: 37 additions & 19 deletions lldb/source/Core/Debugger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,16 +253,18 @@ Status Debugger::SetPropertyValue(const ExecutionContext *exe_ctx,
// Statusline setting changed. If we have a statusline instance, update it
// now. Otherwise it will get created in the default event handler.
std::lock_guard<std::mutex> guard(m_statusline_mutex);
if (StatuslineSupported())
if (StatuslineSupported()) {
m_statusline.emplace(*this);
else
m_statusline->Enable(GetSelectedExecutionContextRef());
} else {
m_statusline.reset();
}
} else if (property_path ==
g_debugger_properties[ePropertyStatuslineFormat].name ||
property_path ==
g_debugger_properties[ePropertySeparator].name) {
// Statusline format changed. Redraw the statusline.
RedrawStatusline();
RedrawStatusline(std::nullopt);
} else if (property_path ==
g_debugger_properties[ePropertyUseSourceCache].name) {
// use-source-cache changed. Wipe out the cache contents if it was
Expand Down Expand Up @@ -501,7 +503,7 @@ FormatEntity::Entry Debugger::GetStatuslineFormat() const {
bool Debugger::SetStatuslineFormat(const FormatEntity::Entry &format) {
constexpr uint32_t idx = ePropertyStatuslineFormat;
bool ret = SetPropertyAtIndex(idx, format);
RedrawStatusline();
RedrawStatusline(std::nullopt);
return ret;
}

Expand All @@ -526,7 +528,7 @@ llvm::StringRef Debugger::GetDisabledAnsiSuffix() const {
bool Debugger::SetSeparator(llvm::StringRef s) {
constexpr uint32_t idx = ePropertySeparator;
bool ret = SetPropertyAtIndex(idx, s);
RedrawStatusline();
RedrawStatusline(std::nullopt);
return ret;
}

Expand Down Expand Up @@ -1210,14 +1212,18 @@ void Debugger::RestoreInputTerminalState() {
{
std::lock_guard<std::mutex> guard(m_statusline_mutex);
if (m_statusline)
m_statusline->Enable();
m_statusline->Enable(GetSelectedExecutionContext());
}
}

void Debugger::RedrawStatusline(bool update) {
void Debugger::RedrawStatusline(
std::optional<ExecutionContextRef> exe_ctx_ref) {
std::lock_guard<std::mutex> guard(m_statusline_mutex);
if (m_statusline)
m_statusline->Redraw(update);

if (!m_statusline)
return;

m_statusline->Redraw(GetSelectedExecutionContextRef());
}

ExecutionContext Debugger::GetSelectedExecutionContext() {
Expand All @@ -1226,6 +1232,13 @@ ExecutionContext Debugger::GetSelectedExecutionContext() {
return ExecutionContext(exe_ctx_ref);
}

ExecutionContextRef Debugger::GetSelectedExecutionContextRef() {
if (TargetSP selected_target_sp = GetSelectedTarget())
return ExecutionContextRef(selected_target_sp.get(),
/*adopt_selected=*/true);
return ExecutionContextRef(m_dummy_target_sp.get(), /*adopt_selected=*/false);
}

void Debugger::DispatchInputInterrupt() {
std::lock_guard<std::recursive_mutex> guard(m_io_handler_stack.GetMutex());
IOHandlerSP reader_sp(m_io_handler_stack.Top());
Expand Down Expand Up @@ -1941,8 +1954,7 @@ void Debugger::FlushProcessOutput(Process &process, bool flush_stdout,
}

// This function handles events that were broadcast by the process.
void Debugger::HandleProcessEvent(const EventSP &event_sp) {
using namespace lldb;
ProcessSP Debugger::HandleProcessEvent(const EventSP &event_sp) {
const uint32_t event_type = event_sp->GetType();
ProcessSP process_sp =
(event_type == Process::eBroadcastBitStructuredData)
Expand Down Expand Up @@ -2024,23 +2036,24 @@ void Debugger::HandleProcessEvent(const EventSP &event_sp) {
if (pop_process_io_handler)
process_sp->PopProcessIOHandler();
}
return process_sp;
}

void Debugger::HandleThreadEvent(const EventSP &event_sp) {
ThreadSP Debugger::HandleThreadEvent(const EventSP &event_sp) {
// At present the only thread event we handle is the Frame Changed event, and
// all we do for that is just reprint the thread status for that thread.
using namespace lldb;
const uint32_t event_type = event_sp->GetType();
const bool stop_format = true;
ThreadSP thread_sp;
if (event_type == Thread::eBroadcastBitStackChanged ||
event_type == Thread::eBroadcastBitThreadSelected) {
ThreadSP thread_sp(
Thread::ThreadEventData::GetThreadFromEvent(event_sp.get()));
thread_sp = Thread::ThreadEventData::GetThreadFromEvent(event_sp.get());
if (thread_sp) {
thread_sp->GetStatus(*GetAsyncOutputStream(), 0, 1, 1, stop_format,
/*show_hidden*/ true);
}
}
return thread_sp;
}

bool Debugger::IsForwardingEvents() { return (bool)m_forward_listener_sp; }
Expand Down Expand Up @@ -2109,28 +2122,33 @@ lldb::thread_result_t Debugger::DefaultEventHandler() {

if (StatuslineSupported()) {
std::lock_guard<std::mutex> guard(m_statusline_mutex);
if (!m_statusline)
if (!m_statusline) {
m_statusline.emplace(*this);
m_statusline->Enable(GetSelectedExecutionContextRef());
}
}

bool done = false;
while (!done) {
EventSP event_sp;
if (listener_sp->GetEvent(event_sp, std::nullopt)) {
std::optional<ExecutionContextRef> exe_ctx_ref = std::nullopt;
if (event_sp) {
Broadcaster *broadcaster = event_sp->GetBroadcaster();
if (broadcaster) {
uint32_t event_type = event_sp->GetType();
ConstString broadcaster_class(broadcaster->GetBroadcasterClass());
if (broadcaster_class == broadcaster_class_process) {
HandleProcessEvent(event_sp);
if (ProcessSP process_sp = HandleProcessEvent(event_sp))
exe_ctx_ref = ExecutionContext(process_sp);
Copy link
Collaborator

@jimingham jimingham Sep 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

You're being a little inconsistent here.

ExecutionContext(lldb::ProcessSP &) returns an ExecutionContext with only the target & process shared pointers set. In other places where you use GetSelectedExecutionContextRef, you're passing in all the selected entities. That makes status line code that might want to show thread info have to guess whether this means it shouldn't show anything about threads because the caller on purpose didn't send in a particular thread, or if it should get that process' selected thread.

I think it would be easier on the status line formatting code if we always put as much detail as we know. Since this is a process event, then we should provide the selected Thread & Frame. For Thread events below, we should fill in the selected frame for the thread. And if there were to be frame events (we don't have those yet) that would determine everything above it.

That way when you're in the status line code you always know what the caller means.

Copy link
Member Author

@JDevlieghere JDevlieghere Sep 22, 2025

Choose a reason for hiding this comment

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

It's not only inconsistent, it's actually incorrect. I didn't notice because of the bug you spotted earlier where we weren't actually passing the computed execution context to the statusline. I originally misunderstood the meaning of ExecutionContext::Lokc(thread_and_frame_only_if_stopped). I thought the argument decided whether we computed the selected thread & frame, but it only decides whether to set it in the ExecutionContext, if they are present in the ExecutionContextRef.

Changing the Lock method could be another way to solve this. I fixed it the way you suggested, by adding overloads to the constructor for the Process & Thread that take an adopt_selected argument.

} else if (broadcaster_class == broadcaster_class_target) {
if (Breakpoint::BreakpointEventData::GetEventDataFromEvent(
event_sp.get())) {
HandleBreakpointEvent(event_sp);
}
} else if (broadcaster_class == broadcaster_class_thread) {
HandleThreadEvent(event_sp);
if (ThreadSP thread_sp = HandleThreadEvent(event_sp))
exe_ctx_ref = ExecutionContext(thread_sp);
} else if (broadcaster == m_command_interpreter_up.get()) {
if (event_type &
CommandInterpreter::eBroadcastBitQuitCommandReceived) {
Expand Down Expand Up @@ -2168,7 +2186,7 @@ lldb::thread_result_t Debugger::DefaultEventHandler() {
if (m_forward_listener_sp)
m_forward_listener_sp->AddEvent(event_sp);
}
RedrawStatusline();
RedrawStatusline(exe_ctx_ref);
}
}

Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Core/IOHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ void IOHandlerEditline::AutoCompleteCallback(CompletionRequest &request) {
}

void IOHandlerEditline::RedrawCallback() {
m_debugger.RedrawStatusline(/*update=*/false);
m_debugger.RedrawStatusline(std::nullopt);
}

#endif
Expand Down
57 changes: 25 additions & 32 deletions lldb/source/Core/Statusline.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ using namespace lldb_private;

Statusline::Statusline(Debugger &debugger)
: m_debugger(debugger), m_terminal_width(m_debugger.GetTerminalWidth()),
m_terminal_height(m_debugger.GetTerminalHeight()) {
Enable();
}
m_terminal_height(m_debugger.GetTerminalHeight()) {}

Statusline::~Statusline() { Disable(); }

Expand All @@ -47,16 +45,16 @@ void Statusline::TerminalSizeChanged() {

UpdateScrollWindow(ResizeStatusline);

// Draw the old statusline.
Redraw(/*update=*/false);
// Redraw the old statusline.
Redraw(std::nullopt);
}

void Statusline::Enable() {
void Statusline::Enable(std::optional<ExecutionContextRef> exe_ctx_ref) {
// Reduce the scroll window to make space for the status bar below.
UpdateScrollWindow(EnableStatusline);

// Draw the statusline.
Redraw(/*update=*/true);
Redraw(exe_ctx_ref);
}

void Statusline::Disable() {
Expand All @@ -69,8 +67,6 @@ void Statusline::Draw(std::string str) {
if (!stream_sp)
return;

m_last_str = str;

str = ansi::TrimAndPad(str, m_terminal_width);

LockedStreamFile locked_stream = stream_sp->Lock();
Expand Down Expand Up @@ -127,33 +123,30 @@ void Statusline::UpdateScrollWindow(ScrollWindowMode mode) {
m_debugger.RefreshIOHandler();
}

void Statusline::Redraw(bool update) {
if (!update) {
Draw(m_last_str);
return;
}

ExecutionContext exe_ctx = m_debugger.GetSelectedExecutionContext();

// For colors and progress events, the format entity needs access to the
// debugger, which requires a target in the execution context.
if (!exe_ctx.HasTargetScope())
exe_ctx.SetTargetPtr(&m_debugger.GetSelectedOrDummyTarget());

SymbolContext symbol_ctx;
if (ProcessSP process_sp = exe_ctx.GetProcessSP()) {
// Check if the process is stopped, and if it is, make sure it remains
// stopped until we've computed the symbol context.
Process::StopLocker stop_locker;
if (stop_locker.TryLock(&process_sp->GetRunLock())) {
if (auto frame_sp = exe_ctx.GetFrameSP())
symbol_ctx = frame_sp->GetSymbolContext(eSymbolContextEverything);
}
void Statusline::Redraw(std::optional<ExecutionContextRef> exe_ctx_ref) {
// Update the cached execution context.
if (exe_ctx_ref)
m_exe_ctx_ref = *exe_ctx_ref;

// Lock the execution context.
ExecutionContext exe_ctx =
m_exe_ctx_ref.Lock(/*thread_and_frame_only_if_stopped=*/true);

// Compute the symbol context if we're stopped.
SymbolContext sym_ctx;
llvm::Expected<StoppedExecutionContext> stopped_exe_ctx =
GetStoppedExecutionContext(&m_exe_ctx_ref);
if (stopped_exe_ctx) {
if (auto frame_sp = stopped_exe_ctx->GetFrameSP())
sym_ctx = frame_sp->GetSymbolContext(eSymbolContextEverything);
Copy link
Collaborator

@jimingham jimingham Sep 22, 2025

Choose a reason for hiding this comment

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

I kinda want an assert here if this isn't true. How could a StoppedExecutionContext not have a frame?

Copy link
Member Author

@JDevlieghere JDevlieghere Sep 22, 2025

Choose a reason for hiding this comment

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

A little counter-intuitively, the StoppedExecutionContext only ensures that we hold the run lock, so the process could be in an exited or unloaded state.

Copy link
Collaborator

@jimingham jimingham Sep 22, 2025

Choose a reason for hiding this comment

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

Ah, makes sense. Might want to add that as a comment, since that's not 100% obvious.

} else {
// We can draw the statusline without being stopped.
llvm::consumeError(stopped_exe_ctx.takeError());
}

StreamString stream;
FormatEntity::Entry format = m_debugger.GetStatuslineFormat();
FormatEntity::Format(format, stream, &symbol_ctx, &exe_ctx, nullptr, nullptr,
FormatEntity::Format(format, stream, &sym_ctx, &exe_ctx, nullptr, nullptr,
false, false);

Draw(stream.GetString().str());
Expand Down
Loading