This revision was automatically updated to reflect the committed changes.
Closed by commit rGe19387e6936c: We can't let GetStackFrameCount get
interrupted or it will give the (authored by jingham).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150236/new/
https://reviews.llvm.org/D150236
Files:
lldb/include/lldb/Target/StackFrameList.h
lldb/include/lldb/lldb-private-enumerations.h
lldb/source/Commands/CommandCompletions.cpp
lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
lldb/source/Target/StackFrameList.cpp
lldb/test/API/functionalities/bt-interrupt/Makefile
lldb/test/API/functionalities/bt-interrupt/TestInterruptBacktrace.py
lldb/test/API/functionalities/bt-interrupt/main.c
Index: lldb/test/API/functionalities/bt-interrupt/main.c
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/bt-interrupt/main.c
@@ -0,0 +1,23 @@
+#include <stdio.h>
+
+// 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);
+}
Index: lldb/test/API/functionalities/bt-interrupt/TestInterruptBacktrace.py
===================================================================
--- /dev/null
+++ 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()
+
+
Index: lldb/test/API/functionalities/bt-interrupt/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/bt-interrupt/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules
Index: lldb/source/Target/StackFrameList.cpp
===================================================================
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -85,8 +85,8 @@
return;
std::lock_guard<std::recursive_mutex> 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<std::recursive_mutex> 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),
Index: lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
===================================================================
--- lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
+++ 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));
Index: lldb/source/Commands/CommandCompletions.cpp
===================================================================
--- lldb/source/Commands/CommandCompletions.cpp
+++ 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());
}
Index: lldb/include/lldb/lldb-private-enumerations.h
===================================================================
--- lldb/include/lldb/lldb-private-enumerations.h
+++ 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
Index: lldb/include/lldb/Target/StackFrameList.h
===================================================================
--- lldb/include/lldb/Target/StackFrameList.h
+++ 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);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
