* [PATCH 1/3] gdb: make lwp_info non-POD
2021-08-28 14:58 [PATCH 0/3] Make lwp_info use intrusive_list Simon Marchi
@ 2021-08-28 14:58 ` Simon Marchi
2021-08-28 14:58 ` [PATCH 2/3] gdb: add destructor to lwp_info Simon Marchi
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Simon Marchi @ 2021-08-28 14:58 UTC (permalink / raw)
To: gdb-patches
Initialize all fields in the class declaration directly. This opens the
door to using intrusive_list, done in the following patch.
Change-Id: I38bb27410cd9ebf511d310bb86fe2ea1872c3b05
---
gdb/linux-nat.c | 27 ++++++---------------------
gdb/linux-nat.h | 38 ++++++++++++++++++++++----------------
2 files changed, 28 insertions(+), 37 deletions(-)
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index e9433b2206bc..26324af3cfa2 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -845,19 +845,10 @@ purge_lwp_list (int pid)
static struct lwp_info *
add_initial_lwp (ptid_t ptid)
{
- struct lwp_info *lp;
-
gdb_assert (ptid.lwp_p ());
- lp = XNEW (struct lwp_info);
-
- memset (lp, 0, sizeof (struct lwp_info));
-
- lp->last_resume_kind = resume_continue;
- lp->waitstatus.kind = TARGET_WAITKIND_IGNORE;
+ lwp_info *lp = new lwp_info (ptid);
- lp->ptid = ptid;
- lp->core = -1;
/* Add to sorted-by-reverse-creation-order list. */
lwp_list_add (lp);
@@ -893,16 +884,13 @@ add_lwp (ptid_t ptid)
static void
delete_lwp (ptid_t ptid)
{
- struct lwp_info *lp;
- void **slot;
- struct lwp_info dummy;
+ lwp_info dummy (ptid);
- dummy.ptid = ptid;
- slot = htab_find_slot (lwp_lwpid_htab, &dummy, NO_INSERT);
+ void **slot = htab_find_slot (lwp_lwpid_htab, &dummy, NO_INSERT);
if (slot == NULL)
return;
- lp = *(struct lwp_info **) slot;
+ lwp_info *lp = *(struct lwp_info **) slot;
gdb_assert (lp != NULL);
htab_clear_slot (lwp_lwpid_htab, slot);
@@ -920,18 +908,15 @@ delete_lwp (ptid_t ptid)
static struct lwp_info *
find_lwp_pid (ptid_t ptid)
{
- struct lwp_info *lp;
int lwp;
- struct lwp_info dummy;
if (ptid.lwp_p ())
lwp = ptid.lwp ();
else
lwp = ptid.pid ();
- dummy.ptid = ptid_t (0, lwp, 0);
- lp = (struct lwp_info *) htab_find (lwp_lwpid_htab, &dummy);
- return lp;
+ lwp_info dummy (ptid_t (0, lwp, 0));
+ return (struct lwp_info *) htab_find (lwp_lwpid_htab, &dummy);
}
/* See nat/linux-nat.h. */
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 83bd6d4a6786..050b8046da48 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -202,20 +202,26 @@ struct arch_lwp_info;
struct lwp_info
{
+ lwp_info (ptid_t ptid)
+ : ptid (ptid)
+ {
+ waitstatus.kind = TARGET_WAITKIND_IGNORE;
+ }
+
/* The process id of the LWP. This is a combination of the LWP id
and overall process id. */
ptid_t ptid;
/* If this flag is set, we need to set the event request flags the
next time we see this LWP stop. */
- int must_set_ptrace_flags;
+ int must_set_ptrace_flags = 0;
/* Non-zero if we sent this LWP a SIGSTOP (but the LWP didn't report
it back yet). */
- int signalled;
+ int signalled = 0;
/* Non-zero if this LWP is stopped. */
- int stopped;
+ int stopped = 0;
/* Non-zero if this LWP will be/has been resumed. Note that an LWP
can be marked both as stopped and resumed at the same time. This
@@ -223,38 +229,38 @@ struct lwp_info
pending. We shouldn't let the LWP run until that wait status has
been processed, but we should not report that wait status if GDB
didn't try to let the LWP run. */
- int resumed;
+ int resumed = 0;
/* The last resume GDB requested on this thread. */
- enum resume_kind last_resume_kind;
+ resume_kind last_resume_kind = resume_continue;
/* If non-zero, a pending wait status. */
- int status;
+ int status = 0;
/* When 'stopped' is set, this is where the lwp last stopped, with
decr_pc_after_break already accounted for. If the LWP is
running and stepping, this is the address at which the lwp was
resumed (that is, it's the previous stop PC). If the LWP is
running and not stepping, this is 0. */
- CORE_ADDR stop_pc;
+ CORE_ADDR stop_pc = 0;
/* Non-zero if we were stepping this LWP. */
- int step;
+ int step = 0;
/* The reason the LWP last stopped, if we need to track it
(breakpoint, watchpoint, etc.). */
- enum target_stop_reason stop_reason;
+ target_stop_reason stop_reason = TARGET_STOPPED_BY_NO_REASON;
/* On architectures where it is possible to know the data address of
a triggered watchpoint, STOPPED_DATA_ADDRESS_P is non-zero, and
STOPPED_DATA_ADDRESS contains such data address. Otherwise,
STOPPED_DATA_ADDRESS_P is false, and STOPPED_DATA_ADDRESS is
undefined. Only valid if STOPPED_BY_WATCHPOINT is true. */
- int stopped_data_address_p;
- CORE_ADDR stopped_data_address;
+ int stopped_data_address_p = 0;
+ CORE_ADDR stopped_data_address = 0;
/* Non-zero if we expect a duplicated SIGINT. */
- int ignore_sigint;
+ int ignore_sigint = 0;
/* If WAITSTATUS->KIND != TARGET_WAITKIND_SPURIOUS, the waitstatus
for this LWP's last event. This may correspond to STATUS above,
@@ -269,15 +275,15 @@ struct lwp_info
enum target_waitkind syscall_state;
/* The processor core this LWP was last seen on. */
- int core;
+ int core = -1;
/* Arch-specific additions. */
- struct arch_lwp_info *arch_private;
+ struct arch_lwp_info *arch_private = nullptr;
/* Previous and next pointers in doubly-linked list of known LWPs,
sorted by reverse creation order. */
- struct lwp_info *prev;
- struct lwp_info *next;
+ struct lwp_info *prev = nullptr;
+ struct lwp_info *next = nullptr;
};
/* The global list of LWPs, for ALL_LWPS. Unlike the threads list,
--
2.33.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/3] gdb: add destructor to lwp_info
2021-08-28 14:58 [PATCH 0/3] Make lwp_info use intrusive_list Simon Marchi
2021-08-28 14:58 ` [PATCH 1/3] gdb: make lwp_info non-POD Simon Marchi
@ 2021-08-28 14:58 ` Simon Marchi
2021-08-28 14:58 ` [PATCH 3/3] gdb: use intrusive_list for linux-nat lwp_list Simon Marchi
2021-09-28 2:34 ` [PATCH 0/3] Make lwp_info use intrusive_list Simon Marchi
3 siblings, 0 replies; 5+ messages in thread
From: Simon Marchi @ 2021-08-28 14:58 UTC (permalink / raw)
To: gdb-patches
Replace the lwp_free function with a destructor. Make lwp_info
non-copyable, since there is now a destructor (we wouldn't want an
lwp_info object getting copied and this->arch_private getting deleted
twice).
Change-Id: I09fcbe967e362566d3a06fed2abca2a9955570fa
---
gdb/linux-nat.c | 11 ++++-------
gdb/linux-nat.h | 4 ++++
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 26324af3cfa2..930209db6e89 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -793,13 +793,10 @@ static int check_ptrace_stopped_lwp_gone (struct lwp_info *lp);
/* Destroy and free LP. */
-static void
-lwp_free (struct lwp_info *lp)
+lwp_info::~lwp_info ()
{
/* Let the arch specific bits release arch_lwp_info. */
- linux_target->low_delete_thread (lp->arch_private);
-
- xfree (lp);
+ linux_target->low_delete_thread (this->arch_private);
}
/* Traversal function for purge_lwp_list. */
@@ -814,7 +811,7 @@ lwp_lwpid_htab_remove_pid (void **slot, void *info)
{
htab_clear_slot (lwp_lwpid_htab, slot);
lwp_list_remove (lp);
- lwp_free (lp);
+ delete lp;
}
return 1;
@@ -899,7 +896,7 @@ delete_lwp (ptid_t ptid)
lwp_list_remove (lp);
/* Release. */
- lwp_free (lp);
+ delete lp;
}
/* Return a pointer to the structure describing the LWP corresponding
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 050b8046da48..b1b168d2bfee 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -208,6 +208,10 @@ struct lwp_info
waitstatus.kind = TARGET_WAITKIND_IGNORE;
}
+ ~lwp_info ();
+
+ DISABLE_COPY_AND_ASSIGN (lwp_info);
+
/* The process id of the LWP. This is a combination of the LWP id
and overall process id. */
ptid_t ptid;
--
2.33.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 3/3] gdb: use intrusive_list for linux-nat lwp_list
2021-08-28 14:58 [PATCH 0/3] Make lwp_info use intrusive_list Simon Marchi
2021-08-28 14:58 ` [PATCH 1/3] gdb: make lwp_info non-POD Simon Marchi
2021-08-28 14:58 ` [PATCH 2/3] gdb: add destructor to lwp_info Simon Marchi
@ 2021-08-28 14:58 ` Simon Marchi
2021-09-28 2:34 ` [PATCH 0/3] Make lwp_info use intrusive_list Simon Marchi
3 siblings, 0 replies; 5+ messages in thread
From: Simon Marchi @ 2021-08-28 14:58 UTC (permalink / raw)
To: gdb-patches
Replace the manually maintained linked list of lwp_info objects with
intrusive_list. Replace the ALL_LWPS macro with all_lwps, which returns
a range. Add all_lwps_safe as well, for use in iterate_over_lwps, which
currently iterates in a safe manner.
Change-Id: I355313502510acc0103f5eaf2fbde80897d6376c
---
gdb/ia64-linux-nat.c | 8 +++-----
gdb/linux-nat.c | 46 ++++++++++++++++++++++---------------------
gdb/linux-nat.h | 35 +++++++++++++++-----------------
gdb/linux-thread-db.c | 3 +--
gdb/mips-linux-nat.c | 7 ++-----
5 files changed, 46 insertions(+), 53 deletions(-)
diff --git a/gdb/ia64-linux-nat.c b/gdb/ia64-linux-nat.c
index 6381e5318e68..371d51f00bba 100644
--- a/gdb/ia64-linux-nat.c
+++ b/gdb/ia64-linux-nat.c
@@ -589,7 +589,6 @@ ia64_linux_nat_target::insert_watchpoint (CORE_ADDR addr, int len,
enum target_hw_bp_type type,
struct expression *cond)
{
- struct lwp_info *lp;
int idx;
long dbr_addr, dbr_mask;
int max_watchpoints = 4;
@@ -630,7 +629,8 @@ ia64_linux_nat_target::insert_watchpoint (CORE_ADDR addr, int len,
debug_registers[2 * idx] = dbr_addr;
debug_registers[2 * idx + 1] = dbr_mask;
- ALL_LWPS (lp)
+
+ for (const lwp_info *lp : all_lwps ())
{
store_debug_register_pair (lp->ptid, idx, &dbr_addr, &dbr_mask);
enable_watchpoints_in_psr (lp->ptid);
@@ -657,14 +657,12 @@ ia64_linux_nat_target::remove_watchpoint (CORE_ADDR addr, int len,
dbr_mask = debug_registers[2 * idx + 1];
if ((dbr_mask & (0x3UL << 62)) && addr == (CORE_ADDR) dbr_addr)
{
- struct lwp_info *lp;
-
debug_registers[2 * idx] = 0;
debug_registers[2 * idx + 1] = 0;
dbr_addr = 0;
dbr_mask = 0;
- ALL_LWPS (lp)
+ for (const lwp_info *lp : all_lwps ())
store_debug_register_pair (lp->ptid, idx, &dbr_addr, &dbr_mask);
return 0;
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 930209db6e89..fd01bd9301c0 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -421,9 +421,8 @@ static int
num_lwps (int pid)
{
int count = 0;
- struct lwp_info *lp;
- for (lp = lwp_list; lp; lp = lp->next)
+ for (const lwp_info *lp ATTRIBUTE_UNUSED : all_lwps ())
if (lp->ptid.pid () == pid)
count++;
@@ -700,17 +699,31 @@ lwp_lwpid_htab_add_lwp (struct lwp_info *lp)
creation order. This order is assumed in some cases. E.g.,
reaping status after killing alls lwps of a process: the leader LWP
must be reaped last. */
-struct lwp_info *lwp_list;
+
+static intrusive_list<lwp_info> lwp_list;
+
+/* See linux-nat.h. */
+
+lwp_info_range
+all_lwps ()
+{
+ return lwp_info_range (lwp_list.begin ());
+}
+
+/* See linux-nat.h. */
+
+lwp_info_safe_range
+all_lwps_safe ()
+{
+ return lwp_info_safe_range (lwp_list.begin ());
+}
/* Add LP to sorted-by-reverse-creation-order doubly-linked list. */
static void
lwp_list_add (struct lwp_info *lp)
{
- lp->next = lwp_list;
- if (lwp_list != NULL)
- lwp_list->prev = lp;
- lwp_list = lp;
+ lwp_list.push_front (*lp);
}
/* Remove LP from sorted-by-reverse-creation-order doubly-linked
@@ -720,12 +733,7 @@ static void
lwp_list_remove (struct lwp_info *lp)
{
/* Remove from sorted-by-creation-order list. */
- if (lp->next != NULL)
- lp->next->prev = lp->prev;
- if (lp->prev != NULL)
- lp->prev->next = lp->next;
- if (lp == lwp_list)
- lwp_list = lp->next;
+ lwp_list.erase (lwp_list.iterator_to (*lp));
}
\f
@@ -922,12 +930,8 @@ struct lwp_info *
iterate_over_lwps (ptid_t filter,
gdb::function_view<iterate_over_lwps_ftype> callback)
{
- struct lwp_info *lp, *lpnext;
-
- for (lp = lwp_list; lp; lp = lpnext)
+ for (lwp_info *lp : all_lwps_safe ())
{
- lpnext = lp->next;
-
if (lp->ptid.matches (filter))
{
if (callback (lp) != 0)
@@ -3716,8 +3720,6 @@ linux_nat_target::thread_alive (ptid_t ptid)
void
linux_nat_target::update_thread_list ()
{
- struct lwp_info *lwp;
-
/* We add/delete threads from the list as clone/exit events are
processed, so just try deleting exited threads still in the
thread list. */
@@ -3725,7 +3727,7 @@ linux_nat_target::update_thread_list ()
/* Update the processor core that each lwp/thread was last seen
running on. */
- ALL_LWPS (lwp)
+ for (lwp_info *lwp : all_lwps ())
{
/* Avoid accessing /proc if the thread hasn't run since we last
time we fetched the thread's core. Accessing /proc becomes
@@ -3949,7 +3951,7 @@ linux_proc_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf,
/* Iterate over LWPs of the current inferior, trying to access
memory through one of them. */
- for (lwp_info *lp = lwp_list; lp != nullptr; lp = lp->next)
+ for (lwp_info *lp : all_lwps ())
{
if (lp->ptid.pid () != cur_pid)
continue;
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index b1b168d2bfee..74b5eddc1365 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -196,11 +196,9 @@ extern linux_nat_target *linux_target;
struct arch_lwp_info;
-/* Structure describing an LWP. This is public only for the purposes
- of ALL_LWPS; target-specific code should generally not access it
- directly. */
+/* Structure describing an LWP. */
-struct lwp_info
+struct lwp_info : intrusive_list_node<lwp_info>
{
lwp_info (ptid_t ptid)
: ptid (ptid)
@@ -283,27 +281,26 @@ struct lwp_info
/* Arch-specific additions. */
struct arch_lwp_info *arch_private = nullptr;
-
- /* Previous and next pointers in doubly-linked list of known LWPs,
- sorted by reverse creation order. */
- struct lwp_info *prev = nullptr;
- struct lwp_info *next = nullptr;
};
-/* The global list of LWPs, for ALL_LWPS. Unlike the threads list,
- there is always at least one LWP on the list while the GNU/Linux
- native target is active. */
-extern struct lwp_info *lwp_list;
+/* lwp_info iterator and range types. */
+
+using lwp_info_iterator
+ = reference_to_pointer_iterator<intrusive_list<lwp_info>::iterator>;
+using lwp_info_range = iterator_range<lwp_info_iterator>;
+using lwp_info_safe_range = basic_safe_range<lwp_info_range>;
+
+/* Get an iterable range over all lwps. */
+
+lwp_info_range all_lwps ();
+
+/* Same as the above, but safe against deletion while iterating. */
+
+lwp_info_safe_range all_lwps_safe ();
/* Does the current host support PTRACE_GETREGSET? */
extern enum tribool have_ptrace_getregset;
-/* Iterate over each active thread (light-weight process). */
-#define ALL_LWPS(LP) \
- for ((LP) = lwp_list; \
- (LP) != NULL; \
- (LP) = (LP)->next)
-
/* Called from the LWP layer to inform the thread_db layer that PARENT
spawned CHILD. Both LWPs are currently stopped. This function
does whatever is required to have the child LWP under the
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index eb2aa5ac409a..aff00dc8d40e 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -920,13 +920,12 @@ try_thread_db_load_1 (struct thread_db_info *info)
if (info->td_ta_thr_iter_p == NULL)
{
- struct lwp_info *lp;
int pid = inferior_ptid.pid ();
thread_info *curr_thread = inferior_thread ();
linux_stop_and_wait_all_lwps ();
- ALL_LWPS (lp)
+ for (const lwp_info *lp : all_lwps ())
if (lp->ptid.pid () == pid)
thread_from_lwp (curr_thread, lp->ptid);
diff --git a/gdb/mips-linux-nat.c b/gdb/mips-linux-nat.c
index b21c7cb2ea68..1088a7dc1b03 100644
--- a/gdb/mips-linux-nat.c
+++ b/gdb/mips-linux-nat.c
@@ -632,12 +632,9 @@ mips_linux_nat_target::region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
static int
write_watchpoint_regs (void)
{
- struct lwp_info *lp;
- int tid;
-
- ALL_LWPS (lp)
+ for (const lwp_info *lp : all_lwps ())
{
- tid = lp->ptid.lwp ();
+ int tid = lp->ptid.lwp ();
if (ptrace (PTRACE_SET_WATCH_REGS, tid, &watch_mirror, NULL) == -1)
perror_with_name (_("Couldn't write debug register"));
}
--
2.33.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/3] Make lwp_info use intrusive_list
2021-08-28 14:58 [PATCH 0/3] Make lwp_info use intrusive_list Simon Marchi
` (2 preceding siblings ...)
2021-08-28 14:58 ` [PATCH 3/3] gdb: use intrusive_list for linux-nat lwp_list Simon Marchi
@ 2021-09-28 2:34 ` Simon Marchi
3 siblings, 0 replies; 5+ messages in thread
From: Simon Marchi @ 2021-09-28 2:34 UTC (permalink / raw)
To: gdb-patches
On 2021-08-28 10:58, Simon Marchi wrote:
> The goal of this series is to replace the manually maintained linked
> list of lwp_info objects with an intrusive_list. No user-visible
> changes expected, no regressions seen on Linux / x86-64.
>
> Simon Marchi (3):
> gdb: make lwp_info non-POD
> gdb: add destructor to lwp_info
> gdb: use intrusive_list for linux-nat lwp_list
>
> gdb/ia64-linux-nat.c | 8 ++---
> gdb/linux-nat.c | 84 ++++++++++++++++++-------------------------
> gdb/linux-nat.h | 73 ++++++++++++++++++++-----------------
> gdb/linux-thread-db.c | 3 +-
> gdb/mips-linux-nat.c | 7 ++--
> 5 files changed, 80 insertions(+), 95 deletions(-)
>
I pushed this.
Simon
^ permalink raw reply [flat|nested] 5+ messages in thread