Skip to content

Commit a4034e2

Browse files
committed
[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.
1 parent 2cb1ecb commit a4034e2

File tree

4 files changed

+65
-35
lines changed

4 files changed

+65
-35
lines changed

lldb/include/lldb/Core/Debugger.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,15 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
181181
return m_target_list.GetSelectedTarget();
182182
}
183183

184+
/// Get the execution context for the selected target.
184185
ExecutionContext GetSelectedExecutionContext();
186+
187+
struct SelectedContext {
188+
ExecutionContext exe_ctx;
189+
std::optional<SymbolContext> sym_ctx;
190+
};
191+
SelectedContext GetSelectedContext();
192+
185193
/// Get accessor for the target list.
186194
///
187195
/// The target list is part of the global debugger object. This the single

lldb/include/lldb/Core/Statusline.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
#ifndef LLDB_CORE_STATUSLINE_H
1010
#define LLDB_CORE_STATUSLINE_H
1111

12+
#include "lldb/Symbol/SymbolContext.h"
13+
#include "lldb/Target/ExecutionContext.h"
1214
#include "lldb/lldb-forward.h"
1315
#include <cstdint>
1416
#include <string>
@@ -25,9 +27,9 @@ class Statusline {
2527
/// Hide the statusline and extend the scroll window.
2628
void Disable();
2729

28-
/// Redraw the statusline. If update is false, this will redraw the last
29-
/// string.
30-
void Redraw(bool update = true);
30+
/// Redraw the statusline. If both exe_ctx and sym_ctx are NULL, this redraws
31+
/// the last string.
32+
void Redraw(const ExecutionContext *exe_ctx, const SymbolContext *sym_ctx);
3133

3234
/// Inform the statusline that the terminal dimensions have changed.
3335
void TerminalSizeChanged();
@@ -46,7 +48,8 @@ class Statusline {
4648
void UpdateScrollWindow(ScrollWindowMode mode);
4749

4850
Debugger &m_debugger;
49-
std::string m_last_str;
51+
ExecutionContext m_exe_ctx;
52+
SymbolContext m_symbol_ctx;
5053
uint64_t m_terminal_width = 0;
5154
uint64_t m_terminal_height = 0;
5255
};

lldb/source/Core/Debugger.cpp

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,8 +1216,20 @@ void Debugger::RestoreInputTerminalState() {
12161216

12171217
void Debugger::RedrawStatusline(bool update) {
12181218
std::lock_guard<std::mutex> guard(m_statusline_mutex);
1219+
1220+
if (!update && m_statusline) {
1221+
m_statusline->Redraw(nullptr, nullptr);
1222+
return;
1223+
}
1224+
1225+
// Always compute the execution and symbol context, regardless of whether the
1226+
// statusline is enabled. This code gets called from the default event handler
1227+
// thread and has uncovered threading issues that otherwise only reproduce
1228+
// when the statusline is enabled.
1229+
SelectedContext ctx = GetSelectedContext();
12191230
if (m_statusline)
1220-
m_statusline->Redraw(update);
1231+
m_statusline->Redraw(&ctx.exe_ctx,
1232+
ctx.sym_ctx ? &ctx.sym_ctx.value() : nullptr);
12211233
}
12221234

12231235
ExecutionContext Debugger::GetSelectedExecutionContext() {
@@ -1226,6 +1238,27 @@ ExecutionContext Debugger::GetSelectedExecutionContext() {
12261238
return ExecutionContext(exe_ctx_ref);
12271239
}
12281240

1241+
Debugger::SelectedContext Debugger::GetSelectedContext() {
1242+
SelectedContext context;
1243+
context.exe_ctx = GetSelectedExecutionContext();
1244+
1245+
if (!context.exe_ctx.HasTargetScope())
1246+
context.exe_ctx.SetTargetPtr(&GetSelectedOrDummyTarget());
1247+
1248+
if (ProcessSP process_sp = context.exe_ctx.GetProcessSP()) {
1249+
// Check if the process is stopped, and if it is, make sure it remains
1250+
// stopped until we've computed the symbol context.
1251+
Process::StopLocker stop_locker;
1252+
if (stop_locker.TryLock(&process_sp->GetRunLock())) {
1253+
if (auto frame_sp = context.exe_ctx.GetFrameSP())
1254+
context.sym_ctx.emplace(
1255+
frame_sp->GetSymbolContext(eSymbolContextEverything));
1256+
}
1257+
}
1258+
1259+
return context;
1260+
}
1261+
12291262
void Debugger::DispatchInputInterrupt() {
12301263
std::lock_guard<std::recursive_mutex> guard(m_io_handler_stack.GetMutex());
12311264
IOHandlerSP reader_sp(m_io_handler_stack.Top());
@@ -2117,13 +2150,16 @@ lldb::thread_result_t Debugger::DefaultEventHandler() {
21172150
while (!done) {
21182151
EventSP event_sp;
21192152
if (listener_sp->GetEvent(event_sp, std::nullopt)) {
2153+
bool update_statusline = false;
21202154
if (event_sp) {
21212155
Broadcaster *broadcaster = event_sp->GetBroadcaster();
21222156
if (broadcaster) {
21232157
uint32_t event_type = event_sp->GetType();
21242158
ConstString broadcaster_class(broadcaster->GetBroadcasterClass());
21252159
if (broadcaster_class == broadcaster_class_process) {
21262160
HandleProcessEvent(event_sp);
2161+
update_statusline =
2162+
(event_type & Process::eBroadcastBitStateChanged) != 0;
21272163
} else if (broadcaster_class == broadcaster_class_target) {
21282164
if (Breakpoint::BreakpointEventData::GetEventDataFromEvent(
21292165
event_sp.get())) {
@@ -2168,7 +2204,7 @@ lldb::thread_result_t Debugger::DefaultEventHandler() {
21682204
if (m_forward_listener_sp)
21692205
m_forward_listener_sp->AddEvent(event_sp);
21702206
}
2171-
RedrawStatusline();
2207+
RedrawStatusline(update_statusline);
21722208
}
21732209
}
21742210

lldb/source/Core/Statusline.cpp

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -47,16 +47,17 @@ void Statusline::TerminalSizeChanged() {
4747

4848
UpdateScrollWindow(ResizeStatusline);
4949

50-
// Draw the old statusline.
51-
Redraw(/*update=*/false);
50+
// Redraw the old statusline.
51+
Redraw(nullptr, nullptr);
5252
}
5353

5454
void Statusline::Enable() {
5555
// Reduce the scroll window to make space for the status bar below.
5656
UpdateScrollWindow(EnableStatusline);
5757

5858
// Draw the statusline.
59-
Redraw(/*update=*/true);
59+
Debugger::SelectedContext ctx = m_debugger.GetSelectedContext();
60+
Redraw(&ctx.exe_ctx, ctx.sym_ctx ? &ctx.sym_ctx.value() : nullptr);
6061
}
6162

6263
void Statusline::Disable() {
@@ -69,8 +70,6 @@ void Statusline::Draw(std::string str) {
6970
if (!stream_sp)
7071
return;
7172

72-
m_last_str = str;
73-
7473
str = ansi::TrimAndPad(str, m_terminal_width);
7574

7675
LockedStreamFile locked_stream = stream_sp->Lock();
@@ -127,34 +126,18 @@ void Statusline::UpdateScrollWindow(ScrollWindowMode mode) {
127126
m_debugger.RefreshIOHandler();
128127
}
129128

130-
void Statusline::Redraw(bool update) {
131-
if (!update) {
132-
Draw(m_last_str);
133-
return;
134-
}
129+
void Statusline::Redraw(const ExecutionContext *exe_ctx,
130+
const SymbolContext *sym_ctx) {
131+
if (exe_ctx)
132+
m_exe_ctx = *exe_ctx;
135133

136-
ExecutionContext exe_ctx = m_debugger.GetSelectedExecutionContext();
137-
138-
// For colors and progress events, the format entity needs access to the
139-
// debugger, which requires a target in the execution context.
140-
if (!exe_ctx.HasTargetScope())
141-
exe_ctx.SetTargetPtr(&m_debugger.GetSelectedOrDummyTarget());
142-
143-
SymbolContext symbol_ctx;
144-
if (ProcessSP process_sp = exe_ctx.GetProcessSP()) {
145-
// Check if the process is stopped, and if it is, make sure it remains
146-
// stopped until we've computed the symbol context.
147-
Process::StopLocker stop_locker;
148-
if (stop_locker.TryLock(&process_sp->GetRunLock())) {
149-
if (auto frame_sp = exe_ctx.GetFrameSP())
150-
symbol_ctx = frame_sp->GetSymbolContext(eSymbolContextEverything);
151-
}
152-
}
134+
if (sym_ctx)
135+
m_symbol_ctx = *sym_ctx;
153136

154137
StreamString stream;
155138
FormatEntity::Entry format = m_debugger.GetStatuslineFormat();
156-
FormatEntity::Format(format, stream, &symbol_ctx, &exe_ctx, nullptr, nullptr,
157-
false, false);
139+
FormatEntity::Format(format, stream, &m_symbol_ctx, &m_exe_ctx, nullptr,
140+
nullptr, false, false);
158141

159142
Draw(stream.GetString().str());
160143
}

0 commit comments

Comments
 (0)