* [PATCH 2/4] C++ify fork_info, use std::list
2019-03-05 18:43 [PATCH 0/4] Make "checkpoint" not rely on inferior_ptid Pedro Alves
` (2 preceding siblings ...)
2019-03-05 18:43 ` [PATCH 3/4] linux-fork.c: rewrite inf_has_multiple_threads Pedro Alves
@ 2019-03-05 18:43 ` Pedro Alves
2019-03-05 19:46 ` [PATCH 0/4] Make "checkpoint" not rely on inferior_ptid Tom Tromey
4 siblings, 0 replies; 8+ messages in thread
From: Pedro Alves @ 2019-03-05 18:43 UTC (permalink / raw)
To: gdb-patches
- Convert new_fork and free_fork to fork_info ctor/dtor.
- Use std::list.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* linux-fork.c: Include <list>.
(fork_list): Now a std::list instance.
(fork_info): Add ctor, dtor, and in-class initialize all fields.
(forks_exist_p, find_last_fork): Adjust.
(new_fork): Delete.
(one_fork_p): New.
(add_fork): Adjust.
(free_fork): Delete, folded into fork_info::~fork_info().
(delete_fork, find_fork_ptid, find_fork_id, find_fork_pid):
Adjust.
(init_fork_list): Delete.
(linux_fork_killall, linux_fork_mourn_inferior)
(linux_fork_detach, info_checkpoints_command): Adjust.
(_initialize_linux_fork): No longer call init_fork_list.
---
gdb/linux-fork.c | 261 +++++++++++++++++++++++--------------------------------
1 file changed, 108 insertions(+), 153 deletions(-)
diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index a748deaa3a..1c7b7ca123 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -35,30 +35,66 @@
#include <dirent.h>
#include <ctype.h>
-struct fork_info *fork_list;
-static int highest_fork_num;
+#include <list>
/* Fork list data structure: */
struct fork_info
{
- struct fork_info *next;
- ptid_t ptid;
- ptid_t parent_ptid;
- int num; /* Convenient handle (GDB fork id). */
- readonly_detached_regcache *savedregs; /* Convenient for info fork, saves
- having to actually switch contexts. */
- CORE_ADDR pc;
- int clobber_regs; /* True if we should restore saved regs. */
- off_t *filepos; /* Set of open file descriptors' offsets. */
- int maxfd;
+ explicit fork_info (pid_t pid)
+ : ptid (pid, pid, 0)
+ {
+ }
+
+ ~fork_info ()
+ {
+ /* Notes on step-resume breakpoints: since this is a concern for
+ threads, let's convince ourselves that it's not a concern for
+ forks. There are two ways for a fork_info to be created.
+ First, by the checkpoint command, in which case we're at a gdb
+ prompt and there can't be any step-resume breakpoint. Second,
+ by a fork in the user program, in which case we *may* have
+ stepped into the fork call, but regardless of whether we follow
+ the parent or the child, we will return to the same place and
+ the step-resume breakpoint, if any, will take care of itself as
+ usual. And unlike threads, we do not save a private copy of
+ the step-resume breakpoint -- so we're OK. */
+
+ if (savedregs)
+ delete savedregs;
+ if (filepos)
+ xfree (filepos);
+ }
+
+ ptid_t ptid = null_ptid;
+ ptid_t parent_ptid = null_ptid;
+
+ /* Convenient handle (GDB fork id). */
+ int num = 0;
+
+ /* Convenient for info fork, saves having to actually switch
+ contexts. */
+ readonly_detached_regcache *savedregs = nullptr;
+
+ CORE_ADDR pc = 0;
+
+ /* True if we should restore saved regs. */
+ int clobber_regs = 0;
+
+ /* Set of open file descriptors' offsets. */
+ off_t *filepos = nullptr;
+
+ int maxfd = 0;
};
+static std::list<fork_info> fork_list;
+static int highest_fork_num;
+
/* Fork list methods: */
int
forks_exist_p (void)
{
- return (fork_list != NULL);
+ return !fork_list.empty ();
}
/* Return the last fork in the list. */
@@ -66,26 +102,19 @@ forks_exist_p (void)
static struct fork_info *
find_last_fork (void)
{
- struct fork_info *last;
-
- if (fork_list == NULL)
+ if (fork_list.empty ())
return NULL;
- for (last = fork_list; last->next != NULL; last = last->next)
- ;
- return last;
+ return &fork_list.back ();
}
-/* Allocate a new fork. */
+/* Return true iff there's one fork in the list. */
-static struct fork_info *
-new_fork (pid_t pid)
+static bool
+one_fork_p ()
{
- struct fork_info *fp;
-
- fp = XCNEW (struct fork_info);
- fp->ptid = ptid_t (pid, pid, 0);
- return fp;
+ return (!fork_list.empty ()
+ && &fork_list.front () == &fork_list.back ());
}
/* Add a new fork to the internal fork list. */
@@ -93,93 +122,46 @@ new_fork (pid_t pid)
void
add_fork (pid_t pid)
{
- struct fork_info *fp = new_fork (pid);
-
- if (fork_list == NULL)
- {
- fork_list = fp;
- highest_fork_num = 0;
- }
- else
- {
- struct fork_info *last = find_last_fork ();
+ fork_list.emplace_back (pid);
- last->next = fp;
- }
+ if (one_fork_p ())
+ highest_fork_num = 0;
+ fork_info *fp = &fork_list.back ();
fp->num = ++highest_fork_num;
}
-static void
-free_fork (struct fork_info *fp)
-{
- /* Notes on step-resume breakpoints: since this is a concern for
- threads, let's convince ourselves that it's not a concern for
- forks. There are two ways for a fork_info to be created. First,
- by the checkpoint command, in which case we're at a gdb prompt
- and there can't be any step-resume breakpoint. Second, by a fork
- in the user program, in which case we *may* have stepped into the
- fork call, but regardless of whether we follow the parent or the
- child, we will return to the same place and the step-resume
- breakpoint, if any, will take care of itself as usual. And
- unlike threads, we do not save a private copy of the step-resume
- breakpoint -- so we're OK. */
-
- if (fp)
- {
- if (fp->savedregs)
- delete fp->savedregs;
- if (fp->filepos)
- xfree (fp->filepos);
- xfree (fp);
- }
-}
-
static void
delete_fork (ptid_t ptid)
{
- struct fork_info *fp, *fpprev;
-
- fpprev = NULL;
-
linux_target->low_forget_process (ptid.pid ());
- for (fp = fork_list; fp; fpprev = fp, fp = fp->next)
- if (fp->ptid == ptid)
- break;
-
- if (!fp)
- return;
-
- if (fpprev)
- fpprev->next = fp->next;
- else
- fork_list = fp->next;
-
- free_fork (fp);
+ for (auto it = fork_list.begin (); it != fork_list.end (); ++it)
+ if (it->ptid == ptid)
+ {
+ fork_list.erase (it);
- /* Special case: if there is now only one process in the list,
- and if it is (hopefully!) the current inferior_ptid, then
- remove it, leaving the list empty -- we're now down to the
- default case of debugging a single process. */
- if (fork_list != NULL && fork_list->next == NULL &&
- fork_list->ptid == inferior_ptid)
- {
- /* Last fork -- delete from list and handle as solo process
- (should be a safe recursion). */
- delete_fork (inferior_ptid);
- }
+ /* Special case: if there is now only one process in the list,
+ and if it is (hopefully!) the current inferior_ptid, then
+ remove it, leaving the list empty -- we're now down to the
+ default case of debugging a single process. */
+ if (one_fork_p () && fork_list.front ().ptid == inferior_ptid)
+ {
+ /* Last fork -- delete from list and handle as solo
+ process (should be a safe recursion). */
+ delete_fork (inferior_ptid);
+ }
+ return;
+ }
}
/* Find a fork_info by matching PTID. */
static struct fork_info *
find_fork_ptid (ptid_t ptid)
{
- struct fork_info *fp;
-
- for (fp = fork_list; fp; fp = fp->next)
- if (fp->ptid == ptid)
- return fp;
+ for (fork_info &fi : fork_list)
+ if (fi.ptid == ptid)
+ return &fi;
return NULL;
}
@@ -188,11 +170,9 @@ find_fork_ptid (ptid_t ptid)
static struct fork_info *
find_fork_id (int num)
{
- struct fork_info *fp;
-
- for (fp = fork_list; fp; fp = fp->next)
- if (fp->num == num)
- return fp;
+ for (fork_info &fi : fork_list)
+ if (fi.num == num)
+ return &fi;
return NULL;
}
@@ -201,11 +181,9 @@ find_fork_id (int num)
extern struct fork_info *
find_fork_pid (pid_t pid)
{
- struct fork_info *fp;
-
- for (fp = fork_list; fp; fp = fp->next)
- if (pid == fp->ptid.pid ())
- return fp;
+ for (fork_info &fi : fork_list)
+ if (pid == fi.ptid.pid ())
+ return &fi;
return NULL;
}
@@ -220,23 +198,6 @@ fork_id_to_ptid (int num)
return ptid_t (-1);
}
-static void
-init_fork_list (void)
-{
- struct fork_info *fp, *fpnext;
-
- if (!fork_list)
- return;
-
- for (fp = fork_list; fp; fp = fpnext)
- {
- fpnext = fp->next;
- free_fork (fp);
- }
-
- fork_list = NULL;
-}
-
/* Fork list <-> gdb interface. */
/* Utility function for fork_load/fork_save.
@@ -350,13 +311,12 @@ linux_fork_killall (void)
status for it) -- however any process may be a child
or a parent, so may get a SIGCHLD from a previously
killed child. Wait them all out. */
- struct fork_info *fp;
- pid_t pid, ret;
- int status;
- for (fp = fork_list; fp; fp = fp->next)
+ for (fork_info &fi : fork_list)
{
- pid = fp->ptid.pid ();
+ pid_t pid = fi.ptid.pid ();
+ int status;
+ pid_t ret;
do {
/* Use SIGKILL instead of PTRACE_KILL because the former works even
if the thread is running, while the later doesn't. */
@@ -367,7 +327,9 @@ linux_fork_killall (void)
died. MVS comment cut-and-pasted from linux-nat. */
} while (ret == pid && WIFSTOPPED (status));
}
- init_fork_list (); /* Clear list, prepare to start fresh. */
+
+ /* Clear list, prepare to start fresh. */
+ fork_list.clear ();
}
/* The current inferior_ptid has exited, but there are other viable
@@ -394,7 +356,7 @@ linux_fork_mourn_inferior (void)
/* There should still be a fork - if there's only one left,
delete_fork won't remove it, because we haven't updated
inferior_ptid yet. */
- gdb_assert (fork_list);
+ gdb_assert (!fork_list.empty ());
last = find_last_fork ();
fork_load_infrun_state (last);
@@ -402,7 +364,7 @@ linux_fork_mourn_inferior (void)
target_pid_to_str (inferior_ptid));
/* If there's only one fork, switch back to non-fork mode. */
- if (fork_list->next == NULL)
+ if (one_fork_p ())
delete_fork (inferior_ptid);
}
@@ -425,16 +387,16 @@ linux_fork_detach (int from_tty)
/* There should still be a fork - if there's only one left,
delete_fork won't remove it, because we haven't updated
inferior_ptid yet. */
- gdb_assert (fork_list);
+ gdb_assert (!fork_list.empty ());
- fork_load_infrun_state (fork_list);
+ fork_load_infrun_state (&fork_list.front ());
if (from_tty)
printf_filtered (_("[Switching to %s]\n"),
target_pid_to_str (inferior_ptid));
/* If there's only one fork, switch back to non-fork mode. */
- if (fork_list->next == NULL)
+ if (one_fork_p ())
delete_fork (inferior_ptid);
}
@@ -606,34 +568,31 @@ static void
info_checkpoints_command (const char *arg, int from_tty)
{
struct gdbarch *gdbarch = get_current_arch ();
- struct symtab_and_line sal;
- struct fork_info *fp;
- ULONGEST pc;
int requested = -1;
- struct fork_info *printed = NULL;
+ const fork_info *printed = NULL;
if (arg && *arg)
requested = (int) parse_and_eval_long (arg);
- for (fp = fork_list; fp; fp = fp->next)
+ for (const fork_info &fi : fork_list)
{
- if (requested > 0 && fp->num != requested)
+ if (requested > 0 && fi.num != requested)
continue;
- printed = fp;
- if (fp->ptid == inferior_ptid)
+ printed = &fi;
+ if (fi.ptid == inferior_ptid)
printf_filtered ("* ");
else
printf_filtered (" ");
- pc = fp->pc;
- printf_filtered ("%d %s", fp->num, target_pid_to_str (fp->ptid));
- if (fp->num == 0)
+ ULONGEST pc = fi.pc;
+ printf_filtered ("%d %s", fi.num, target_pid_to_str (fi.ptid));
+ if (fi.num == 0)
printf_filtered (_(" (main process)"));
printf_filtered (_(" at "));
fputs_filtered (paddress (gdbarch, pc), gdb_stdout);
- sal = find_pc_line (pc, 0);
+ symtab_and_line sal = find_pc_line (pc, 0);
if (sal.symtab)
printf_filtered (_(", file %s"),
symtab_to_filename_for_display (sal.symtab));
@@ -762,14 +721,12 @@ checkpoint_command (const char *args, int from_tty)
if (!fp)
error (_("Failed to find new fork"));
- if (fork_list->next == NULL)
+ if (one_fork_p ())
{
/* Special case -- if this is the first fork in the list (the
list was hitherto empty), then add inferior_ptid first, as a
special zeroeth fork id. */
- fork_info *first = new_fork (inferior_ptid.pid ());
- first->next = fork_list;
- fork_list = first;
+ fork_list.emplace_front (inferior_ptid.pid ());
}
fork_save_infrun_state (fp, 1);
@@ -816,8 +773,6 @@ restart_command (const char *args, int from_tty)
void
_initialize_linux_fork (void)
{
- init_fork_list ();
-
/* Checkpoint command: create a fork of the inferior process
and set it aside for later debugging. */
--
2.14.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/4] Make "checkpoint" not rely on inferior_ptid
2019-03-05 18:43 [PATCH 0/4] Make "checkpoint" not rely on inferior_ptid Pedro Alves
@ 2019-03-05 18:43 ` Pedro Alves
2019-03-05 19:32 ` Tom Tromey
2019-03-05 18:43 ` [PATCH 4/4] Eliminate fork_info::clobber_regs Pedro Alves
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2019-03-05 18:43 UTC (permalink / raw)
To: gdb-patches
Don't rely on "inferior_ptid" deep within add_fork. In the
multi-target branch, I'm forcing inferior_ptid to null_ptid early in
infrun event handling to make sure we inadvertently rely on the
current thread/target when we shouldn't, and that caught some bad
or unnecessary assumptions throughout.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* linux-fork.c (new_fork): New, split out of ...
(add_fork): ... this. Return void. Move "first fork" special
case from here, to ...
(checkpoint_command): ... here.
* linux-linux.h (add_fork): Return void.
---
gdb/linux-fork.c | 44 ++++++++++++++++++++++++++++----------------
gdb/linux-fork.h | 2 +-
2 files changed, 29 insertions(+), 17 deletions(-)
diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index b1b390c5c6..a748deaa3a 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -76,29 +76,30 @@ find_last_fork (void)
return last;
}
-/* Add a fork to the internal fork list. */
+/* Allocate a new fork. */
-struct fork_info *
-add_fork (pid_t pid)
+static struct fork_info *
+new_fork (pid_t pid)
{
struct fork_info *fp;
- if (fork_list == NULL && pid != inferior_ptid.pid ())
- {
- /* Special case -- if this is the first fork in the list
- (the list is hitherto empty), and if this new fork is
- NOT the current inferior_ptid, then add inferior_ptid
- first, as a special zeroeth fork id. */
- highest_fork_num = -1;
- add_fork (inferior_ptid.pid ()); /* safe recursion */
- }
-
fp = XCNEW (struct fork_info);
fp->ptid = ptid_t (pid, pid, 0);
- fp->num = ++highest_fork_num;
+ return fp;
+}
+
+/* Add a new fork to the internal fork list. */
+
+void
+add_fork (pid_t pid)
+{
+ struct fork_info *fp = new_fork (pid);
if (fork_list == NULL)
- fork_list = fp;
+ {
+ fork_list = fp;
+ highest_fork_num = 0;
+ }
else
{
struct fork_info *last = find_last_fork ();
@@ -106,7 +107,7 @@ add_fork (pid_t pid)
last->next = fp;
}
- return fp;
+ fp->num = ++highest_fork_num;
}
static void
@@ -760,6 +761,17 @@ checkpoint_command (const char *args, int from_tty)
if (!fp)
error (_("Failed to find new fork"));
+
+ if (fork_list->next == NULL)
+ {
+ /* Special case -- if this is the first fork in the list (the
+ list was hitherto empty), then add inferior_ptid first, as a
+ special zeroeth fork id. */
+ fork_info *first = new_fork (inferior_ptid.pid ());
+ first->next = fork_list;
+ fork_list = first;
+ }
+
fork_save_infrun_state (fp, 1);
fp->parent_ptid = last_target_ptid;
}
diff --git a/gdb/linux-fork.h b/gdb/linux-fork.h
index e6f130438c..f918bcb3d9 100644
--- a/gdb/linux-fork.h
+++ b/gdb/linux-fork.h
@@ -21,7 +21,7 @@
#define LINUX_FORK_H
struct fork_info;
-extern struct fork_info *add_fork (pid_t);
+extern void add_fork (pid_t);
extern struct fork_info *find_fork_pid (pid_t);
extern void linux_fork_killall (void);
extern void linux_fork_mourn_inferior (void);
--
2.14.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 4/4] Eliminate fork_info::clobber_regs
2019-03-05 18:43 [PATCH 0/4] Make "checkpoint" not rely on inferior_ptid Pedro Alves
2019-03-05 18:43 ` [PATCH 1/4] " Pedro Alves
@ 2019-03-05 18:43 ` Pedro Alves
2019-03-05 18:43 ` [PATCH 3/4] linux-fork.c: rewrite inf_has_multiple_threads Pedro Alves
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Pedro Alves @ 2019-03-05 18:43 UTC (permalink / raw)
To: gdb-patches
All fork_save_infrun_state callers pass '1' as CLOBBER_REGS nowadays.
The larger hunk in fork_save_infrun_state is just a reindentation.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* linux-fork.c (fork_info::clobber_regs): Delete.
(fork_load_infrun_state): Remove reference to 'clobber_regs'.
(fork_save_infrun_state): Remove 'clobber_regs' parameter. Update
comment. Adjust.
(scoped_switch_fork_info::scoped_switch_fork_info)
(checkpoint_command, linux_fork_context): Adjust
fork_save_infrun_state calls.
---
gdb/linux-fork.c | 79 +++++++++++++++++++++++++-------------------------------
1 file changed, 35 insertions(+), 44 deletions(-)
diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index 4bc454ba6d..51e8fddbc2 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -77,9 +77,6 @@ struct fork_info
CORE_ADDR pc = 0;
- /* True if we should restore saved regs. */
- int clobber_regs = 0;
-
/* Set of open file descriptors' offsets. */
off_t *filepos = nullptr;
@@ -223,7 +220,7 @@ fork_load_infrun_state (struct fork_info *fp)
linux_nat_switch_fork (fp->ptid);
- if (fp->savedregs && fp->clobber_regs)
+ if (fp->savedregs)
get_current_regcache ()->restore (fp->savedregs);
registers_changed ();
@@ -245,11 +242,10 @@ fork_load_infrun_state (struct fork_info *fp)
}
}
-/* Save infrun state for the fork PTID.
- Exported for use by linux child_follow_fork. */
+/* Save infrun state for the fork FP. */
static void
-fork_save_infrun_state (struct fork_info *fp, int clobber_regs)
+fork_save_infrun_state (struct fork_info *fp)
{
char path[PATH_MAX];
struct dirent *de;
@@ -260,44 +256,39 @@ fork_save_infrun_state (struct fork_info *fp, int clobber_regs)
fp->savedregs = new readonly_detached_regcache (*get_current_regcache ());
fp->pc = regcache_read_pc (get_current_regcache ());
- fp->clobber_regs = clobber_regs;
- if (clobber_regs)
+ /* Now save the 'state' (file position) of all open file descriptors.
+ Unfortunately fork does not take care of that for us... */
+ snprintf (path, PATH_MAX, "/proc/%ld/fd", (long) fp->ptid.pid ());
+ if ((d = opendir (path)) != NULL)
{
- /* Now save the 'state' (file position) of all open file descriptors.
- Unfortunately fork does not take care of that for us... */
- snprintf (path, PATH_MAX, "/proc/%ld/fd",
- (long) fp->ptid.pid ());
- if ((d = opendir (path)) != NULL)
+ long tmp;
+
+ fp->maxfd = 0;
+ while ((de = readdir (d)) != NULL)
{
- long tmp;
-
- fp->maxfd = 0;
- while ((de = readdir (d)) != NULL)
- {
- /* Count open file descriptors (actually find highest
- numbered). */
- tmp = strtol (&de->d_name[0], NULL, 10);
- if (fp->maxfd < tmp)
- fp->maxfd = tmp;
- }
- /* Allocate array of file positions. */
- fp->filepos = XRESIZEVEC (off_t, fp->filepos, fp->maxfd + 1);
-
- /* Initialize to -1 (invalid). */
- for (tmp = 0; tmp <= fp->maxfd; tmp++)
- fp->filepos[tmp] = -1;
-
- /* Now find actual file positions. */
- rewinddir (d);
- while ((de = readdir (d)) != NULL)
- if (isdigit (de->d_name[0]))
- {
- tmp = strtol (&de->d_name[0], NULL, 10);
- fp->filepos[tmp] = call_lseek (tmp, 0, SEEK_CUR);
- }
- closedir (d);
+ /* Count open file descriptors (actually find highest
+ numbered). */
+ tmp = strtol (&de->d_name[0], NULL, 10);
+ if (fp->maxfd < tmp)
+ fp->maxfd = tmp;
}
+ /* Allocate array of file positions. */
+ fp->filepos = XRESIZEVEC (off_t, fp->filepos, fp->maxfd + 1);
+
+ /* Initialize to -1 (invalid). */
+ for (tmp = 0; tmp <= fp->maxfd; tmp++)
+ fp->filepos[tmp] = -1;
+
+ /* Now find actual file positions. */
+ rewinddir (d);
+ while ((de = readdir (d)) != NULL)
+ if (isdigit (de->d_name[0]))
+ {
+ tmp = strtol (&de->d_name[0], NULL, 10);
+ fp->filepos[tmp] = call_lseek (tmp, 0, SEEK_CUR);
+ }
+ closedir (d);
}
}
@@ -421,7 +412,7 @@ public:
gdb_assert (m_oldfp != nullptr);
newfp = find_fork_ptid (pptid);
gdb_assert (newfp != nullptr);
- fork_save_infrun_state (m_oldfp, 1);
+ fork_save_infrun_state (m_oldfp);
remove_breakpoints ();
fork_load_infrun_state (newfp);
insert_breakpoints ();
@@ -718,7 +709,7 @@ checkpoint_command (const char *args, int from_tty)
fork_list.emplace_front (inferior_ptid.pid ());
}
- fork_save_infrun_state (fp, 1);
+ fork_save_infrun_state (fp);
fp->parent_ptid = last_target_ptid;
}
@@ -733,7 +724,7 @@ linux_fork_context (struct fork_info *newfp, int from_tty)
oldfp = find_fork_ptid (inferior_ptid);
gdb_assert (oldfp != NULL);
- fork_save_infrun_state (oldfp, 1);
+ fork_save_infrun_state (oldfp);
remove_breakpoints ();
fork_load_infrun_state (newfp);
insert_breakpoints ();
--
2.14.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 0/4] Make "checkpoint" not rely on inferior_ptid
@ 2019-03-05 18:43 Pedro Alves
2019-03-05 18:43 ` [PATCH 1/4] " Pedro Alves
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Pedro Alves @ 2019-03-05 18:43 UTC (permalink / raw)
To: gdb-patches
Patch #1 is a patch that I had on the multi-target branch. Patches
#2-#4 are follow up C++ifications/cleanups while I was at it.
Pedro Alves (4):
Make "checkpoint" not rely on inferior_ptid
C++ify fork_info, use std::list
linux-fork.c: rewrite inf_has_multiple_threads
Eliminate fork_info::clobber_regs
gdb/linux-fork.c | 377 ++++++++++++++++++++++++-------------------------------
gdb/linux-fork.h | 2 +-
2 files changed, 163 insertions(+), 216 deletions(-)
--
2.14.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/4] linux-fork.c: rewrite inf_has_multiple_threads
2019-03-05 18:43 [PATCH 0/4] Make "checkpoint" not rely on inferior_ptid Pedro Alves
2019-03-05 18:43 ` [PATCH 1/4] " Pedro Alves
2019-03-05 18:43 ` [PATCH 4/4] Eliminate fork_info::clobber_regs Pedro Alves
@ 2019-03-05 18:43 ` Pedro Alves
2019-03-05 18:43 ` [PATCH 2/4] C++ify fork_info, use std::list Pedro Alves
2019-03-05 19:46 ` [PATCH 0/4] Make "checkpoint" not rely on inferior_ptid Tom Tromey
4 siblings, 0 replies; 8+ messages in thread
From: Pedro Alves @ 2019-03-05 18:43 UTC (permalink / raw)
To: gdb-patches
There's no need to iterate over all threads of all inferiors here.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* linux-fork.c (inf_has_multiple_thread_cb): Delete.
(inf_has_multiple_threads): Return 'bool' and rewrite using
inferior_info::threads().
---
gdb/linux-fork.c | 29 +++++++++--------------------
1 file changed, 9 insertions(+), 20 deletions(-)
diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index 1c7b7ca123..4bc454ba6d 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -627,31 +627,20 @@ linux_fork_checkpointing_p (int pid)
return (checkpointing_pid == pid);
}
-/* Callback for iterate over threads. Used to check whether
- the current inferior is multi-threaded. Returns true as soon
- as it sees the second thread of the current inferior. */
-
-static int
-inf_has_multiple_thread_cb (struct thread_info *tp, void *data)
-{
- int *count_p = (int *) data;
-
- if (current_inferior ()->pid == tp->ptid.pid ())
- (*count_p)++;
-
- /* Stop the iteration if multiple threads have been detected. */
- return *count_p > 1;
-}
-
/* Return true if the current inferior is multi-threaded. */
-static int
-inf_has_multiple_threads (void)
+static bool
+inf_has_multiple_threads ()
{
int count = 0;
- iterate_over_threads (inf_has_multiple_thread_cb, &count);
- return (count > 1);
+ /* Return true as soon as we see the second thread of the current
+ inferior. */
+ for (thread_info *tp ATTRIBUTE_UNUSED : current_inferior ()->threads ())
+ if (++count > 1)
+ return true;
+
+ return false;
}
static void
--
2.14.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] Make "checkpoint" not rely on inferior_ptid
2019-03-05 18:43 ` [PATCH 1/4] " Pedro Alves
@ 2019-03-05 19:32 ` Tom Tromey
2019-03-05 19:46 ` Pedro Alves
0 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2019-03-05 19:32 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> In the
Pedro> multi-target branch, I'm forcing inferior_ptid to null_ptid early in
Pedro> infrun event handling to make sure we inadvertently rely on the
Pedro> current thread/target when we shouldn't, and that caught some bad
Pedro> or unnecessary assumptions throughout.
This read a bit strangely to me. Maybe instead write "to make sure we
don't inadvertently rely on the current thread/target,..."?
The patch itself seemed fine to me. Big +1 to reducing the impact of
globals.
Tom
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] Make "checkpoint" not rely on inferior_ptid
2019-03-05 19:32 ` Tom Tromey
@ 2019-03-05 19:46 ` Pedro Alves
0 siblings, 0 replies; 8+ messages in thread
From: Pedro Alves @ 2019-03-05 19:46 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 03/05/2019 07:32 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> In the
> Pedro> multi-target branch, I'm forcing inferior_ptid to null_ptid early in
> Pedro> infrun event handling to make sure we inadvertently rely on the
> Pedro> current thread/target when we shouldn't, and that caught some bad
> Pedro> or unnecessary assumptions throughout.
>
> This read a bit strangely to me. Maybe instead write "to make sure we
> don't inadvertently rely on the current thread/target,..."?
>
Oh, yes, I missed typing the "don't". Thanks for spotting that.
> The patch itself seemed fine to me. Big +1 to reducing the impact of
> globals.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/4] Make "checkpoint" not rely on inferior_ptid
2019-03-05 18:43 [PATCH 0/4] Make "checkpoint" not rely on inferior_ptid Pedro Alves
` (3 preceding siblings ...)
2019-03-05 18:43 ` [PATCH 2/4] C++ify fork_info, use std::list Pedro Alves
@ 2019-03-05 19:46 ` Tom Tromey
4 siblings, 0 replies; 8+ messages in thread
From: Tom Tromey @ 2019-03-05 19:46 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> Patch #1 is a patch that I had on the multi-target branch. Patches
Pedro> #2-#4 are follow up C++ifications/cleanups while I was at it.
I read through all of them and they seem fine to me.
Thanks for doing this.
Tom
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-03-05 19:46 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-05 18:43 [PATCH 0/4] Make "checkpoint" not rely on inferior_ptid Pedro Alves
2019-03-05 18:43 ` [PATCH 1/4] " Pedro Alves
2019-03-05 19:32 ` Tom Tromey
2019-03-05 19:46 ` Pedro Alves
2019-03-05 18:43 ` [PATCH 4/4] Eliminate fork_info::clobber_regs Pedro Alves
2019-03-05 18:43 ` [PATCH 3/4] linux-fork.c: rewrite inf_has_multiple_threads Pedro Alves
2019-03-05 18:43 ` [PATCH 2/4] C++ify fork_info, use std::list Pedro Alves
2019-03-05 19:46 ` [PATCH 0/4] Make "checkpoint" not rely on inferior_ptid Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox