Skip to content

Commit 3d0be01

Browse files
jiminghamjimingham
committed
Refine the reporting mechanism for interruption.
Also, make it possible for new Targets which haven't been added to the TargetList yet to check for interruption, and add a few more places in building modules where we can check for interruption. Differential Revision: https://reviews.llvm.org/D154542 (cherry picked from commit 2b0c886)
1 parent 167febc commit 3d0be01

File tree

13 files changed

+259
-48
lines changed

13 files changed

+259
-48
lines changed

‎lldb/include/lldb/Core/Debugger.h

Lines changed: 70 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include "llvm/ADT/StringMap.h"
4242
#include "llvm/ADT/StringRef.h"
4343
#include "llvm/Support/DynamicLibrary.h"
44+
#include "llvm/Support/FormatVariadic.h"
4445
#include "llvm/Support/Threading.h"
4546

4647
#include <cassert>
@@ -85,6 +86,8 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
8586
eBroadcastSymbolChange = (1 << 3),
8687
};
8788

89+
using DebuggerList = std::vector<lldb::DebuggerSP>;
90+
8891
static ConstString GetStaticBroadcasterClass();
8992

9093
/// Get the public broadcaster for this debugger.
@@ -373,6 +376,11 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
373376
bool IsHandlingEvents() const { return m_event_handler_thread.IsJoinable(); }
374377

375378
Status RunREPL(lldb::LanguageType language, const char *repl_options);
379+
380+
bool REPLIsActive() { return m_io_handler_stack.REPLIsActive(); }
381+
382+
bool REPLIsEnabled() { return m_io_handler_stack.REPLIsEnabled(); }
383+
376384

377385
/// Interruption in LLDB:
378386
///
@@ -410,16 +418,75 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
410418
/// If you are on the RunCommandInterpreter thread, it will check the
411419
/// command interpreter state, and if it is on another thread it will
412420
/// check the debugger Interrupt Request state.
421+
/// \param[in] cur_func
422+
/// For reporting if the interruption was requested. Don't provide this by
423+
/// hand, use INTERRUPT_REQUESTED so this gets done consistently.
413424
///
425+
/// \param[in] formatv
426+
/// A formatv string for the interrupt message. If the elements of the
427+
/// message are expensive to compute, you can use the no-argument form of
428+
/// InterruptRequested, then make up the report using REPORT_INTERRUPTION.
429+
///
414430
/// \return
415431
/// A boolean value, if \b true an interruptible operation should interrupt
416432
/// itself.
417-
bool InterruptRequested();
433+
template <typename... Args>
434+
bool InterruptRequested(const char *cur_func,
435+
const char *formatv, Args &&... args) {
436+
bool ret_val = InterruptRequested();
437+
if (ret_val) {
438+
if (!formatv)
439+
formatv = "Unknown message";
440+
if (!cur_func)
441+
cur_func = "<UNKNOWN>";
442+
ReportInterruption(InterruptionReport(cur_func,
443+
llvm::formatv(formatv,
444+
std::forward<Args>(args)...)));
445+
}
446+
return ret_val;
447+
}
448+
449+
450+
/// This handy define will keep you from having to generate a report for the
451+
/// interruption by hand. Use this except in the case where the arguments to
452+
/// the message description are expensive to compute.
453+
#define INTERRUPT_REQUESTED(debugger, ...) \
454+
(debugger).InterruptRequested(__func__, __VA_ARGS__)
418455

419-
bool REPLIsActive() { return m_io_handler_stack.REPLIsActive(); }
456+
// This form just queries for whether to interrupt, and does no reporting:
457+
bool InterruptRequested();
458+
459+
// FIXME: Do we want to capture a backtrace at the interruption point?
460+
class InterruptionReport {
461+
public:
462+
InterruptionReport(std::string function_name, std::string description) :
463+
m_function_name(std::move(function_name)),
464+
m_description(std::move(description)),
465+
m_interrupt_time(std::chrono::system_clock::now()),
466+
m_thread_id(llvm::get_threadid()) {}
467+
468+
InterruptionReport(std::string function_name,
469+
const llvm::formatv_object_base &payload);
470+
471+
template <typename... Args>
472+
InterruptionReport(std::string function_name,
473+
const char *format, Args &&... args) :
474+
InterruptionReport(function_name, llvm::formatv(format, std::forward<Args>(args)...)) {}
475+
476+
std::string m_function_name;
477+
std::string m_description;
478+
const std::chrono::time_point<std::chrono::system_clock> m_interrupt_time;
479+
const uint64_t m_thread_id;
480+
};
481+
void ReportInterruption(const InterruptionReport &report);
482+
#define REPORT_INTERRUPTION(debugger, ...) \
483+
(debugger).ReportInterruption(Debugger::InterruptionReport(__func__, \
484+
__VA_ARGS__))
420485

421-
bool REPLIsEnabled() { return m_io_handler_stack.REPLIsEnabled(); }
486+
static DebuggerList DebuggersRequestingInterruption();
422487

488+
public:
489+
423490
// This is for use in the command interpreter, when you either want the
424491
// selected target, or if no target is present you want to prime the dummy
425492
// target with entities that will be copied over to new targets.

‎lldb/include/lldb/Target/TargetList.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,13 +184,20 @@ class TargetList : public Broadcaster {
184184
void SetSelectedTarget(const lldb::TargetSP &target);
185185

186186
lldb::TargetSP GetSelectedTarget();
187+
188+
/// Returns whether any module, including ones in the process of being
189+
/// added, contains this module. I don't want to give direct access to
190+
/// these not yet added target, but for interruption purposes, we might
191+
/// need to ask whether this target contains this module.
192+
bool AnyTargetContainsModule(Module &module);
187193

188194
TargetIterable Targets() {
189195
return TargetIterable(m_target_list, m_target_list_mutex);
190196
}
191197

192198
private:
193199
collection m_target_list;
200+
std::unordered_set<lldb::TargetSP> m_in_process_target_list;
194201
mutable std::recursive_mutex m_target_list_mutex;
195202
uint32_t m_selected_target_idx;
196203

@@ -206,6 +213,12 @@ class TargetList : public Broadcaster {
206213
lldb::PlatformSP &platform_sp,
207214
lldb::TargetSP &target_sp);
208215

216+
void RegisterInProcessTarget(lldb::TargetSP target_sp);
217+
218+
void UnregisterInProcessTarget(lldb::TargetSP target_sp);
219+
220+
bool IsTargetInProcess(lldb::TargetSP target_sp);
221+
209222
void AddTargetInternal(lldb::TargetSP target_sp, bool do_select);
210223

211224
void SetSelectedTargetInternal(uint32_t index);

‎lldb/source/API/SBFrame.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -826,12 +826,13 @@ SBValueList SBFrame::GetVariables(const lldb::SBVariablesOptions &options) {
826826
if (variable_list) {
827827
const size_t num_variables = variable_list->GetSize();
828828
if (num_variables) {
829+
size_t num_produced = 0;
829830
for (const VariableSP &variable_sp : *variable_list) {
830-
if (dbg.InterruptRequested()) {
831-
Log *log = GetLog(LLDBLog::Host);
832-
LLDB_LOG(log, "Interrupted SBFrame::GetVariables");
831+
if (INTERRUPT_REQUESTED(dbg,
832+
"Interrupted getting frame variables with {0} of {1} "
833+
"produced.", num_produced, num_variables))
833834
return {};
834-
}
835+
835836
if (variable_sp) {
836837
bool add_variable = false;
837838
switch (variable_sp->GetScope()) {
@@ -875,6 +876,7 @@ SBValueList SBFrame::GetVariables(const lldb::SBVariablesOptions &options) {
875876
}
876877
}
877878
}
879+
num_produced++;
878880
}
879881
}
880882
if (recognized_arguments) {

‎lldb/source/Commands/CommandCompletions.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -735,7 +735,7 @@ void CommandCompletions::FrameIndexes(CommandInterpreter &interpreter,
735735
lldb::StackFrameSP frame_sp = thread_sp->GetStackFrameAtIndex(i);
736736
StreamString strm;
737737
// Dumping frames can be slow, allow interruption.
738-
if (dbg.InterruptRequested())
738+
if (INTERRUPT_REQUESTED(dbg, "Interrupted in frame completion"))
739739
break;
740740
frame_sp->Dump(&strm, false, true);
741741
request.TryCompleteCurrentArg(std::to_string(i), strm.GetString());

‎lldb/source/Commands/CommandObjectFrame.cpp

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -325,21 +325,29 @@ class CommandObjectFrameSelect : public CommandObjectParsed {
325325
}
326326
} else if (*m_options.relative_frame_offset > 0) {
327327
// I don't want "up 20" where "20" takes you past the top of the stack
328-
// to produce
329-
// an error, but rather to just go to the top. So I have to count the
330-
// stack here...
331-
const uint32_t num_frames = thread->GetStackFrameCount();
332-
if (static_cast<int32_t>(num_frames - frame_idx) >
333-
*m_options.relative_frame_offset)
334-
frame_idx += *m_options.relative_frame_offset;
328+
// to produce an error, but rather to just go to the top. OTOH, start
329+
// by seeing if the requested frame exists, in which case we can avoid
330+
// counting the stack here...
331+
const uint32_t frame_requested = frame_idx
332+
+ *m_options.relative_frame_offset;
333+
StackFrameSP frame_sp = thread->GetStackFrameAtIndex(frame_requested);
334+
if (frame_sp)
335+
frame_idx = frame_requested;
335336
else {
336-
if (frame_idx == num_frames - 1) {
337-
// If we are already at the top of the stack, just warn and don't
338-
// reset the frame.
339-
result.AppendError("Already at the top of the stack.");
340-
return false;
341-
} else
342-
frame_idx = num_frames - 1;
337+
// The request went past the stack, so handle that case:
338+
const uint32_t num_frames = thread->GetStackFrameCount();
339+
if (static_cast<int32_t>(num_frames - frame_idx) >
340+
*m_options.relative_frame_offset)
341+
frame_idx += *m_options.relative_frame_offset;
342+
else {
343+
if (frame_idx == num_frames - 1) {
344+
// If we are already at the top of the stack, just warn and don't
345+
// reset the frame.
346+
result.AppendError("Already at the top of the stack.");
347+
return false;
348+
} else
349+
frame_idx = num_frames - 1;
350+
}
343351
}
344352
}
345353
} else {

‎lldb/source/Commands/CommandObjectTarget.cpp

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1999,8 +1999,11 @@ class CommandObjectTargetModulesDumpSymtab
19991999
result.GetOutputStream().EOL();
20002000
result.GetOutputStream().EOL();
20012001
}
2002-
if (GetDebugger().InterruptRequested())
2002+
if (INTERRUPT_REQUESTED(GetDebugger(),
2003+
"Interrupted in dump all symtabs with {0} "
2004+
"of {1} dumped.", num_dumped, num_modules))
20032005
break;
2006+
20042007
num_dumped++;
20052008
DumpModuleSymtab(m_interpreter, result.GetOutputStream(),
20062009
module_sp.get(), m_options.m_sort_order,
@@ -2026,8 +2029,11 @@ class CommandObjectTargetModulesDumpSymtab
20262029
result.GetOutputStream().EOL();
20272030
result.GetOutputStream().EOL();
20282031
}
2029-
if (GetDebugger().InterruptRequested())
2032+
if (INTERRUPT_REQUESTED(GetDebugger(),
2033+
"Interrupted in dump symtab list with {0} of {1} dumped.",
2034+
num_dumped, num_matches))
20302035
break;
2036+
20312037
num_dumped++;
20322038
DumpModuleSymtab(m_interpreter, result.GetOutputStream(),
20332039
module_sp.get(), m_options.m_sort_order,
@@ -2087,8 +2093,11 @@ class CommandObjectTargetModulesDumpSections
20872093
result.GetOutputStream().Format("Dumping sections for {0} modules.\n",
20882094
num_modules);
20892095
for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
2090-
if (GetDebugger().InterruptRequested())
2096+
if (INTERRUPT_REQUESTED(GetDebugger(),
2097+
"Interrupted in dump all sections with {0} of {1} dumped",
2098+
image_idx, num_modules))
20912099
break;
2100+
20922101
num_dumped++;
20932102
DumpModuleSections(
20942103
m_interpreter, result.GetOutputStream(),
@@ -2105,8 +2114,11 @@ class CommandObjectTargetModulesDumpSections
21052114
FindModulesByName(target, arg_cstr, module_list, true);
21062115
if (num_matches > 0) {
21072116
for (size_t i = 0; i < num_matches; ++i) {
2108-
if (GetDebugger().InterruptRequested())
2117+
if (INTERRUPT_REQUESTED(GetDebugger(),
2118+
"Interrupted in dump section list with {0} of {1} dumped.",
2119+
i, num_matches))
21092120
break;
2121+
21102122
Module *module = module_list.GetModulePointerAtIndex(i);
21112123
if (module) {
21122124
num_dumped++;
@@ -2219,7 +2231,7 @@ class CommandObjectTargetModulesDumpClangAST
22192231
result.GetOutputStream().Format("Dumping clang ast for {0} modules.\n",
22202232
num_modules);
22212233
for (ModuleSP module_sp : module_list.ModulesNoLocking()) {
2222-
if (GetDebugger().InterruptRequested())
2234+
if (INTERRUPT_REQUESTED(GetDebugger(), "Interrupted dumping clang ast"))
22232235
break;
22242236
if (SymbolFile *sf = module_sp->GetSymbolFile())
22252237
sf->DumpClangAST(result.GetOutputStream());
@@ -2244,8 +2256,11 @@ class CommandObjectTargetModulesDumpClangAST
22442256
}
22452257

22462258
for (size_t i = 0; i < num_matches; ++i) {
2247-
if (GetDebugger().InterruptRequested())
2259+
if (INTERRUPT_REQUESTED(GetDebugger(),
2260+
"Interrupted in dump clang ast list with {0} of {1} dumped.",
2261+
i, num_matches))
22482262
break;
2263+
22492264
Module *m = module_list.GetModulePointerAtIndex(i);
22502265
if (SymbolFile *sf = m->GetSymbolFile())
22512266
sf->DumpClangAST(result.GetOutputStream());
@@ -2293,8 +2308,11 @@ class CommandObjectTargetModulesDumpSymfile
22932308
result.GetOutputStream().Format(
22942309
"Dumping debug symbols for {0} modules.\n", num_modules);
22952310
for (ModuleSP module_sp : target_modules.ModulesNoLocking()) {
2296-
if (GetDebugger().InterruptRequested())
2311+
if (INTERRUPT_REQUESTED(GetDebugger(), "Interrupted in dumping all "
2312+
"debug symbols with {0} of {1} modules dumped",
2313+
num_dumped, num_modules))
22972314
break;
2315+
22982316
if (DumpModuleSymbolFile(result.GetOutputStream(), module_sp.get()))
22992317
num_dumped++;
23002318
}
@@ -2309,7 +2327,9 @@ class CommandObjectTargetModulesDumpSymfile
23092327
FindModulesByName(target, arg_cstr, module_list, true);
23102328
if (num_matches > 0) {
23112329
for (size_t i = 0; i < num_matches; ++i) {
2312-
if (GetDebugger().InterruptRequested())
2330+
if (INTERRUPT_REQUESTED(GetDebugger(), "Interrupted dumping {0} "
2331+
"of {1} requested modules",
2332+
i, num_matches))
23132333
break;
23142334
Module *module = module_list.GetModulePointerAtIndex(i);
23152335
if (module) {
@@ -2373,11 +2393,16 @@ class CommandObjectTargetModulesDumpLineTable
23732393

23742394
const ModuleList &target_modules = target->GetImages();
23752395
std::lock_guard<std::recursive_mutex> guard(target_modules.GetMutex());
2376-
if (target_modules.GetSize() > 0) {
2396+
size_t num_modules = target_modules.GetSize();
2397+
if (num_modules > 0) {
23772398
uint32_t num_dumped = 0;
23782399
for (ModuleSP module_sp : target_modules.ModulesNoLocking()) {
2379-
if (GetDebugger().InterruptRequested())
2400+
if (INTERRUPT_REQUESTED(GetDebugger(),
2401+
"Interrupted in dump all line tables with "
2402+
"{0} of {1} dumped", num_dumped,
2403+
num_modules))
23802404
break;
2405+
23812406
if (DumpCompileUnitLineTable(
23822407
m_interpreter, result.GetOutputStream(), module_sp.get(),
23832408
file_spec,

‎lldb/source/Commands/CommandObjectThread.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,11 @@ class CommandObjectThreadBacktrace : public CommandObjectIterateOverThreads {
227227
thread->GetIndexID());
228228
return false;
229229
}
230-
if (m_options.m_extended_backtrace && !GetDebugger().InterruptRequested()) {
231-
DoExtendedBacktrace(thread, result);
230+
if (m_options.m_extended_backtrace) {
231+
if (!INTERRUPT_REQUESTED(GetDebugger(),
232+
"Interrupt skipped extended backtrace")) {
233+
DoExtendedBacktrace(thread, result);
234+
}
232235
}
233236

234237
return true;

‎lldb/source/Core/Debugger.cpp

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,9 @@ static size_t g_debugger_event_thread_stack_bytes = 8 * 1024 * 1024;
9999

100100
#pragma mark Static Functions
101101

102-
typedef std::vector<DebuggerSP> DebuggerList;
103102
static std::recursive_mutex *g_debugger_list_mutex_ptr =
104103
nullptr; // NOTE: intentional leak to avoid issues with C++ destructor chain
105-
static DebuggerList *g_debugger_list_ptr =
104+
static Debugger::DebuggerList *g_debugger_list_ptr =
106105
nullptr; // NOTE: intentional leak to avoid issues with C++ destructor chain
107106
static llvm::ThreadPool *g_thread_pool = nullptr;
108107

@@ -1259,6 +1258,33 @@ bool Debugger::InterruptRequested() {
12591258
return GetCommandInterpreter().WasInterrupted();
12601259
}
12611260

1261+
Debugger::InterruptionReport::InterruptionReport(std::string function_name,
1262+
const llvm::formatv_object_base &payload) :
1263+
m_function_name(std::move(function_name)),
1264+
m_interrupt_time(std::chrono::system_clock::now()),
1265+
m_thread_id(llvm::get_threadid()) {
1266+
llvm::raw_string_ostream desc(m_description);
1267+
desc << payload << "\n";
1268+
}
1269+
1270+
void Debugger::ReportInterruption(const InterruptionReport &report) {
1271+
// For now, just log the description:
1272+
Log *log = GetLog(LLDBLog::Host);
1273+
LLDB_LOG(log, "Interruption: {0}", report.m_description);
1274+
}
1275+
1276+
Debugger::DebuggerList Debugger::DebuggersRequestingInterruption() {
1277+
DebuggerList result;
1278+
if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
1279+
std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr);
1280+
for (auto debugger_sp : *g_debugger_list_ptr) {
1281+
if (debugger_sp->InterruptRequested())
1282+
result.push_back(debugger_sp);
1283+
}
1284+
}
1285+
return result;
1286+
}
1287+
12621288
size_t Debugger::GetNumDebuggers() {
12631289
if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
12641290
std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr);

0 commit comments

Comments
 (0)