This revision was automatically updated to reflect the committed changes.
Closed by commit rG83695d45d621: [lldb][gui] Update TreeItem children's
m_parent on move (authored by kpdev42).
Changed prior to commit:
https://reviews.llvm.org/D157960?vs=550240&id=550655#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157960/new/
https://reviews.llvm.org/D157960
Files:
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/test/API/commands/gui/spawn-threads/Makefile
lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py
lldb/test/API/commands/gui/spawn-threads/main.cpp
Index: lldb/test/API/commands/gui/spawn-threads/main.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/commands/gui/spawn-threads/main.cpp
@@ -0,0 +1,25 @@
+#include <iostream>
+#include <thread>
+#include <vector>
+
+#include "pseudo_barrier.h"
+
+pseudo_barrier_t barrier_inside;
+
+void thread_func() { pseudo_barrier_wait(barrier_inside); }
+
+void test_thread() {
+ std::vector<std::thread> 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;
+}
Index: lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py
===================================================================
--- /dev/null
+++ 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()
Index: lldb/test/API/commands/gui/spawn-threads/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/commands/gui/spawn-threads/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+ENABLE_THREADS := YES
+include Makefile.rules
Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===================================================================
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -4614,30 +4614,48 @@
typedef std::shared_ptr<TreeDelegate> 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<TreeItem> &children) {
+ m_children = std::move(children);
+ for (auto &child : m_children)
+ child.m_parent = this;
+ }
+
std::vector<TreeItem> 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<ThreadTreeDelegate>(m_debugger);
}
- TreeItem t(&item, *m_thread_delegate_sp, false);
ThreadList &threads = process_sp->GetThreadList();
std::lock_guard<std::recursive_mutex> 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<TextTreeDelegate>();
- 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<BreakpointLocationTreeDelegate>(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<BreakpointTreeDelegate>(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);
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
