https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/148994
>From a4034e2f0948bfa61bfbf681bd5d9355aeb09beb Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Tue, 15 Jul 2025 16:54:05 -0700 Subject: [PATCH] [lldb] Always compute the execution & symbol context Always compute the execution and symbol context, regardless of whether the statusline is enabled. This code gets called from the default event handler thread and has uncovered threading issues that otherwise only reproduce when the statusline is enabled. --- lldb/include/lldb/Core/Debugger.h | 8 ++++++ lldb/include/lldb/Core/Statusline.h | 11 +++++--- lldb/source/Core/Debugger.cpp | 40 ++++++++++++++++++++++++++-- lldb/source/Core/Statusline.cpp | 41 +++++++++-------------------- 4 files changed, 65 insertions(+), 35 deletions(-) diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index 250ad64b76d9a..083f6519bf090 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -181,7 +181,15 @@ class Debugger : public std::enable_shared_from_this<Debugger>, return m_target_list.GetSelectedTarget(); } + /// Get the execution context for the selected target. ExecutionContext GetSelectedExecutionContext(); + + struct SelectedContext { + ExecutionContext exe_ctx; + std::optional<SymbolContext> sym_ctx; + }; + SelectedContext GetSelectedContext(); + /// Get accessor for the target list. /// /// The target list is part of the global debugger object. This the single diff --git a/lldb/include/lldb/Core/Statusline.h b/lldb/include/lldb/Core/Statusline.h index 6bda153f822d2..1c5a74e24f793 100644 --- a/lldb/include/lldb/Core/Statusline.h +++ b/lldb/include/lldb/Core/Statusline.h @@ -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> @@ -25,9 +27,9 @@ class Statusline { /// 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. If both exe_ctx and sym_ctx are NULL, this redraws + /// the last string. + void Redraw(const ExecutionContext *exe_ctx, const SymbolContext *sym_ctx); /// Inform the statusline that the terminal dimensions have changed. void TerminalSizeChanged(); @@ -46,7 +48,8 @@ class Statusline { void UpdateScrollWindow(ScrollWindowMode mode); Debugger &m_debugger; - std::string m_last_str; + ExecutionContext m_exe_ctx; + SymbolContext m_symbol_ctx; uint64_t m_terminal_width = 0; uint64_t m_terminal_height = 0; }; diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index ed674ee1275c7..5c4afe70e1062 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -1216,8 +1216,20 @@ void Debugger::RestoreInputTerminalState() { void Debugger::RedrawStatusline(bool update) { std::lock_guard<std::mutex> guard(m_statusline_mutex); + + if (!update && m_statusline) { + m_statusline->Redraw(nullptr, nullptr); + return; + } + + // Always compute the execution and symbol context, regardless of whether the + // statusline is enabled. This code gets called from the default event handler + // thread and has uncovered threading issues that otherwise only reproduce + // when the statusline is enabled. + SelectedContext ctx = GetSelectedContext(); if (m_statusline) - m_statusline->Redraw(update); + m_statusline->Redraw(&ctx.exe_ctx, + ctx.sym_ctx ? &ctx.sym_ctx.value() : nullptr); } ExecutionContext Debugger::GetSelectedExecutionContext() { @@ -1226,6 +1238,27 @@ ExecutionContext Debugger::GetSelectedExecutionContext() { return ExecutionContext(exe_ctx_ref); } +Debugger::SelectedContext Debugger::GetSelectedContext() { + SelectedContext context; + context.exe_ctx = GetSelectedExecutionContext(); + + if (!context.exe_ctx.HasTargetScope()) + context.exe_ctx.SetTargetPtr(&GetSelectedOrDummyTarget()); + + if (ProcessSP process_sp = context.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 = context.exe_ctx.GetFrameSP()) + context.sym_ctx.emplace( + frame_sp->GetSymbolContext(eSymbolContextEverything)); + } + } + + return context; +} + void Debugger::DispatchInputInterrupt() { std::lock_guard<std::recursive_mutex> guard(m_io_handler_stack.GetMutex()); IOHandlerSP reader_sp(m_io_handler_stack.Top()); @@ -2117,6 +2150,7 @@ lldb::thread_result_t Debugger::DefaultEventHandler() { while (!done) { EventSP event_sp; if (listener_sp->GetEvent(event_sp, std::nullopt)) { + bool update_statusline = false; if (event_sp) { Broadcaster *broadcaster = event_sp->GetBroadcaster(); if (broadcaster) { @@ -2124,6 +2158,8 @@ lldb::thread_result_t Debugger::DefaultEventHandler() { ConstString broadcaster_class(broadcaster->GetBroadcasterClass()); if (broadcaster_class == broadcaster_class_process) { HandleProcessEvent(event_sp); + update_statusline = + (event_type & Process::eBroadcastBitStateChanged) != 0; } else if (broadcaster_class == broadcaster_class_target) { if (Breakpoint::BreakpointEventData::GetEventDataFromEvent( event_sp.get())) { @@ -2168,7 +2204,7 @@ lldb::thread_result_t Debugger::DefaultEventHandler() { if (m_forward_listener_sp) m_forward_listener_sp->AddEvent(event_sp); } - RedrawStatusline(); + RedrawStatusline(update_statusline); } } diff --git a/lldb/source/Core/Statusline.cpp b/lldb/source/Core/Statusline.cpp index 393d427241021..e6ff3e5844270 100644 --- a/lldb/source/Core/Statusline.cpp +++ b/lldb/source/Core/Statusline.cpp @@ -47,8 +47,8 @@ void Statusline::TerminalSizeChanged() { UpdateScrollWindow(ResizeStatusline); - // Draw the old statusline. - Redraw(/*update=*/false); + // Redraw the old statusline. + Redraw(nullptr, nullptr); } void Statusline::Enable() { @@ -56,7 +56,8 @@ void Statusline::Enable() { UpdateScrollWindow(EnableStatusline); // Draw the statusline. - Redraw(/*update=*/true); + Debugger::SelectedContext ctx = m_debugger.GetSelectedContext(); + Redraw(&ctx.exe_ctx, ctx.sym_ctx ? &ctx.sym_ctx.value() : nullptr); } void Statusline::Disable() { @@ -69,8 +70,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(); @@ -127,34 +126,18 @@ void Statusline::UpdateScrollWindow(ScrollWindowMode mode) { m_debugger.RefreshIOHandler(); } -void Statusline::Redraw(bool update) { - if (!update) { - Draw(m_last_str); - return; - } +void Statusline::Redraw(const ExecutionContext *exe_ctx, + const SymbolContext *sym_ctx) { + if (exe_ctx) + m_exe_ctx = *exe_ctx; - 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); - } - } + if (sym_ctx) + m_symbol_ctx = *sym_ctx; StreamString stream; FormatEntity::Entry format = m_debugger.GetStatuslineFormat(); - FormatEntity::Format(format, stream, &symbol_ctx, &exe_ctx, nullptr, nullptr, - false, false); + FormatEntity::Format(format, stream, &m_symbol_ctx, &m_exe_ctx, nullptr, + nullptr, false, false); Draw(stream.GetString().str()); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits