public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Minor breakpoint cleanups
@ 2022-01-13 16:29 Tom Tromey
  2022-01-13 16:29 ` [PATCH 1/4] Unify "catch fork" and "catch vfork" Tom Tromey
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Tom Tromey @ 2022-01-13 16:29 UTC (permalink / raw)
  To: gdb-patches

I was looking into making 'breakpoint' use virtual methods -- which
turns out to be pretty involved -- but in the course of doing so I
found a few smaller cleanups that seemed worth doing.

This series removes a bit of redundant code, and splits a bit of code
out of breakpoint.c into two new files.

Regression tested on x86-64 Fedora 34.

Tom



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/4] Unify "catch fork" and "catch vfork"
  2022-01-13 16:29 [PATCH 0/4] Minor breakpoint cleanups Tom Tromey
@ 2022-01-13 16:29 ` Tom Tromey
  2022-01-13 16:29 ` [PATCH 2/4] Move "catch fork" to a new file Tom Tromey
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2022-01-13 16:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

I noticed that "catch fork" and "catch vfork" are nearly identical.
This patch simplifies the code by unifying these two cases.
---
 gdb/breakpoint.c | 183 +++++++++++------------------------------------
 1 file changed, 41 insertions(+), 142 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index c7d75731621..3c8bed6e2ea 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7730,6 +7730,9 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile)
 
 struct fork_catchpoint : public breakpoint
 {
+  /* True if the breakpoint is for vfork, false for fork.  */
+  bool is_vfork;
+
   /* Process id of a child process whose forking triggered this
      catchpoint.  This field is only valid immediately after this
      catchpoint has triggered.  */
@@ -7742,7 +7745,12 @@ struct fork_catchpoint : public breakpoint
 static int
 insert_catch_fork (struct bp_location *bl)
 {
-  return target_insert_fork_catchpoint (inferior_ptid.pid ());
+  struct fork_catchpoint *c = (struct fork_catchpoint *) bl->owner;
+
+  if (c->is_vfork)
+    return target_insert_vfork_catchpoint (inferior_ptid.pid ());
+  else
+    return target_insert_fork_catchpoint (inferior_ptid.pid ());
 }
 
 /* Implement the "remove" breakpoint_ops method for fork
@@ -7751,7 +7759,12 @@ insert_catch_fork (struct bp_location *bl)
 static int
 remove_catch_fork (struct bp_location *bl, enum remove_bp_reason reason)
 {
-  return target_remove_fork_catchpoint (inferior_ptid.pid ());
+  struct fork_catchpoint *c = (struct fork_catchpoint *) bl->owner;
+
+  if (c->is_vfork)
+    return target_remove_vfork_catchpoint (inferior_ptid.pid ());
+  else
+    return target_remove_fork_catchpoint (inferior_ptid.pid ());
 }
 
 /* Implement the "breakpoint_hit" breakpoint_ops method for fork
@@ -7764,7 +7777,9 @@ breakpoint_hit_catch_fork (const struct bp_location *bl,
 {
   struct fork_catchpoint *c = (struct fork_catchpoint *) bl->owner;
 
-  if (ws.kind () != TARGET_WAITKIND_FORKED)
+  if (ws.kind () != (c->is_vfork
+		     ? TARGET_WAITKIND_VFORKED
+		     : TARGET_WAITKIND_FORKED))
     return 0;
 
   c->forked_inferior_pid = ws.child_ptid ();
@@ -7789,11 +7804,17 @@ print_it_catch_fork (bpstat *bs)
     uiout->text ("Catchpoint ");
   if (uiout->is_mi_like_p ())
     {
-      uiout->field_string ("reason", async_reason_lookup (EXEC_ASYNC_FORK));
+      uiout->field_string ("reason",
+			   async_reason_lookup (c->is_vfork
+						? EXEC_ASYNC_VFORK
+						: EXEC_ASYNC_FORK));
       uiout->field_string ("disp", bpdisp_text (b->disposition));
     }
   uiout->field_signed ("bkptno", b->number);
-  uiout->text (" (forked process ");
+  if (c->is_vfork)
+    uiout->text (" (vforked process ");
+  else
+    uiout->text (" (forked process ");
   uiout->field_signed ("newpid", c->forked_inferior_pid.pid ());
   uiout->text ("), ");
   return PRINT_SRC_AND_LOC;
@@ -7817,7 +7838,8 @@ print_one_catch_fork (struct breakpoint *b, struct bp_location **last_loc)
   if (opts.addressprint)
     uiout->field_skip ("addr");
   annotate_field (5);
-  uiout->text ("fork");
+  const char *name = c->is_vfork ? "vfork" : "fork";
+  uiout->text (name);
   if (c->forked_inferior_pid != null_ptid)
     {
       uiout->text (", process ");
@@ -7826,7 +7848,7 @@ print_one_catch_fork (struct breakpoint *b, struct bp_location **last_loc)
     }
 
   if (uiout->is_mi_like_p ())
-    uiout->field_string ("catch-type", "fork");
+    uiout->field_string ("catch-type", name);
 }
 
 /* Implement the "print_mention" breakpoint_ops method for fork
@@ -7835,7 +7857,9 @@ print_one_catch_fork (struct breakpoint *b, struct bp_location **last_loc)
 static void
 print_mention_catch_fork (struct breakpoint *b)
 {
-  printf_filtered (_("Catchpoint %d (fork)"), b->number);
+  struct fork_catchpoint *c = (struct fork_catchpoint *) b;
+  printf_filtered (_("Catchpoint %d (%s)"), c->number,
+		   c->is_vfork ? "vfork" : "fork");
 }
 
 /* Implement the "print_recreate" breakpoint_ops method for fork
@@ -7844,7 +7868,9 @@ print_mention_catch_fork (struct breakpoint *b)
 static void
 print_recreate_catch_fork (struct breakpoint *b, struct ui_file *fp)
 {
-  fprintf_unfiltered (fp, "catch fork");
+  struct fork_catchpoint *c = (struct fork_catchpoint *) b;
+  fprintf_unfiltered (fp, "catch %s",
+		      c->is_vfork ? "vfork" : "fork");
   print_recreate_thread (b, fp);
 }
 
@@ -7852,121 +7878,6 @@ print_recreate_catch_fork (struct breakpoint *b, struct ui_file *fp)
 
 static struct breakpoint_ops catch_fork_breakpoint_ops;
 
-/* Implement the "insert" breakpoint_ops method for vfork
-   catchpoints.  */
-
-static int
-insert_catch_vfork (struct bp_location *bl)
-{
-  return target_insert_vfork_catchpoint (inferior_ptid.pid ());
-}
-
-/* Implement the "remove" breakpoint_ops method for vfork
-   catchpoints.  */
-
-static int
-remove_catch_vfork (struct bp_location *bl, enum remove_bp_reason reason)
-{
-  return target_remove_vfork_catchpoint (inferior_ptid.pid ());
-}
-
-/* Implement the "breakpoint_hit" breakpoint_ops method for vfork
-   catchpoints.  */
-
-static int
-breakpoint_hit_catch_vfork (const struct bp_location *bl,
-			    const address_space *aspace, CORE_ADDR bp_addr,
-			    const target_waitstatus &ws)
-{
-  struct fork_catchpoint *c = (struct fork_catchpoint *) bl->owner;
-
-  if (ws.kind () != TARGET_WAITKIND_VFORKED)
-    return 0;
-
-  c->forked_inferior_pid = ws.child_ptid ();
-  return 1;
-}
-
-/* Implement the "print_it" breakpoint_ops method for vfork
-   catchpoints.  */
-
-static enum print_stop_action
-print_it_catch_vfork (bpstat *bs)
-{
-  struct ui_out *uiout = current_uiout;
-  struct breakpoint *b = bs->breakpoint_at;
-  struct fork_catchpoint *c = (struct fork_catchpoint *) b;
-
-  annotate_catchpoint (b->number);
-  maybe_print_thread_hit_breakpoint (uiout);
-  if (b->disposition == disp_del)
-    uiout->text ("Temporary catchpoint ");
-  else
-    uiout->text ("Catchpoint ");
-  if (uiout->is_mi_like_p ())
-    {
-      uiout->field_string ("reason", async_reason_lookup (EXEC_ASYNC_VFORK));
-      uiout->field_string ("disp", bpdisp_text (b->disposition));
-    }
-  uiout->field_signed ("bkptno", b->number);
-  uiout->text (" (vforked process ");
-  uiout->field_signed ("newpid", c->forked_inferior_pid.pid ());
-  uiout->text ("), ");
-  return PRINT_SRC_AND_LOC;
-}
-
-/* Implement the "print_one" breakpoint_ops method for vfork
-   catchpoints.  */
-
-static void
-print_one_catch_vfork (struct breakpoint *b, struct bp_location **last_loc)
-{
-  struct fork_catchpoint *c = (struct fork_catchpoint *) b;
-  struct value_print_options opts;
-  struct ui_out *uiout = current_uiout;
-
-  get_user_print_options (&opts);
-  /* Field 4, the address, is omitted (which makes the columns not
-     line up too nicely with the headers, but the effect is relatively
-     readable).  */
-  if (opts.addressprint)
-    uiout->field_skip ("addr");
-  annotate_field (5);
-  uiout->text ("vfork");
-  if (c->forked_inferior_pid != null_ptid)
-    {
-      uiout->text (", process ");
-      uiout->field_signed ("what", c->forked_inferior_pid.pid ());
-      uiout->spaces (1);
-    }
-
-  if (uiout->is_mi_like_p ())
-    uiout->field_string ("catch-type", "vfork");
-}
-
-/* Implement the "print_mention" breakpoint_ops method for vfork
-   catchpoints.  */
-
-static void
-print_mention_catch_vfork (struct breakpoint *b)
-{
-  printf_filtered (_("Catchpoint %d (vfork)"), b->number);
-}
-
-/* Implement the "print_recreate" breakpoint_ops method for vfork
-   catchpoints.  */
-
-static void
-print_recreate_catch_vfork (struct breakpoint *b, struct ui_file *fp)
-{
-  fprintf_unfiltered (fp, "catch vfork");
-  print_recreate_thread (b, fp);
-}
-
-/* The breakpoint_ops structure to be used in vfork catchpoints.  */
-
-static struct breakpoint_ops catch_vfork_breakpoint_ops;
-
 /* An instance of this type is used to represent an solib catchpoint.
    A breakpoint is really of this type iff its ops pointer points to
    CATCH_SOLIB_BREAKPOINT_OPS.  */
@@ -8234,12 +8145,13 @@ install_breakpoint (int internal, std::unique_ptr<breakpoint> &&arg, int update_
 static void
 create_fork_vfork_event_catchpoint (struct gdbarch *gdbarch,
 				    bool temp, const char *cond_string,
-				    const struct breakpoint_ops *ops)
+				    bool is_vfork)
 {
   std::unique_ptr<fork_catchpoint> c (new fork_catchpoint ());
 
-  init_catchpoint (c.get (), gdbarch, temp, cond_string, ops);
-
+  init_catchpoint (c.get (), gdbarch, temp, cond_string,
+		   &catch_fork_breakpoint_ops);
+  c->is_vfork = is_vfork;
   c->forked_inferior_pid = null_ptid;
 
   install_breakpoint (0, std::move (c), 1);
@@ -11290,13 +11202,11 @@ catch_fork_command_1 (const char *arg, int from_tty,
     {
     case catch_fork_temporary:
     case catch_fork_permanent:
-      create_fork_vfork_event_catchpoint (gdbarch, temp, cond_string,
-					  &catch_fork_breakpoint_ops);
+      create_fork_vfork_event_catchpoint (gdbarch, temp, cond_string, false);
       break;
     case catch_vfork_temporary:
     case catch_vfork_permanent:
-      create_fork_vfork_event_catchpoint (gdbarch, temp, cond_string,
-					  &catch_vfork_breakpoint_ops);
+      create_fork_vfork_event_catchpoint (gdbarch, temp, cond_string, true);
       break;
     default:
       error (_("unsupported or unknown fork kind; cannot catch it"));
@@ -15365,17 +15275,6 @@ initialize_breakpoint_ops (void)
   ops->print_mention = print_mention_catch_fork;
   ops->print_recreate = print_recreate_catch_fork;
 
-  /* Vfork catchpoints.  */
-  ops = &catch_vfork_breakpoint_ops;
-  *ops = base_breakpoint_ops;
-  ops->insert_location = insert_catch_vfork;
-  ops->remove_location = remove_catch_vfork;
-  ops->breakpoint_hit = breakpoint_hit_catch_vfork;
-  ops->print_it = print_it_catch_vfork;
-  ops->print_one = print_one_catch_vfork;
-  ops->print_mention = print_mention_catch_vfork;
-  ops->print_recreate = print_recreate_catch_vfork;
-
   /* Exec catchpoints.  */
   ops = &catch_exec_breakpoint_ops;
   *ops = base_breakpoint_ops;
-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/4] Move "catch fork" to a new file
  2022-01-13 16:29 [PATCH 0/4] Minor breakpoint cleanups Tom Tromey
  2022-01-13 16:29 ` [PATCH 1/4] Unify "catch fork" and "catch vfork" Tom Tromey
@ 2022-01-13 16:29 ` Tom Tromey
  2022-01-13 16:29 ` [PATCH 3/4] Move "catch exec" " Tom Tromey
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2022-01-13 16:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

The "catch fork" code is reasonably self-contained, and so this patch
moves it out of breakpoint.c (the second largest source file in gdb)
and into a new file, break-catch-fork.c.
---
 gdb/Makefile.in        |   1 +
 gdb/break-catch-fork.c | 286 +++++++++++++++++++++++++++++++++++++++++
 gdb/breakpoint.c       | 243 ----------------------------------
 3 files changed, 287 insertions(+), 243 deletions(-)
 create mode 100644 gdb/break-catch-fork.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index d0db5fbdee1..9ea595bed4c 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -999,6 +999,7 @@ COMMON_SFILES = \
 	bfd-target.c \
 	block.c \
 	blockframe.c \
+	break-catch-fork.c \
 	break-catch-sig.c \
 	break-catch-syscall.c \
 	break-catch-throw.c \
diff --git a/gdb/break-catch-fork.c b/gdb/break-catch-fork.c
new file mode 100644
index 00000000000..af44a9fe76e
--- /dev/null
+++ b/gdb/break-catch-fork.c
@@ -0,0 +1,286 @@
+/* Everything about vfork catchpoints, for GDB.
+
+   Copyright (C) 1986-2022 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+
+#include "annotate.h"
+#include "arch-utils.h"
+#include "breakpoint.h"
+#include "cli/cli-decode.h"
+#include "inferior.h"
+#include "mi/mi-common.h"
+#include "target.h"
+#include "valprint.h"
+
+/* An instance of this type is used to represent a fork or vfork
+   catchpoint.  A breakpoint is really of this type iff its ops pointer points
+   to CATCH_FORK_BREAKPOINT_OPS.  */
+
+struct fork_catchpoint : public breakpoint
+{
+  /* True if the breakpoint is for vfork, false for fork.  */
+  bool is_vfork;
+
+  /* Process id of a child process whose forking triggered this
+     catchpoint.  This field is only valid immediately after this
+     catchpoint has triggered.  */
+  ptid_t forked_inferior_pid;
+};
+
+/* Implement the "insert" breakpoint_ops method for fork
+   catchpoints.  */
+
+static int
+insert_catch_fork (struct bp_location *bl)
+{
+  struct fork_catchpoint *c = (struct fork_catchpoint *) bl->owner;
+
+  if (c->is_vfork)
+    return target_insert_vfork_catchpoint (inferior_ptid.pid ());
+  else
+    return target_insert_fork_catchpoint (inferior_ptid.pid ());
+}
+
+/* Implement the "remove" breakpoint_ops method for fork
+   catchpoints.  */
+
+static int
+remove_catch_fork (struct bp_location *bl, enum remove_bp_reason reason)
+{
+  struct fork_catchpoint *c = (struct fork_catchpoint *) bl->owner;
+
+  if (c->is_vfork)
+    return target_remove_vfork_catchpoint (inferior_ptid.pid ());
+  else
+    return target_remove_fork_catchpoint (inferior_ptid.pid ());
+}
+
+/* Implement the "breakpoint_hit" breakpoint_ops method for fork
+   catchpoints.  */
+
+static int
+breakpoint_hit_catch_fork (const struct bp_location *bl,
+			   const address_space *aspace, CORE_ADDR bp_addr,
+			   const target_waitstatus &ws)
+{
+  struct fork_catchpoint *c = (struct fork_catchpoint *) bl->owner;
+
+  if (ws.kind () != (c->is_vfork
+		     ? TARGET_WAITKIND_VFORKED
+		     : TARGET_WAITKIND_FORKED))
+    return 0;
+
+  c->forked_inferior_pid = ws.child_ptid ();
+  return 1;
+}
+
+/* Implement the "print_it" breakpoint_ops method for fork
+   catchpoints.  */
+
+static enum print_stop_action
+print_it_catch_fork (bpstat *bs)
+{
+  struct ui_out *uiout = current_uiout;
+  struct breakpoint *b = bs->breakpoint_at;
+  struct fork_catchpoint *c = (struct fork_catchpoint *) bs->breakpoint_at;
+
+  annotate_catchpoint (b->number);
+  maybe_print_thread_hit_breakpoint (uiout);
+  if (b->disposition == disp_del)
+    uiout->text ("Temporary catchpoint ");
+  else
+    uiout->text ("Catchpoint ");
+  if (uiout->is_mi_like_p ())
+    {
+      uiout->field_string ("reason",
+			   async_reason_lookup (c->is_vfork
+						? EXEC_ASYNC_VFORK
+						: EXEC_ASYNC_FORK));
+      uiout->field_string ("disp", bpdisp_text (b->disposition));
+    }
+  uiout->field_signed ("bkptno", b->number);
+  if (c->is_vfork)
+    uiout->text (" (vforked process ");
+  else
+    uiout->text (" (forked process ");
+  uiout->field_signed ("newpid", c->forked_inferior_pid.pid ());
+  uiout->text ("), ");
+  return PRINT_SRC_AND_LOC;
+}
+
+/* Implement the "print_one" breakpoint_ops method for fork
+   catchpoints.  */
+
+static void
+print_one_catch_fork (struct breakpoint *b, struct bp_location **last_loc)
+{
+  struct fork_catchpoint *c = (struct fork_catchpoint *) b;
+  struct value_print_options opts;
+  struct ui_out *uiout = current_uiout;
+
+  get_user_print_options (&opts);
+
+  /* Field 4, the address, is omitted (which makes the columns not
+     line up too nicely with the headers, but the effect is relatively
+     readable).  */
+  if (opts.addressprint)
+    uiout->field_skip ("addr");
+  annotate_field (5);
+  const char *name = c->is_vfork ? "vfork" : "fork";
+  uiout->text (name);
+  if (c->forked_inferior_pid != null_ptid)
+    {
+      uiout->text (", process ");
+      uiout->field_signed ("what", c->forked_inferior_pid.pid ());
+      uiout->spaces (1);
+    }
+
+  if (uiout->is_mi_like_p ())
+    uiout->field_string ("catch-type", name);
+}
+
+/* Implement the "print_mention" breakpoint_ops method for fork
+   catchpoints.  */
+
+static void
+print_mention_catch_fork (struct breakpoint *b)
+{
+  struct fork_catchpoint *c = (struct fork_catchpoint *) b;
+  printf_filtered (_("Catchpoint %d (%s)"), c->number,
+		   c->is_vfork ? "vfork" : "fork");
+}
+
+/* Implement the "print_recreate" breakpoint_ops method for fork
+   catchpoints.  */
+
+static void
+print_recreate_catch_fork (struct breakpoint *b, struct ui_file *fp)
+{
+  struct fork_catchpoint *c = (struct fork_catchpoint *) b;
+  fprintf_unfiltered (fp, "catch %s",
+		      c->is_vfork ? "vfork" : "fork");
+  print_recreate_thread (b, fp);
+}
+
+/* The breakpoint_ops structure to be used in fork catchpoints.  */
+
+static struct breakpoint_ops catch_fork_breakpoint_ops;
+
+static void
+create_fork_vfork_event_catchpoint (struct gdbarch *gdbarch,
+				    bool temp, const char *cond_string,
+				    bool is_vfork)
+{
+  std::unique_ptr<fork_catchpoint> c (new fork_catchpoint ());
+
+  init_catchpoint (c.get (), gdbarch, temp, cond_string,
+		   &catch_fork_breakpoint_ops);
+  c->is_vfork = is_vfork;
+  c->forked_inferior_pid = null_ptid;
+
+  install_breakpoint (0, std::move (c), 1);
+}
+
+typedef enum
+{
+  catch_fork_temporary, catch_vfork_temporary,
+  catch_fork_permanent, catch_vfork_permanent
+}
+catch_fork_kind;
+
+static void
+catch_fork_command_1 (const char *arg, int from_tty,
+		      struct cmd_list_element *command)
+{
+  struct gdbarch *gdbarch = get_current_arch ();
+  const char *cond_string = NULL;
+  catch_fork_kind fork_kind;
+
+  fork_kind = (catch_fork_kind) (uintptr_t) command->context ();
+  bool temp = (fork_kind == catch_fork_temporary
+	       || fork_kind == catch_vfork_temporary);
+
+  if (!arg)
+    arg = "";
+  arg = skip_spaces (arg);
+
+  /* The allowed syntax is:
+     catch [v]fork
+     catch [v]fork if <cond>
+
+     First, check if there's an if clause.  */
+  cond_string = ep_parse_optional_if_clause (&arg);
+
+  if ((*arg != '\0') && !isspace (*arg))
+    error (_("Junk at end of arguments."));
+
+  /* If this target supports it, create a fork or vfork catchpoint
+     and enable reporting of such events.  */
+  switch (fork_kind)
+    {
+    case catch_fork_temporary:
+    case catch_fork_permanent:
+      create_fork_vfork_event_catchpoint (gdbarch, temp, cond_string, false);
+      break;
+    case catch_vfork_temporary:
+    case catch_vfork_permanent:
+      create_fork_vfork_event_catchpoint (gdbarch, temp, cond_string, true);
+      break;
+    default:
+      error (_("unsupported or unknown fork kind; cannot catch it"));
+      break;
+    }
+}
+
+static void
+initialize_ops ()
+{
+  struct breakpoint_ops *ops;
+
+  initialize_breakpoint_ops ();
+
+  /* Fork catchpoints.  */
+  ops = &catch_fork_breakpoint_ops;
+  *ops = base_breakpoint_ops;
+  ops->insert_location = insert_catch_fork;
+  ops->remove_location = remove_catch_fork;
+  ops->breakpoint_hit = breakpoint_hit_catch_fork;
+  ops->print_it = print_it_catch_fork;
+  ops->print_one = print_one_catch_fork;
+  ops->print_mention = print_mention_catch_fork;
+  ops->print_recreate = print_recreate_catch_fork;
+}
+
+void _initialize_break_catch_fork ();
+void
+_initialize_break_catch_fork ()
+{
+  initialize_ops ();
+
+  add_catch_command ("fork", _("Catch calls to fork."),
+		     catch_fork_command_1,
+		     NULL,
+		     (void *) (uintptr_t) catch_fork_permanent,
+		     (void *) (uintptr_t) catch_fork_temporary);
+  add_catch_command ("vfork", _("Catch calls to vfork."),
+		     catch_fork_command_1,
+		     NULL,
+		     (void *) (uintptr_t) catch_vfork_permanent,
+		     (void *) (uintptr_t) catch_vfork_temporary);
+}
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3c8bed6e2ea..cdc342aeb11 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7722,162 +7722,6 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile)
     }
 }
 
-/* FORK & VFORK catchpoints.  */
-
-/* An instance of this type is used to represent a fork or vfork
-   catchpoint.  A breakpoint is really of this type iff its ops pointer points
-   to CATCH_FORK_BREAKPOINT_OPS.  */
-
-struct fork_catchpoint : public breakpoint
-{
-  /* True if the breakpoint is for vfork, false for fork.  */
-  bool is_vfork;
-
-  /* Process id of a child process whose forking triggered this
-     catchpoint.  This field is only valid immediately after this
-     catchpoint has triggered.  */
-  ptid_t forked_inferior_pid;
-};
-
-/* Implement the "insert" breakpoint_ops method for fork
-   catchpoints.  */
-
-static int
-insert_catch_fork (struct bp_location *bl)
-{
-  struct fork_catchpoint *c = (struct fork_catchpoint *) bl->owner;
-
-  if (c->is_vfork)
-    return target_insert_vfork_catchpoint (inferior_ptid.pid ());
-  else
-    return target_insert_fork_catchpoint (inferior_ptid.pid ());
-}
-
-/* Implement the "remove" breakpoint_ops method for fork
-   catchpoints.  */
-
-static int
-remove_catch_fork (struct bp_location *bl, enum remove_bp_reason reason)
-{
-  struct fork_catchpoint *c = (struct fork_catchpoint *) bl->owner;
-
-  if (c->is_vfork)
-    return target_remove_vfork_catchpoint (inferior_ptid.pid ());
-  else
-    return target_remove_fork_catchpoint (inferior_ptid.pid ());
-}
-
-/* Implement the "breakpoint_hit" breakpoint_ops method for fork
-   catchpoints.  */
-
-static int
-breakpoint_hit_catch_fork (const struct bp_location *bl,
-			   const address_space *aspace, CORE_ADDR bp_addr,
-			   const target_waitstatus &ws)
-{
-  struct fork_catchpoint *c = (struct fork_catchpoint *) bl->owner;
-
-  if (ws.kind () != (c->is_vfork
-		     ? TARGET_WAITKIND_VFORKED
-		     : TARGET_WAITKIND_FORKED))
-    return 0;
-
-  c->forked_inferior_pid = ws.child_ptid ();
-  return 1;
-}
-
-/* Implement the "print_it" breakpoint_ops method for fork
-   catchpoints.  */
-
-static enum print_stop_action
-print_it_catch_fork (bpstat *bs)
-{
-  struct ui_out *uiout = current_uiout;
-  struct breakpoint *b = bs->breakpoint_at;
-  struct fork_catchpoint *c = (struct fork_catchpoint *) bs->breakpoint_at;
-
-  annotate_catchpoint (b->number);
-  maybe_print_thread_hit_breakpoint (uiout);
-  if (b->disposition == disp_del)
-    uiout->text ("Temporary catchpoint ");
-  else
-    uiout->text ("Catchpoint ");
-  if (uiout->is_mi_like_p ())
-    {
-      uiout->field_string ("reason",
-			   async_reason_lookup (c->is_vfork
-						? EXEC_ASYNC_VFORK
-						: EXEC_ASYNC_FORK));
-      uiout->field_string ("disp", bpdisp_text (b->disposition));
-    }
-  uiout->field_signed ("bkptno", b->number);
-  if (c->is_vfork)
-    uiout->text (" (vforked process ");
-  else
-    uiout->text (" (forked process ");
-  uiout->field_signed ("newpid", c->forked_inferior_pid.pid ());
-  uiout->text ("), ");
-  return PRINT_SRC_AND_LOC;
-}
-
-/* Implement the "print_one" breakpoint_ops method for fork
-   catchpoints.  */
-
-static void
-print_one_catch_fork (struct breakpoint *b, struct bp_location **last_loc)
-{
-  struct fork_catchpoint *c = (struct fork_catchpoint *) b;
-  struct value_print_options opts;
-  struct ui_out *uiout = current_uiout;
-
-  get_user_print_options (&opts);
-
-  /* Field 4, the address, is omitted (which makes the columns not
-     line up too nicely with the headers, but the effect is relatively
-     readable).  */
-  if (opts.addressprint)
-    uiout->field_skip ("addr");
-  annotate_field (5);
-  const char *name = c->is_vfork ? "vfork" : "fork";
-  uiout->text (name);
-  if (c->forked_inferior_pid != null_ptid)
-    {
-      uiout->text (", process ");
-      uiout->field_signed ("what", c->forked_inferior_pid.pid ());
-      uiout->spaces (1);
-    }
-
-  if (uiout->is_mi_like_p ())
-    uiout->field_string ("catch-type", name);
-}
-
-/* Implement the "print_mention" breakpoint_ops method for fork
-   catchpoints.  */
-
-static void
-print_mention_catch_fork (struct breakpoint *b)
-{
-  struct fork_catchpoint *c = (struct fork_catchpoint *) b;
-  printf_filtered (_("Catchpoint %d (%s)"), c->number,
-		   c->is_vfork ? "vfork" : "fork");
-}
-
-/* Implement the "print_recreate" breakpoint_ops method for fork
-   catchpoints.  */
-
-static void
-print_recreate_catch_fork (struct breakpoint *b, struct ui_file *fp)
-{
-  struct fork_catchpoint *c = (struct fork_catchpoint *) b;
-  fprintf_unfiltered (fp, "catch %s",
-		      c->is_vfork ? "vfork" : "fork");
-  print_recreate_thread (b, fp);
-}
-
-/* The breakpoint_ops structure to be used in fork catchpoints.  */
-
-static struct breakpoint_ops catch_fork_breakpoint_ops;
-
 /* An instance of this type is used to represent an solib catchpoint.
    A breakpoint is really of this type iff its ops pointer points to
    CATCH_SOLIB_BREAKPOINT_OPS.  */
@@ -8142,21 +7986,6 @@ install_breakpoint (int internal, std::unique_ptr<breakpoint> &&arg, int update_
     update_global_location_list (UGLL_MAY_INSERT);
 }
 
-static void
-create_fork_vfork_event_catchpoint (struct gdbarch *gdbarch,
-				    bool temp, const char *cond_string,
-				    bool is_vfork)
-{
-  std::unique_ptr<fork_catchpoint> c (new fork_catchpoint ());
-
-  init_catchpoint (c.get (), gdbarch, temp, cond_string,
-		   &catch_fork_breakpoint_ops);
-  c->is_vfork = is_vfork;
-  c->forked_inferior_pid = null_ptid;
-
-  install_breakpoint (0, std::move (c), 1);
-}
-
 /* Exec catchpoints.  */
 
 /* An instance of this type is used to represent an exec catchpoint.
@@ -11163,57 +10992,6 @@ ep_parse_optional_if_clause (const char **arg)
 /* Commands to deal with catching events, such as signals, exceptions,
    process start/exit, etc.  */
 
-typedef enum
-{
-  catch_fork_temporary, catch_vfork_temporary,
-  catch_fork_permanent, catch_vfork_permanent
-}
-catch_fork_kind;
-
-static void
-catch_fork_command_1 (const char *arg, int from_tty,
-		      struct cmd_list_element *command)
-{
-  struct gdbarch *gdbarch = get_current_arch ();
-  const char *cond_string = NULL;
-  catch_fork_kind fork_kind;
-
-  fork_kind = (catch_fork_kind) (uintptr_t) command->context ();
-  bool temp = (fork_kind == catch_fork_temporary
-	       || fork_kind == catch_vfork_temporary);
-
-  if (!arg)
-    arg = "";
-  arg = skip_spaces (arg);
-
-  /* The allowed syntax is:
-     catch [v]fork
-     catch [v]fork if <cond>
-
-     First, check if there's an if clause.  */
-  cond_string = ep_parse_optional_if_clause (&arg);
-
-  if ((*arg != '\0') && !isspace (*arg))
-    error (_("Junk at end of arguments."));
-
-  /* If this target supports it, create a fork or vfork catchpoint
-     and enable reporting of such events.  */
-  switch (fork_kind)
-    {
-    case catch_fork_temporary:
-    case catch_fork_permanent:
-      create_fork_vfork_event_catchpoint (gdbarch, temp, cond_string, false);
-      break;
-    case catch_vfork_temporary:
-    case catch_vfork_permanent:
-      create_fork_vfork_event_catchpoint (gdbarch, temp, cond_string, true);
-      break;
-    default:
-      error (_("unsupported or unknown fork kind; cannot catch it"));
-      break;
-    }
-}
-
 static void
 catch_exec_command_1 (const char *arg, int from_tty,
 		      struct cmd_list_element *command)
@@ -15264,17 +15042,6 @@ initialize_breakpoint_ops (void)
   ops->create_breakpoints_sal = strace_marker_create_breakpoints_sal;
   ops->decode_location = strace_marker_decode_location;
 
-  /* Fork catchpoints.  */
-  ops = &catch_fork_breakpoint_ops;
-  *ops = base_breakpoint_ops;
-  ops->insert_location = insert_catch_fork;
-  ops->remove_location = remove_catch_fork;
-  ops->breakpoint_hit = breakpoint_hit_catch_fork;
-  ops->print_it = print_it_catch_fork;
-  ops->print_one = print_one_catch_fork;
-  ops->print_mention = print_mention_catch_fork;
-  ops->print_recreate = print_recreate_catch_fork;
-
   /* Exec catchpoints.  */
   ops = &catch_exec_breakpoint_ops;
   *ops = base_breakpoint_ops;
@@ -15590,16 +15357,6 @@ Set temporary catchpoints to catch events."),
 			&tcatch_cmdlist,
 			0/*allow-unknown*/, &cmdlist);
 
-  add_catch_command ("fork", _("Catch calls to fork."),
-		     catch_fork_command_1,
-		     NULL,
-		     (void *) (uintptr_t) catch_fork_permanent,
-		     (void *) (uintptr_t) catch_fork_temporary);
-  add_catch_command ("vfork", _("Catch calls to vfork."),
-		     catch_fork_command_1,
-		     NULL,
-		     (void *) (uintptr_t) catch_vfork_permanent,
-		     (void *) (uintptr_t) catch_vfork_temporary);
   add_catch_command ("exec", _("Catch calls to exec."),
 		     catch_exec_command_1,
 		     NULL,
-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/4] Move "catch exec" to a new file
  2022-01-13 16:29 [PATCH 0/4] Minor breakpoint cleanups Tom Tromey
  2022-01-13 16:29 ` [PATCH 1/4] Unify "catch fork" and "catch vfork" Tom Tromey
  2022-01-13 16:29 ` [PATCH 2/4] Move "catch fork" to a new file Tom Tromey
@ 2022-01-13 16:29 ` Tom Tromey
  2022-01-13 16:29 ` [PATCH 4/4] Simplify Ada catchpoints Tom Tromey
  2022-01-13 20:05 ` [PATCH 0/4] Minor breakpoint cleanups John Baldwin
  4 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2022-01-13 16:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

The "catch exec" code is reasonably self-contained, and so this patch
moves it out of breakpoint.c (the second largest source file in gdb)
and into a new file, break-catch-exec.c.
---
 gdb/Makefile.in        |   1 +
 gdb/break-catch-exec.c | 236 +++++++++++++++++++++++++++++++++++++++++
 gdb/breakpoint.c       | 191 ---------------------------------
 3 files changed, 237 insertions(+), 191 deletions(-)
 create mode 100644 gdb/break-catch-exec.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 9ea595bed4c..28fa8f0fdb5 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -999,6 +999,7 @@ COMMON_SFILES = \
 	bfd-target.c \
 	block.c \
 	blockframe.c \
+	break-catch-exec.c \
 	break-catch-fork.c \
 	break-catch-sig.c \
 	break-catch-syscall.c \
diff --git a/gdb/break-catch-exec.c b/gdb/break-catch-exec.c
new file mode 100644
index 00000000000..c0cf2aeb3b5
--- /dev/null
+++ b/gdb/break-catch-exec.c
@@ -0,0 +1,236 @@
+/* Everything about exec catchpoints, for GDB.
+
+   Copyright (C) 1986-2022 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+
+#include "annotate.h"
+#include "arch-utils.h"
+#include "breakpoint.h"
+#include "cli/cli-decode.h"
+#include "inferior.h"
+#include "mi/mi-common.h"
+#include "target.h"
+#include "valprint.h"
+
+/* Exec catchpoints.  */
+
+/* An instance of this type is used to represent an exec catchpoint.
+   A breakpoint is really of this type iff its ops pointer points to
+   CATCH_EXEC_BREAKPOINT_OPS.  */
+
+struct exec_catchpoint : public breakpoint
+{
+  /* Filename of a program whose exec triggered this catchpoint.
+     This field is only valid immediately after this catchpoint has
+     triggered.  */
+  gdb::unique_xmalloc_ptr<char> exec_pathname;
+};
+
+static int
+insert_catch_exec (struct bp_location *bl)
+{
+  return target_insert_exec_catchpoint (inferior_ptid.pid ());
+}
+
+static int
+remove_catch_exec (struct bp_location *bl, enum remove_bp_reason reason)
+{
+  return target_remove_exec_catchpoint (inferior_ptid.pid ());
+}
+
+static int
+breakpoint_hit_catch_exec (const struct bp_location *bl,
+			   const address_space *aspace, CORE_ADDR bp_addr,
+			   const target_waitstatus &ws)
+{
+  struct exec_catchpoint *c = (struct exec_catchpoint *) bl->owner;
+
+  if (ws.kind () != TARGET_WAITKIND_EXECD)
+    return 0;
+
+  c->exec_pathname = make_unique_xstrdup (ws.execd_pathname ());
+  return 1;
+}
+
+static enum print_stop_action
+print_it_catch_exec (bpstat *bs)
+{
+  struct ui_out *uiout = current_uiout;
+  struct breakpoint *b = bs->breakpoint_at;
+  struct exec_catchpoint *c = (struct exec_catchpoint *) b;
+
+  annotate_catchpoint (b->number);
+  maybe_print_thread_hit_breakpoint (uiout);
+  if (b->disposition == disp_del)
+    uiout->text ("Temporary catchpoint ");
+  else
+    uiout->text ("Catchpoint ");
+  if (uiout->is_mi_like_p ())
+    {
+      uiout->field_string ("reason", async_reason_lookup (EXEC_ASYNC_EXEC));
+      uiout->field_string ("disp", bpdisp_text (b->disposition));
+    }
+  uiout->field_signed ("bkptno", b->number);
+  uiout->text (" (exec'd ");
+  uiout->field_string ("new-exec", c->exec_pathname.get ());
+  uiout->text ("), ");
+
+  return PRINT_SRC_AND_LOC;
+}
+
+static void
+print_one_catch_exec (struct breakpoint *b, struct bp_location **last_loc)
+{
+  struct exec_catchpoint *c = (struct exec_catchpoint *) b;
+  struct value_print_options opts;
+  struct ui_out *uiout = current_uiout;
+
+  get_user_print_options (&opts);
+
+  /* Field 4, the address, is omitted (which makes the columns
+     not line up too nicely with the headers, but the effect
+     is relatively readable).  */
+  if (opts.addressprint)
+    uiout->field_skip ("addr");
+  annotate_field (5);
+  uiout->text ("exec");
+  if (c->exec_pathname != NULL)
+    {
+      uiout->text (", program \"");
+      uiout->field_string ("what", c->exec_pathname.get ());
+      uiout->text ("\" ");
+    }
+
+  if (uiout->is_mi_like_p ())
+    uiout->field_string ("catch-type", "exec");
+}
+
+static void
+print_mention_catch_exec (struct breakpoint *b)
+{
+  printf_filtered (_("Catchpoint %d (exec)"), b->number);
+}
+
+/* Implement the "print_recreate" breakpoint_ops method for exec
+   catchpoints.  */
+
+static void
+print_recreate_catch_exec (struct breakpoint *b, struct ui_file *fp)
+{
+  fprintf_unfiltered (fp, "catch exec");
+  print_recreate_thread (b, fp);
+}
+
+static struct breakpoint_ops catch_exec_breakpoint_ops;
+
+/* This function attempts to parse an optional "if <cond>" clause
+   from the arg string.  If one is not found, it returns NULL.
+
+   Else, it returns a pointer to the condition string.  (It does not
+   attempt to evaluate the string against a particular block.)  And,
+   it updates arg to point to the first character following the parsed
+   if clause in the arg string.  */
+
+const char *
+ep_parse_optional_if_clause (const char **arg)
+{
+  const char *cond_string;
+
+  if (((*arg)[0] != 'i') || ((*arg)[1] != 'f') || !isspace ((*arg)[2]))
+    return NULL;
+
+  /* Skip the "if" keyword.  */
+  (*arg) += 2;
+
+  /* Skip any extra leading whitespace, and record the start of the
+     condition string.  */
+  *arg = skip_spaces (*arg);
+  cond_string = *arg;
+
+  /* Assume that the condition occupies the remainder of the arg
+     string.  */
+  (*arg) += strlen (cond_string);
+
+  return cond_string;
+}
+
+/* Commands to deal with catching events, such as signals, exceptions,
+   process start/exit, etc.  */
+
+static void
+catch_exec_command_1 (const char *arg, int from_tty,
+		      struct cmd_list_element *command)
+{
+  struct gdbarch *gdbarch = get_current_arch ();
+  const char *cond_string = NULL;
+  bool temp = command->context () == CATCH_TEMPORARY;
+
+  if (!arg)
+    arg = "";
+  arg = skip_spaces (arg);
+
+  /* The allowed syntax is:
+     catch exec
+     catch exec if <cond>
+
+     First, check if there's an if clause.  */
+  cond_string = ep_parse_optional_if_clause (&arg);
+
+  if ((*arg != '\0') && !isspace (*arg))
+    error (_("Junk at end of arguments."));
+
+  std::unique_ptr<exec_catchpoint> c (new exec_catchpoint ());
+  init_catchpoint (c.get (), gdbarch, temp, cond_string,
+		   &catch_exec_breakpoint_ops);
+  c->exec_pathname.reset ();
+
+  install_breakpoint (0, std::move (c), 1);
+}
+
+static void
+initialize_ops ()
+{
+  struct breakpoint_ops *ops;
+
+  initialize_breakpoint_ops ();
+
+  /* Exec catchpoints.  */
+  ops = &catch_exec_breakpoint_ops;
+  *ops = base_breakpoint_ops;
+  ops->insert_location = insert_catch_exec;
+  ops->remove_location = remove_catch_exec;
+  ops->breakpoint_hit = breakpoint_hit_catch_exec;
+  ops->print_it = print_it_catch_exec;
+  ops->print_one = print_one_catch_exec;
+  ops->print_mention = print_mention_catch_exec;
+  ops->print_recreate = print_recreate_catch_exec;
+}
+
+void _initialize_break_catch_exec ();
+void
+_initialize_break_catch_exec ()
+{
+  initialize_ops ();
+
+  add_catch_command ("exec", _("Catch calls to exec."),
+		     catch_exec_command_1,
+		     NULL,
+		     CATCH_PERMANENT,
+		     CATCH_TEMPORARY);
+}
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index cdc342aeb11..9c223a4f6d3 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7986,117 +7986,6 @@ install_breakpoint (int internal, std::unique_ptr<breakpoint> &&arg, int update_
     update_global_location_list (UGLL_MAY_INSERT);
 }
 
-/* Exec catchpoints.  */
-
-/* An instance of this type is used to represent an exec catchpoint.
-   A breakpoint is really of this type iff its ops pointer points to
-   CATCH_EXEC_BREAKPOINT_OPS.  */
-
-struct exec_catchpoint : public breakpoint
-{
-  /* Filename of a program whose exec triggered this catchpoint.
-     This field is only valid immediately after this catchpoint has
-     triggered.  */
-  gdb::unique_xmalloc_ptr<char> exec_pathname;
-};
-
-static int
-insert_catch_exec (struct bp_location *bl)
-{
-  return target_insert_exec_catchpoint (inferior_ptid.pid ());
-}
-
-static int
-remove_catch_exec (struct bp_location *bl, enum remove_bp_reason reason)
-{
-  return target_remove_exec_catchpoint (inferior_ptid.pid ());
-}
-
-static int
-breakpoint_hit_catch_exec (const struct bp_location *bl,
-			   const address_space *aspace, CORE_ADDR bp_addr,
-			   const target_waitstatus &ws)
-{
-  struct exec_catchpoint *c = (struct exec_catchpoint *) bl->owner;
-
-  if (ws.kind () != TARGET_WAITKIND_EXECD)
-    return 0;
-
-  c->exec_pathname = make_unique_xstrdup (ws.execd_pathname ());
-  return 1;
-}
-
-static enum print_stop_action
-print_it_catch_exec (bpstat *bs)
-{
-  struct ui_out *uiout = current_uiout;
-  struct breakpoint *b = bs->breakpoint_at;
-  struct exec_catchpoint *c = (struct exec_catchpoint *) b;
-
-  annotate_catchpoint (b->number);
-  maybe_print_thread_hit_breakpoint (uiout);
-  if (b->disposition == disp_del)
-    uiout->text ("Temporary catchpoint ");
-  else
-    uiout->text ("Catchpoint ");
-  if (uiout->is_mi_like_p ())
-    {
-      uiout->field_string ("reason", async_reason_lookup (EXEC_ASYNC_EXEC));
-      uiout->field_string ("disp", bpdisp_text (b->disposition));
-    }
-  uiout->field_signed ("bkptno", b->number);
-  uiout->text (" (exec'd ");
-  uiout->field_string ("new-exec", c->exec_pathname.get ());
-  uiout->text ("), ");
-
-  return PRINT_SRC_AND_LOC;
-}
-
-static void
-print_one_catch_exec (struct breakpoint *b, struct bp_location **last_loc)
-{
-  struct exec_catchpoint *c = (struct exec_catchpoint *) b;
-  struct value_print_options opts;
-  struct ui_out *uiout = current_uiout;
-
-  get_user_print_options (&opts);
-
-  /* Field 4, the address, is omitted (which makes the columns
-     not line up too nicely with the headers, but the effect
-     is relatively readable).  */
-  if (opts.addressprint)
-    uiout->field_skip ("addr");
-  annotate_field (5);
-  uiout->text ("exec");
-  if (c->exec_pathname != NULL)
-    {
-      uiout->text (", program \"");
-      uiout->field_string ("what", c->exec_pathname.get ());
-      uiout->text ("\" ");
-    }
-
-  if (uiout->is_mi_like_p ())
-    uiout->field_string ("catch-type", "exec");
-}
-
-static void
-print_mention_catch_exec (struct breakpoint *b)
-{
-  printf_filtered (_("Catchpoint %d (exec)"), b->number);
-}
-
-/* Implement the "print_recreate" breakpoint_ops method for exec
-   catchpoints.  */
-
-static void
-print_recreate_catch_exec (struct breakpoint *b, struct ui_file *fp)
-{
-  fprintf_unfiltered (fp, "catch exec");
-  print_recreate_thread (b, fp);
-}
-
-static struct breakpoint_ops catch_exec_breakpoint_ops;
-
 static int
 hw_breakpoint_used_count (void)
 {
@@ -10958,70 +10847,6 @@ until_break_command (const char *arg, int from_tty, int anywhere)
   proceed (-1, GDB_SIGNAL_DEFAULT);
 }
 
-/* This function attempts to parse an optional "if <cond>" clause
-   from the arg string.  If one is not found, it returns NULL.
-
-   Else, it returns a pointer to the condition string.  (It does not
-   attempt to evaluate the string against a particular block.)  And,
-   it updates arg to point to the first character following the parsed
-   if clause in the arg string.  */
-
-const char *
-ep_parse_optional_if_clause (const char **arg)
-{
-  const char *cond_string;
-
-  if (((*arg)[0] != 'i') || ((*arg)[1] != 'f') || !isspace ((*arg)[2]))
-    return NULL;
-
-  /* Skip the "if" keyword.  */
-  (*arg) += 2;
-
-  /* Skip any extra leading whitespace, and record the start of the
-     condition string.  */
-  *arg = skip_spaces (*arg);
-  cond_string = *arg;
-
-  /* Assume that the condition occupies the remainder of the arg
-     string.  */
-  (*arg) += strlen (cond_string);
-
-  return cond_string;
-}
-
-/* Commands to deal with catching events, such as signals, exceptions,
-   process start/exit, etc.  */
-
-static void
-catch_exec_command_1 (const char *arg, int from_tty,
-		      struct cmd_list_element *command)
-{
-  struct gdbarch *gdbarch = get_current_arch ();
-  const char *cond_string = NULL;
-  bool temp = command->context () == CATCH_TEMPORARY;
-
-  if (!arg)
-    arg = "";
-  arg = skip_spaces (arg);
-
-  /* The allowed syntax is:
-     catch exec
-     catch exec if <cond>
-
-     First, check if there's an if clause.  */
-  cond_string = ep_parse_optional_if_clause (&arg);
-
-  if ((*arg != '\0') && !isspace (*arg))
-    error (_("Junk at end of arguments."));
-
-  std::unique_ptr<exec_catchpoint> c (new exec_catchpoint ());
-  init_catchpoint (c.get (), gdbarch, temp, cond_string,
-		   &catch_exec_breakpoint_ops);
-  c->exec_pathname.reset ();
-
-  install_breakpoint (0, std::move (c), 1);
-}
-
 void
 init_ada_exception_breakpoint (struct breakpoint *b,
 			       struct gdbarch *gdbarch,
@@ -15042,17 +14867,6 @@ initialize_breakpoint_ops (void)
   ops->create_breakpoints_sal = strace_marker_create_breakpoints_sal;
   ops->decode_location = strace_marker_decode_location;
 
-  /* Exec catchpoints.  */
-  ops = &catch_exec_breakpoint_ops;
-  *ops = base_breakpoint_ops;
-  ops->insert_location = insert_catch_exec;
-  ops->remove_location = remove_catch_exec;
-  ops->breakpoint_hit = breakpoint_hit_catch_exec;
-  ops->print_it = print_it_catch_exec;
-  ops->print_one = print_one_catch_exec;
-  ops->print_mention = print_mention_catch_exec;
-  ops->print_recreate = print_recreate_catch_exec;
-
   /* Solib-related catchpoints.  */
   ops = &catch_solib_breakpoint_ops;
   *ops = base_breakpoint_ops;
@@ -15357,11 +15171,6 @@ Set temporary catchpoints to catch events."),
 			&tcatch_cmdlist,
 			0/*allow-unknown*/, &cmdlist);
 
-  add_catch_command ("exec", _("Catch calls to exec."),
-		     catch_exec_command_1,
-		     NULL,
-		     CATCH_PERMANENT,
-		     CATCH_TEMPORARY);
   add_catch_command ("load", _("Catch loads of shared libraries.\n\
 Usage: catch load [REGEX]\n\
 If REGEX is given, only stop for libraries matching the regular expression."),
-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 4/4] Simplify Ada catchpoints
  2022-01-13 16:29 [PATCH 0/4] Minor breakpoint cleanups Tom Tromey
                   ` (2 preceding siblings ...)
  2022-01-13 16:29 ` [PATCH 3/4] Move "catch exec" " Tom Tromey
@ 2022-01-13 16:29 ` Tom Tromey
  2022-01-13 20:05 ` [PATCH 0/4] Minor breakpoint cleanups John Baldwin
  4 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2022-01-13 16:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

All the Ada catchpoints use the same breakpoint_ops contents, because
the catchpoint itself records its kind.  This patch simplifies the
code by removing the redundant ops structures.
---
 gdb/ada-lang.c | 68 +++-----------------------------------------------
 1 file changed, 3 insertions(+), 65 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index f2f8617cc1e..80f98930b5f 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12042,21 +12042,15 @@ print_recreate_exception (struct breakpoint *b, struct ui_file *fp)
   print_recreate_thread (b, fp);
 }
 
-/* Virtual tables for various breakpoint types.  */
+/* Virtual table for breakpoint type.  */
 static struct breakpoint_ops catch_exception_breakpoint_ops;
-static struct breakpoint_ops catch_exception_unhandled_breakpoint_ops;
-static struct breakpoint_ops catch_assert_breakpoint_ops;
-static struct breakpoint_ops catch_handlers_breakpoint_ops;
 
 /* See ada-lang.h.  */
 
 bool
 is_ada_exception_catchpoint (breakpoint *bp)
 {
-  return (bp->ops == &catch_exception_breakpoint_ops
-	  || bp->ops == &catch_exception_unhandled_breakpoint_ops
-	  || bp->ops == &catch_assert_breakpoint_ops
-	  || bp->ops == &catch_handlers_breakpoint_ops);
+  return bp->ops == &catch_exception_breakpoint_ops;
 }
 
 /* Split the arguments specified in a "catch exception" command.  
@@ -12166,32 +12160,6 @@ ada_exception_sym_name (enum ada_exception_catchpoint_kind ex)
     }
 }
 
-/* Return the breakpoint ops "virtual table" used for catchpoints
-   of the EX kind.  */
-
-static const struct breakpoint_ops *
-ada_exception_breakpoint_ops (enum ada_exception_catchpoint_kind ex)
-{
-  switch (ex)
-    {
-      case ada_catch_exception:
-	return (&catch_exception_breakpoint_ops);
-	break;
-      case ada_catch_exception_unhandled:
-	return (&catch_exception_unhandled_breakpoint_ops);
-	break;
-      case ada_catch_assert:
-	return (&catch_assert_breakpoint_ops);
-	break;
-      case ada_catch_handlers:
-	return (&catch_handlers_breakpoint_ops);
-	break;
-      default:
-	internal_error (__FILE__, __LINE__,
-			_("unexpected catchpoint kind (%d)"), ex);
-    }
-}
-
 /* Return the condition that will be used to match the current exception
    being raised with the exception that the user wants to catch.  This
    assumes that this condition is used when the inferior just triggered
@@ -12285,7 +12253,7 @@ ada_exception_sal (enum ada_exception_catchpoint_kind ex,
   *addr_string = sym_name;
 
   /* Set OPS.  */
-  *ops = ada_exception_breakpoint_ops (ex);
+  *ops = &catch_exception_breakpoint_ops;
 
   return find_function_start_sal (sym, 1);
 }
@@ -13445,36 +13413,6 @@ initialize_ada_catchpoint_ops (void)
   ops->print_one = print_one_exception;
   ops->print_mention = print_mention_exception;
   ops->print_recreate = print_recreate_exception;
-
-  ops = &catch_exception_unhandled_breakpoint_ops;
-  *ops = bkpt_breakpoint_ops;
-  ops->allocate_location = allocate_location_exception;
-  ops->re_set = re_set_exception;
-  ops->check_status = check_status_exception;
-  ops->print_it = print_it_exception;
-  ops->print_one = print_one_exception;
-  ops->print_mention = print_mention_exception;
-  ops->print_recreate = print_recreate_exception;
-
-  ops = &catch_assert_breakpoint_ops;
-  *ops = bkpt_breakpoint_ops;
-  ops->allocate_location = allocate_location_exception;
-  ops->re_set = re_set_exception;
-  ops->check_status = check_status_exception;
-  ops->print_it = print_it_exception;
-  ops->print_one = print_one_exception;
-  ops->print_mention = print_mention_exception;
-  ops->print_recreate = print_recreate_exception;
-
-  ops = &catch_handlers_breakpoint_ops;
-  *ops = bkpt_breakpoint_ops;
-  ops->allocate_location = allocate_location_exception;
-  ops->re_set = re_set_exception;
-  ops->check_status = check_status_exception;
-  ops->print_it = print_it_exception;
-  ops->print_one = print_one_exception;
-  ops->print_mention = print_mention_exception;
-  ops->print_recreate = print_recreate_exception;
 }
 
 /* This module's 'new_objfile' observer.  */
-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/4] Minor breakpoint cleanups
  2022-01-13 16:29 [PATCH 0/4] Minor breakpoint cleanups Tom Tromey
                   ` (3 preceding siblings ...)
  2022-01-13 16:29 ` [PATCH 4/4] Simplify Ada catchpoints Tom Tromey
@ 2022-01-13 20:05 ` John Baldwin
  2022-01-18 17:32   ` Tom Tromey
  4 siblings, 1 reply; 7+ messages in thread
From: John Baldwin @ 2022-01-13 20:05 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 1/13/22 8:29 AM, Tom Tromey wrote:
> I was looking into making 'breakpoint' use virtual methods -- which
> turns out to be pretty involved -- but in the course of doing so I
> found a few smaller cleanups that seemed worth doing.
> 
> This series removes a bit of redundant code, and splits a bit of code
> out of breakpoint.c into two new files.
> 
> Regression tested on x86-64 Fedora 34.

Looks good to me.

-- 
John Baldwin

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/4] Minor breakpoint cleanups
  2022-01-13 20:05 ` [PATCH 0/4] Minor breakpoint cleanups John Baldwin
@ 2022-01-18 17:32   ` Tom Tromey
  0 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2022-01-18 17:32 UTC (permalink / raw)
  To: John Baldwin; +Cc: Tom Tromey, gdb-patches

John> Looks good to me.

Thanks.  I'm going to check this in.

Tom

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-01-18 17:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-13 16:29 [PATCH 0/4] Minor breakpoint cleanups Tom Tromey
2022-01-13 16:29 ` [PATCH 1/4] Unify "catch fork" and "catch vfork" Tom Tromey
2022-01-13 16:29 ` [PATCH 2/4] Move "catch fork" to a new file Tom Tromey
2022-01-13 16:29 ` [PATCH 3/4] Move "catch exec" " Tom Tromey
2022-01-13 16:29 ` [PATCH 4/4] Simplify Ada catchpoints Tom Tromey
2022-01-13 20:05 ` [PATCH 0/4] Minor breakpoint cleanups John Baldwin
2022-01-18 17:32   ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).