diff --git a/lldb/include/lldb/Target/StackFrameList.h b/lldb/include/lldb/Target/StackFrameList.h --- a/lldb/include/lldb/Target/StackFrameList.h +++ b/lldb/include/lldb/Target/StackFrameList.h @@ -100,7 +100,11 @@ bool SetFrameAtIndex(uint32_t idx, lldb::StackFrameSP &frame_sp); - void GetFramesUpTo(uint32_t end_idx); + /// Realizes frames up to (and including) end_idx (which can be greater than + /// the actual number of frames.) + /// Returns true if the function was interrupted, false otherwise. + bool GetFramesUpTo(uint32_t end_idx, + InterruptionControl allow_interrupt = AllowInterruption); void GetOnlyConcreteFramesUpTo(uint32_t end_idx, Unwind &unwinder); diff --git a/lldb/include/lldb/lldb-private-enumerations.h b/lldb/include/lldb/lldb-private-enumerations.h --- a/lldb/include/lldb/lldb-private-enumerations.h +++ b/lldb/include/lldb/lldb-private-enumerations.h @@ -274,4 +274,9 @@ DoNoSelectMostRelevantFrame = false, }; +enum InterruptionControl : bool { + AllowInterruption = true, + DoNotAllowInterruption = false, +}; + #endif // LLDB_LLDB_PRIVATE_ENUMERATIONS_H diff --git a/lldb/source/Commands/CommandCompletions.cpp b/lldb/source/Commands/CommandCompletions.cpp --- a/lldb/source/Commands/CommandCompletions.cpp +++ b/lldb/source/Commands/CommandCompletions.cpp @@ -718,10 +718,14 @@ return; lldb::ThreadSP thread_sp = exe_ctx.GetThreadSP(); + Debugger &dbg = interpreter.GetDebugger(); const uint32_t frame_num = thread_sp->GetStackFrameCount(); for (uint32_t i = 0; i < frame_num; ++i) { lldb::StackFrameSP frame_sp = thread_sp->GetStackFrameAtIndex(i); StreamString strm; + // Dumping frames can be slow, allow interruption. + if (dbg.InterruptRequested()) + break; frame_sp->Dump(&strm, false, true); request.TryCompleteCurrentArg(std::to_string(i), strm.GetString()); } diff --git a/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp b/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp --- a/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp +++ b/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp @@ -220,8 +220,7 @@ } bool SystemRuntimeMacOSX::SafeToCallFunctionsOnThisThread(ThreadSP thread_sp) { - if (thread_sp && thread_sp->GetStackFrameCount() > 0 && - thread_sp->GetFrameWithConcreteFrameIndex(0)) { + if (thread_sp && thread_sp->GetFrameWithConcreteFrameIndex(0)) { const SymbolContext sym_ctx( thread_sp->GetFrameWithConcreteFrameIndex(0)->GetSymbolContext( eSymbolContextSymbol)); diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp --- a/lldb/source/Target/StackFrameList.cpp +++ b/lldb/source/Target/StackFrameList.cpp @@ -85,8 +85,8 @@ return; std::lock_guard guard(m_mutex); - - GetFramesUpTo(0); + + GetFramesUpTo(0, DoNotAllowInterruption); if (m_frames.empty()) return; if (!m_frames[0]->IsInlined()) { @@ -436,21 +436,23 @@ next_frame.SetFrameIndex(m_frames.size()); } -void StackFrameList::GetFramesUpTo(uint32_t end_idx) { +bool StackFrameList::GetFramesUpTo(uint32_t end_idx, + InterruptionControl allow_interrupt) { // Do not fetch frames for an invalid thread. + bool was_interrupted = false; if (!m_thread.IsValid()) - return; + return false; // We've already gotten more frames than asked for, or we've already finished // unwinding, return. if (m_frames.size() > end_idx || GetAllFramesFetched()) - return; + return false; Unwind &unwinder = m_thread.GetUnwinder(); if (!m_show_inlined_frames) { GetOnlyConcreteFramesUpTo(end_idx, unwinder); - return; + return false; } #if defined(DEBUG_STACK_FRAMES) @@ -474,13 +476,6 @@ StackFrameSP unwind_frame_sp; Debugger &dbg = m_thread.GetProcess()->GetTarget().GetDebugger(); do { - // Check for interruption here when building the frames - this is the - // expensive part, Dump later on is cheap. - if (dbg.InterruptRequested()) { - Log *log = GetLog(LLDBLog::Host); - LLDB_LOG(log, "Interrupted %s", __FUNCTION__); - break; - } uint32_t idx = m_concrete_frames_fetched++; lldb::addr_t pc = LLDB_INVALID_ADDRESS; lldb::addr_t cfa = LLDB_INVALID_ADDRESS; @@ -512,6 +507,15 @@ cfa = unwind_frame_sp->m_id.GetCallFrameAddress(); } } else { + // Check for interruption when building the frames. + // Do the check in idx > 0 so that we'll always create a 0th frame. + if (allow_interrupt && dbg.InterruptRequested()) { + Log *log = GetLog(LLDBLog::Host); + LLDB_LOG(log, "Interrupted %s", __FUNCTION__); + was_interrupted = true; + break; + } + const bool success = unwinder.GetFrameInfoAtIndex(idx, cfa, pc, behaves_like_zeroth_frame); if (!success) { @@ -624,14 +628,19 @@ Dump(&s); s.EOL(); #endif + // Don't report interrupted if we happen to have gotten all the frames: + if (!GetAllFramesFetched()) + return was_interrupted; + return false; } uint32_t StackFrameList::GetNumFrames(bool can_create) { std::lock_guard guard(m_mutex); - if (can_create) - GetFramesUpTo(UINT32_MAX); - + if (can_create) { + // Don't allow interrupt or we might not return the correct count + GetFramesUpTo(UINT32_MAX, DoNotAllowInterruption); + } return GetVisibleStackFrameIndex(m_frames.size()); } @@ -672,7 +681,13 @@ // GetFramesUpTo will fill m_frames with as many frames as you asked for, if // there are that many. If there weren't then you asked for too many frames. - GetFramesUpTo(idx); + // GetFramesUpTo returns true if interrupted: + if (GetFramesUpTo(idx)) { + Log *log = GetLog(LLDBLog::Thread); + LLDB_LOG(log, "GetFrameAtIndex was interrupted"); + return {}; + } + if (idx < m_frames.size()) { if (m_show_inlined_frames) { // When inline frames are enabled we actually create all the frames in @@ -947,6 +962,14 @@ else marker = unselected_marker; } + // Check for interruption here. If we're fetching arguments, this loop + // can go slowly: + Debugger &dbg = m_thread.GetProcess()->GetTarget().GetDebugger(); + if (dbg.InterruptRequested()) { + Log *log = GetLog(LLDBLog::Host); + LLDB_LOG(log, "Interrupted %s", __FUNCTION__); + break; + } if (!frame_sp->GetStatus(strm, show_frame_info, num_frames_with_source > (first_frame - frame_idx), diff --git a/lldb/test/API/functionalities/bt-interrupt/Makefile b/lldb/test/API/functionalities/bt-interrupt/Makefile new file mode 100644 --- /dev/null +++ b/lldb/test/API/functionalities/bt-interrupt/Makefile @@ -0,0 +1,4 @@ +C_SOURCES := main.c +CFLAGS_EXTRAS := -std=c99 + +include Makefile.rules diff --git a/lldb/test/API/functionalities/bt-interrupt/TestInterruptBacktrace.py b/lldb/test/API/functionalities/bt-interrupt/TestInterruptBacktrace.py new file mode 100644 --- /dev/null +++ b/lldb/test/API/functionalities/bt-interrupt/TestInterruptBacktrace.py @@ -0,0 +1,49 @@ +""" +Ensure that when the interrupt is raised we still make frame 0. +and make sure "GetNumFrames" isn't interrupted. +""" + +import lldb +import lldbsuite.test.lldbutil as lldbutil +from lldbsuite.test.lldbtest import * + + +class TestInterruptingBacktrace(TestBase): + + NO_DEBUG_INFO_TESTCASE = True + + def test_backtrace_interrupt(self): + """Use RequestInterrupt followed by stack operations + to ensure correct interrupt behavior for stacks.""" + self.build() + self.main_source_file = lldb.SBFileSpec("main.c") + self.bt_interrupt_test() + + def bt_interrupt_test(self): + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, + "Set a breakpoint here", self.main_source_file) + + # Now continue, and when we stop we will have crashed. + process.Continue() + self.dbg.RequestInterrupt() + + # Be sure to turn this off again: + def cleanup (): + if self.dbg.InterruptRequested(): + self.dbg.CancelInterruptRequest() + self.addTearDownHook(cleanup) + + frame_0 = thread.GetFrameAtIndex(0) + self.assertTrue(frame_0.IsValid(), "Got a good 0th frame") + # The interrupt flag is up already, so any attempt to backtrace + # should be cut short: + frame_1 = thread.GetFrameAtIndex(1) + self.assertFalse(frame_1.IsValid(), "Prevented from getting more frames") + # Since GetNumFrames is a contract, we don't interrupt it: + num_frames = thread.GetNumFrames() + print(f"Number of frames: {num_frames}") + self.assertGreater(num_frames, 1, "Got many frames") + + self.dbg.CancelInterruptRequest() + + diff --git a/lldb/test/API/functionalities/bt-interrupt/main.c b/lldb/test/API/functionalities/bt-interrupt/main.c new file mode 100644 --- /dev/null +++ b/lldb/test/API/functionalities/bt-interrupt/main.c @@ -0,0 +1,23 @@ +#include + +// This example is meant to recurse infinitely. +// The extra struct is just to make the frame dump +// more complicated. + +struct Foo { + int a; + int b; + char *c; +}; + +int +forgot_termination(int input, struct Foo my_foo) { + return forgot_termination(++input, my_foo); +} + +int +main() +{ + struct Foo myFoo = {100, 300, "A string you will print a lot"}; // Set a breakpoint here + return forgot_termination(1, myFoo); +}