diff --git a/lldb/source/Core/IOHandlerCursesGUI.cpp b/lldb/source/Core/IOHandlerCursesGUI.cpp --- a/lldb/source/Core/IOHandlerCursesGUI.cpp +++ b/lldb/source/Core/IOHandlerCursesGUI.cpp @@ -4614,30 +4614,48 @@ typedef std::shared_ptr TreeDelegateSP; -class TreeItem { +struct TreeItemData { + TreeItemData(TreeItem *parent, TreeDelegate &delegate, + bool might_have_children, bool is_expanded) + : m_parent(parent), m_delegate(&delegate), + m_might_have_children(might_have_children), m_is_expanded(is_expanded) { + } + +protected: + TreeItem *m_parent; + TreeDelegate *m_delegate; + void *m_user_data = nullptr; + uint64_t m_identifier = 0; + std::string m_text; + int m_row_idx = -1; // Zero based visible row index, -1 if not visible or for + // the root item + bool m_might_have_children; + bool m_is_expanded = false; +}; + +class TreeItem : public TreeItemData { public: TreeItem(TreeItem *parent, TreeDelegate &delegate, bool might_have_children) - : m_parent(parent), m_delegate(delegate), m_children(), - m_might_have_children(might_have_children) { - if (m_parent == nullptr) - m_is_expanded = m_delegate.TreeDelegateExpandRootByDefault(); - } + : TreeItemData(parent, delegate, might_have_children, + parent == nullptr + ? delegate.TreeDelegateExpandRootByDefault() + : false), + m_children() {} + + TreeItem(const TreeItem &) = delete; + TreeItem &operator=(const TreeItem &rhs) = delete; - TreeItem &operator=(const TreeItem &rhs) { + TreeItem &operator=(TreeItem &&rhs) { if (this != &rhs) { - m_parent = rhs.m_parent; - m_delegate = rhs.m_delegate; - m_user_data = rhs.m_user_data; - m_identifier = rhs.m_identifier; - m_row_idx = rhs.m_row_idx; - m_children = rhs.m_children; - m_might_have_children = rhs.m_might_have_children; - m_is_expanded = rhs.m_is_expanded; + TreeItemData::operator=(std::move(rhs)); + AdoptChildren(rhs.m_children); } return *this; } - TreeItem(const TreeItem &) = default; + TreeItem(TreeItem &&rhs) : TreeItemData(std::move(rhs)) { + AdoptChildren(rhs.m_children); + } size_t GetDepth() const { if (m_parent) @@ -4649,18 +4667,28 @@ void ClearChildren() { m_children.clear(); } - void Resize(size_t n, const TreeItem &t) { m_children.resize(n, t); } + void Resize(size_t n, TreeDelegate &delegate, bool might_have_children) { + if (m_children.size() >= n) { + m_children.erase(m_children.begin() + n, m_children.end()); + return; + } + m_children.reserve(n); + std::generate_n(std::back_inserter(m_children), n - m_children.size(), + [&, parent = this]() { + return TreeItem(parent, delegate, might_have_children); + }); + } TreeItem &operator[](size_t i) { return m_children[i]; } void SetRowIndex(int row_idx) { m_row_idx = row_idx; } size_t GetNumChildren() { - m_delegate.TreeDelegateGenerateChildren(*this); + m_delegate->TreeDelegateGenerateChildren(*this); return m_children.size(); } - void ItemWasSelected() { m_delegate.TreeDelegateItemSelected(*this); } + void ItemWasSelected() { m_delegate->TreeDelegateItemSelected(*this); } void CalculateRowIndexes(int &row_idx) { SetRowIndex(row_idx); @@ -4727,7 +4755,7 @@ if (highlight) window.AttributeOn(A_REVERSE); - m_delegate.TreeDelegateDrawTreeItem(*this, window); + m_delegate->TreeDelegateDrawTreeItem(*this, window); if (highlight) window.AttributeOff(A_REVERSE); @@ -4811,16 +4839,13 @@ void SetMightHaveChildren(bool b) { m_might_have_children = b; } protected: - TreeItem *m_parent; - TreeDelegate &m_delegate; - void *m_user_data = nullptr; - uint64_t m_identifier = 0; - std::string m_text; - int m_row_idx = -1; // Zero based visible row index, -1 if not visible or for - // the root item + void AdoptChildren(std::vector &children) { + m_children = std::move(children); + for (auto &child : m_children) + child.m_parent = this; + } + std::vector m_children; - bool m_might_have_children; - bool m_is_expanded = false; }; class TreeWindowDelegate : public WindowDelegate { @@ -5117,9 +5142,8 @@ m_stop_id = process_sp->GetStopID(); m_tid = thread_sp->GetID(); - TreeItem t(&item, *m_frame_delegate_sp, false); size_t num_frames = thread_sp->GetStackFrameCount(); - item.Resize(num_frames, t); + item.Resize(num_frames, *m_frame_delegate_sp, false); for (size_t i = 0; i < num_frames; ++i) { item[i].SetUserData(thread_sp.get()); item[i].SetIdentifier(i); @@ -5219,12 +5243,11 @@ std::make_shared(m_debugger); } - TreeItem t(&item, *m_thread_delegate_sp, false); ThreadList &threads = process_sp->GetThreadList(); std::lock_guard guard(threads.GetMutex()); ThreadSP selected_thread = threads.GetSelectedThread(); size_t num_threads = threads.GetSize(); - item.Resize(num_threads, t); + item.Resize(num_threads, *m_thread_delegate_sp, false); for (size_t i = 0; i < num_threads; ++i) { ThreadSP thread = threads.GetThreadAtIndex(i); item[i].SetIdentifier(thread->GetID()); @@ -5404,9 +5427,8 @@ if (!m_string_delegate_sp) m_string_delegate_sp = std::make_shared(); - TreeItem details_tree_item(&item, *m_string_delegate_sp, false); - item.Resize(details.GetSize(), details_tree_item); + item.Resize(details.GetSize(), *m_string_delegate_sp, false); for (size_t i = 0; i < details.GetSize(); i++) { item[i].SetText(details.GetStringAtIndex(i)); } @@ -5448,10 +5470,9 @@ if (!m_breakpoint_location_delegate_sp) m_breakpoint_location_delegate_sp = std::make_shared(m_debugger); - TreeItem breakpoint_location_tree_item( - &item, *m_breakpoint_location_delegate_sp, true); - item.Resize(breakpoint->GetNumLocations(), breakpoint_location_tree_item); + item.Resize(breakpoint->GetNumLocations(), + *m_breakpoint_location_delegate_sp, true); for (size_t i = 0; i < breakpoint->GetNumLocations(); i++) { item[i].SetIdentifier(i); item[i].SetUserData(breakpoint.get()); @@ -5495,9 +5516,8 @@ if (!m_breakpoint_delegate_sp) m_breakpoint_delegate_sp = std::make_shared(m_debugger); - TreeItem breakpoint_tree_item(&item, *m_breakpoint_delegate_sp, true); - item.Resize(breakpoints.GetSize(), breakpoint_tree_item); + item.Resize(breakpoints.GetSize(), *m_breakpoint_delegate_sp, true); for (size_t i = 0; i < breakpoints.GetSize(); i++) { item[i].SetIdentifier(i); } diff --git a/lldb/test/API/commands/gui/spawn-threads/Makefile b/lldb/test/API/commands/gui/spawn-threads/Makefile new file mode 100644 --- /dev/null +++ b/lldb/test/API/commands/gui/spawn-threads/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp +ENABLE_THREADS := YES +include Makefile.rules diff --git a/lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py b/lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py new file mode 100644 --- /dev/null +++ b/lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py @@ -0,0 +1,47 @@ +""" +Test that 'gui' does not crash when adding new threads, which +populate TreeItem's children and may be reallocated elsewhere. +""" + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test.lldbpexpect import PExpectTest + +import sys + +class TestGuiSpawnThreadsTest(PExpectTest): + # PExpect uses many timeouts internally and doesn't play well + # under ASAN on a loaded machine.. + @skipIfAsan + @skipIfCursesSupportMissing + def test_gui(self): + self.build() + + self.launch(executable=self.getBuildArtifact('a.out'), dimensions=(100, 500)) + self.expect( + 'breakpoint set -f main.cpp -p "break here"', substrs=['Breakpoint 1', 'address ='] + ) + self.expect( + 'breakpoint set -f main.cpp -p "before join"', substrs=['Breakpoint 2', 'address ='] + ) + self.expect("run", substrs=["stop reason ="]) + + escape_key = chr(27).encode() + + # Start the GUI + self.child.sendline("gui") + self.child.expect_exact("Threads") + self.child.expect_exact(f"thread #1: tid =") + + for i in range(5): + # Stopped at the breakpoint, continue over the thread creation + self.child.send("c") + # Check the newly created thread + self.child.expect_exact(f"thread #{i + 2}: tid =") + + # Exit GUI. + self.child.send(escape_key) + self.expect_prompt() + + self.quit() diff --git a/lldb/test/API/commands/gui/spawn-threads/main.cpp b/lldb/test/API/commands/gui/spawn-threads/main.cpp new file mode 100644 --- /dev/null +++ b/lldb/test/API/commands/gui/spawn-threads/main.cpp @@ -0,0 +1,25 @@ +#include +#include +#include + +#include "pseudo_barrier.h" + +pseudo_barrier_t barrier_inside; + +void thread_func() { pseudo_barrier_wait(barrier_inside); } + +void test_thread() { + std::vector thrs; + for (int i = 0; i < 5; i++) + thrs.push_back(std::thread(thread_func)); // break here + + pseudo_barrier_wait(barrier_inside); // break before join + for (auto &t : thrs) + t.join(); +} + +int main() { + pseudo_barrier_init(barrier_inside, 6); + test_thread(); + return 0; +}