aboutsummaryrefslogtreecommitdiff
path: root/gdb/gdbthread.h
diff options
context:
space:
mode:
authorPedro Alves <pedro@palves.net>2021-06-11 18:28:32 -0400
committerSimon Marchi <simon.marchi@polymtl.ca>2021-07-12 20:46:52 -0400
commitbf8093108164a7ed7fdf4c6dc751e0b2043caf7b (patch)
tree0b37758a65adfb5e6c26fb17cf52e3976936f8c4 /gdb/gdbthread.h
parentc9e7dfb64f8c8a6fe4788b15b23b709f87827a39 (diff)
downloadgdb-bf8093108164a7ed7fdf4c6dc751e0b2043caf7b.zip
gdb-bf8093108164a7ed7fdf4c6dc751e0b2043caf7b.tar.gz
gdb-bf8093108164a7ed7fdf4c6dc751e0b2043caf7b.tar.bz2
gdb: introduce intrusive_list, make thread_info use it
GDB currently has several objects that are put in a singly linked list, by having the object's type have a "next" pointer directly. For example, struct thread_info and struct inferior. Because these are simply-linked lists, and we don't keep track of a "tail" pointer, when we want to append a new element on the list, we need to walk the whole list to find the current tail. It would be nice to get rid of that walk. Removing elements from such lists also requires a walk, to find the "previous" position relative to the element being removed. To eliminate the need for that walk, we could make those lists doubly-linked, by adding a "prev" pointer alongside "next". It would be nice to avoid the boilerplate associated with maintaining such a list manually, though. That is what the new intrusive_list type addresses. With an intrusive list, it's also possible to move items out of the list without destroying them, which is interesting in our case for example for threads, when we exit them, but can't destroy them immediately. We currently keep exited threads on the thread list, but we could change that which would simplify some things. Note that with std::list, element removal is O(N). I.e., with std::list, we need to walk the list to find the iterator pointing to the position to remove. However, we could store a list iterator inside the object as soon as we put the object in the list, to address it, because std::list iterators are not invalidated when other elements are added/removed. However, if you need to put the same object in more than one list, then std::list<object> doesn't work. You need to instead use std::list<object *>, which is less efficient for requiring extra memory allocations. For an example of an object in multiple lists, see the step_over_next/step_over_prev fields in thread_info: /* Step-over chain. A thread is in the step-over queue if these are non-NULL. If only a single thread is in the chain, then these fields point to self. */ struct thread_info *step_over_prev = NULL; struct thread_info *step_over_next = NULL; The new intrusive_list type gives us the advantages of an intrusive linked list, while avoiding the boilerplate associated with manually maintaining it. intrusive_list's API follows the standard container interface, and thus std::list's interface. It is based the API of Boost's intrusive list, here: https://www.boost.org/doc/libs/1_73_0/doc/html/boost/intrusive/list.html Our implementation is relatively simple, while Boost's is complicated and intertwined due to a lot of customization options, which our version doesn't have. The easiest way to use an intrusive_list is to make the list's element type inherit from intrusive_node. This adds a prev/next pointers to the element type. However, to support putting the same object in more than one list, intrusive_list supports putting the "node" info as a field member, so you can have more than one such nodes, one per list. As a first guinea pig, this patch makes the per-inferior thread list use intrusive_list using the base class method. Unlike Boost's implementation, ours is not a circular list. An earlier version of the patch was circular: the intrusive_list type included an intrusive_list_node "head". In this design, a node contained pointers to the previous and next nodes, not the previous and next elements. This wasn't great for when debugging GDB with GDB, as it was difficult to get from a pointer to the node to a pointer to the element. With the design proposed in this patch, nodes contain pointers to the previous and next elements, making it easy to traverse the list by hand and inspect each element. The intrusive_list object contains pointers to the first and last elements of the list. They are nullptr if the list is empty. Each element's node contains a pointer to the previous and next elements. The first element's previous pointer is nullptr and the last element's next pointer is nullptr. Therefore, if there's a single element in the list, both its previous and next pointers are nullptr. To differentiate such an element from an element that is not linked into a list, the previous and next pointers contain a special value (-1) when the node is not linked. This is necessary to be able to reliably tell if a given node is currently linked or not. A begin() iterator points to the first item in the list. An end() iterator contains nullptr. This makes iteration until end naturally work, as advancing past the last element will make the iterator contain nullptr, making it equal to the end iterator. If the list is empty, a begin() iterator will contain nullptr from the start, and therefore be immediately equal to the end. Iterating on an intrusive_list yields references to objects (e.g. `thread_info&`). The rest of GDB currently expects iterators and ranges to yield pointers (e.g. `thread_info*`). To bridge the gap, add the reference_to_pointer_iterator type. It is used to define inf_threads_iterator. Add a Python pretty-printer, to help inspecting intrusive lists when debugging GDB with GDB. Here's an example of the output: (top-gdb) p current_inferior_.m_obj.thread_list $1 = intrusive list of thread_info = {0x61700002c000, 0x617000069080, 0x617000069400, 0x61700006d680, 0x61700006eb80} It's not possible with current master, but with this patch [1] that I hope will be merged eventually, it's possible to index the list and access the pretty-printed value's children: (top-gdb) p current_inferior_.m_obj.thread_list[1] $2 = (thread_info *) 0x617000069080 (top-gdb) p current_inferior_.m_obj.thread_list[1].ptid $3 = { m_pid = 406499, m_lwp = 406503, m_tid = 0 } Even though iterating the list in C++ yields references, the Python pretty-printer yields pointers. The reason for this is that the output of printing the thread list above would be unreadable, IMO, if each thread_info object was printed in-line, since they contain so much information. I think it's more useful to print pointers, and let the user drill down as needed. [1] https://sourceware.org/pipermail/gdb-patches/2021-April/178050.html Co-Authored-By: Simon Marchi <simon.marchi@efficios.com> Change-Id: I3412a14dc77f25876d742dab8f44e0ba7c7586c0
Diffstat (limited to 'gdb/gdbthread.h')
-rw-r--r--gdb/gdbthread.h13
1 files changed, 10 insertions, 3 deletions
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index cb3dcc3..1305b20 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -33,6 +33,7 @@ struct symtab;
#include "gdbsupport/common-gdbthread.h"
#include "gdbsupport/forward-scope-exit.h"
#include "displaced-stepping.h"
+#include "gdbsupport/intrusive_list.h"
struct inferior;
struct process_stratum_target;
@@ -222,9 +223,12 @@ struct private_thread_info
delete_thread). All other thread references are considered weak
references. Placing a thread in the thread list is an implicit
strong reference, and is thus not accounted for in the thread's
- refcount. */
+ refcount.
-class thread_info : public refcounted_object
+ The intrusive_list_node base links threads in a per-inferior list. */
+
+class thread_info : public refcounted_object,
+ public intrusive_list_node<thread_info>
{
public:
explicit thread_info (inferior *inf, ptid_t ptid);
@@ -235,7 +239,6 @@ public:
/* Mark this thread as running and notify observers. */
void set_running (bool running);
- struct thread_info *next = NULL;
ptid_t ptid; /* "Actual process id";
In fact, this may be overloaded with
kernel thread id, etc. */
@@ -435,6 +438,10 @@ extern void delete_thread (struct thread_info *thread);
this thread belonged to has already exited, for example. */
extern void delete_thread_silent (struct thread_info *thread);
+/* Mark the thread exited, but don't delete it or remove it from the
+ inferior thread list. */
+extern void set_thread_exited (thread_info *tp, bool silent);
+
/* Delete a step_resume_breakpoint from the thread database. */
extern void delete_step_resume_breakpoint (struct thread_info *);