Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH 1/4] 'catch syscall' feature -- Architecture-independent  part
@ 2008-11-04  4:32 Sérgio Durigan Júnior
  2008-11-04 16:17 ` Pedro Alves
                   ` (2 more replies)
  0 siblings, 3 replies; 57+ messages in thread
From: Sérgio Durigan Júnior @ 2008-11-04  4:32 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 3719 bytes --]

This is the architecture independent part of the patch. The ChangeLog is
inlined, and the patch itself is attached.

Regards,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil

gdb/ChangeLog:

2008-11-04  Sergio Durigan Junior  <sergiodj@linux.vnet.ibm.com>

	* breakpoint.c (clear_syscall_catchpoints_info): New function.
	(print_it_typical): Handle the case of the breakpoint at the
	entrypoint.
	(bpstat_what): Add the entry breakpoint case.
	(print_one_breakpoint_location): Add the entry breakpoint.
	(allocate_bp_location): Likewise.
	(set_raw_breakpoint_without_location): New.  Used by the
	catch syscall feature because it needs to mention the
	catchpoint after some few adjustments.
	(create_entry_breakpoint): New.
	(insert_catch_syscall): New.
	(breakpoint_hit_catch_syscall): New.
	(print_it_catch_syscall): New.
	(print_one_catch_syscall): New.
	(print_mention_catch_syscall): New.
	(catch_syscall_breakpoint_ops): New struct.
	(syscall_catchpoint_p): New.
	(create_catchpoint): Modified in order to use
	create_catchpoint_without_mention.
	(create_syscall_event_catchpoint): New.
	(mention): Add the entry breakpoint.
	(catch_syscall_split_args): New.
	(catch_syscall_command_1): New.
	(delete_command): Add the entry breakpoint case.
	(breakpoint_re_set_one): Likewise.
	(is_syscall_catchpoint_enabled): New.
	(catch_syscall_enabled): New.
	(catching_syscall_number): New.
	* breakpoint.h (enum bptype): Add the entry breakpoint case.
	(struct breakpoint): Add field 'syscall_number' (used to know
	which syscall triggered the catchpoint) and
	'syscall_to_be_caught' (used to know which syscall we are trying
	to catch).
	(enum bpstat_what_main_action): Add BPSTAT_WHAT_ENTRY_BREAKPOINT,
	to identify whether we hit an entry breakpoint.
	(catch_syscall_enabled): New.
	(catching_syscall_number): New.
	(create_entry_breakpoint): New.
	* completer.c: Include gdbarch.h file.
	(catch_syscall_completer): New.
	* completer.h (catch_syscall_completer): New.
	* defs.h (gdb_datadir): New variable.
	* gdbarch.c: Regenerated.
	* gdbarch.h: Regenerated.
	* gdbarch.sh: Add syscall catchpoint functions.
	(get_syscall_number): New.
	(syscall_name_from_number): new.
	(syscall_number_from_name): New.
	(get_syscalls_names): New.
	* gdbthread.h (struct thread_info): Add field 'syscall_state',
	used to know if we are calling or returning from a syscall.
	* inf-child.c (inf_child_insert_syscall_catchpoint): New.
	(inf_child_remove_syscall_catchpoint): New.
	(inf_child_target): Assign default values to target_ops.
	* inf-ptrace.c (inf_ptrace_resume): Select the proper request
	to be made for ptrace() considering if we are catching syscalls
	or not.
	* infcmd.c (run_command_1): Clean syscall catchpoint info on every
	run.
	* infrun.c (resume): Add syscall catchpoint and entry breakpoint.
	(deal_with_syscall_event): New.
	(handle_inferior_event): Add syscall entry/return events.  Also,
	add entry breakpoint event.
	(inferior_has_called_syscall): New.
	* main.c: Add gdb_datadir variable to store the current GDB datadir.
	(captured_main): Add the GDB datadir relocatable handler.
	* maint.c: Create the "maintanence set gdb_datadir" c
	* target.c (update_current_target): Update/copy functions related to
	syscall catchpoint and entry breakpoint.
	(debug_to_wait): Add syscall catchpoint entry/return events.
	* target.h (struct target_waitstatus): Add syscall number.
	(struct target_ops): Add ops for syscall catchpoint and entry
	breakpoint.
	(inferior_has_called_syscall): New.
	(target_passed_by_entrypoint): New.
	(target_insert_syscall_catchpoint): New.
	(target_remove_syscall_catchpoint): New.
	(target_enable_tracesysgood): New.

[-- Attachment #2: catch-syscall-arch-indep.patch --]
[-- Type: text/x-patch, Size: 53122 bytes --]

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 912d7ea..f3bea56 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -192,6 +192,12 @@ static int is_hardware_watchpoint (struct breakpoint *bpt);
 
 static void insert_breakpoint_locations (void);
 
+static int syscall_catchpoint_p (struct breakpoint *b);
+
+/* The maximum number of arguments the user can provide to
+   the 'catch syscall' command.  */
+#define MAX_CATCH_SYSCALL_ARGS 10
+
 static const char *
 bpdisp_text (enum bpdisp disp)
 {
@@ -395,6 +401,18 @@ set_breakpoint_count (int num)
 		   value_from_longest (builtin_type_int32, (LONGEST) num));
 }
 
+/* Used in run_command to reset syscall catchpoints fields.  */
+
+void
+clear_syscall_catchpoints_info (void)
+{
+  struct breakpoint *b;
+
+  ALL_BREAKPOINTS (b)
+    if (syscall_catchpoint_p (b))
+      b->syscall_number = UNKNOWN_SYSCALL;
+}
+
 /* Used in run_command to zero the hit count when a new run starts. */
 
 void
@@ -2315,6 +2333,14 @@ print_it_typical (bpstat bs)
       return PRINT_NOTHING;
       break;
 
+    case bp_entry_breakpoint:
+       /* Not sure how we will get here. 
+	 GDB should not stop for these breakpoints.  */
+      internal_error (__FILE__, __LINE__,
+                      _("Entry Breakpoint: gdb should not stop!\n"));
+      return PRINT_NOTHING;
+      break;    
+
     case bp_overlay_event:
       /* By analogy with the thread event, GDB should not stop for these. */
       printf_filtered (_("Overlay Event Breakpoint: gdb should not stop!\n"));
@@ -3165,6 +3191,9 @@ bpstat_what (bpstat bs)
       /* We caught a shared library event.  */
       catch_shlib_event,
 
+      /* We are in a entry breakpoint.  */
+      entry_breakpoint,
+
       /* This is just used to count how many enums there are.  */
       class_last
     };
@@ -3181,6 +3210,7 @@ bpstat_what (bpstat bs)
 #define sr BPSTAT_WHAT_STEP_RESUME
 #define shl BPSTAT_WHAT_CHECK_SHLIBS
 #define shlr BPSTAT_WHAT_CHECK_SHLIBS_RESUME_FROM_HOOK
+#define entrybp BPSTAT_WHAT_ENTRY_BREAKPOINT
 
 /* "Can't happen."  Might want to print an error message.
    abort() is not out of the question, but chances are GDB is just
@@ -3225,30 +3255,33 @@ bpstat_what (bpstat bs)
     table[(int) class_last][(int) BPSTAT_WHAT_LAST] =
   {
   /*                              old action */
-  /*       kc    ss    sn    sgl    slr   clr   sr   shl   shlr
+  /*       kc    ss    sn    sgl    slr   clr   sr   shl   shlr   entrybp
    */
 /*no_effect */
-    {kc, ss, sn, sgl, slr, clr, sr, shl, shlr},
+    {kc, ss, sn, sgl, slr, clr, sr, shl, shlr, shlr},
 /*wp_silent */
-    {ss, ss, sn, ss, ss, ss, sr, shl, shlr},
+    {ss, ss, sn, ss, ss, ss, sr, shl, shlr, shlr},
 /*wp_noisy */
-    {sn, sn, sn, sn, sn, sn, sr, shl, shlr},
+    {sn, sn, sn, sn, sn, sn, sr, shl, shlr, shlr},
 /*bp_nostop */
-    {sgl, ss, sn, sgl, slr, slr, sr, shl, shlr},
+    {sgl, ss, sn, sgl, slr, slr, sr, shl, shlr, shlr},
 /*bp_silent */
-    {ss, ss, sn, ss, ss, ss, sr, shl, shlr},
+    {ss, ss, sn, ss, ss, ss, sr, shl, shlr, shlr},
 /*bp_noisy */
-    {sn, sn, sn, sn, sn, sn, sr, shl, shlr},
+    {sn, sn, sn, sn, sn, sn, sr, shl, shlr, shlr},
 /*long_jump */
-    {slr, ss, sn, slr, slr, err, sr, shl, shlr},
+    {slr, ss, sn, slr, slr, err, sr, shl, shlr, shlr},
 /*long_resume */
-    {clr, ss, sn, err, err, err, sr, shl, shlr},
+    {clr, ss, sn, err, err, err, sr, shl, shlr, shlr},
 /*step_resume */
-    {sr, sr, sr, sr, sr, sr, sr, sr, sr},
+    {sr, sr, sr, sr, sr, sr, sr, sr, sr, sr},
 /*shlib */
-    {shl, shl, shl, shl, shl, shl, sr, shl, shlr},
+    {shl, shl, shl, shl, shl, shl, sr, shl, shlr, shl},
 /*catch_shlib */
-    {shlr, shlr, shlr, shlr, shlr, shlr, sr, shlr, shlr}
+    {shlr, shlr, shlr, shlr, shlr, shlr, sr, shlr, shlr, shlr},
+/* entry_breakpoint */
+    {entrybp, entrybp, entrybp, entrybp, entrybp, entrybp, sr, entrybp,
+      entrybp, entrybp}
   };
 
 #undef kc
@@ -3262,6 +3295,7 @@ bpstat_what (bpstat bs)
 #undef ts
 #undef shl
 #undef shlr
+#undef entrybp
   enum bpstat_what_main_action current_action = BPSTAT_WHAT_KEEP_CHECKING;
   struct bpstat_what retval;
 
@@ -3365,6 +3399,12 @@ bpstat_what (bpstat bs)
 	  bs_class = bp_silent;
 	  retval.call_dummy = 1;
 	  break;
+        case bp_entry_breakpoint:
+          if (bs->stop)
+            bs_class = entry_breakpoint;
+          else
+            bs_class = no_effect;
+          break;
 	}
       current_action = table[(int) bs_class][(int) current_action];
     }
@@ -3526,7 +3566,8 @@ print_one_breakpoint_location (struct breakpoint *b,
     {bp_overlay_event, "overlay events"},
     {bp_catchpoint, "catchpoint"},
     {bp_catch_load, "catch load"},
-    {bp_catch_unload, "catch unload"}
+    {bp_catch_unload, "catch unload"},
+    {bp_entry_breakpoint, "entry breakpoint"}
   };
   
   static char bpenables[] = "nynny";
@@ -3675,6 +3716,7 @@ print_one_breakpoint_location (struct breakpoint *b,
       case bp_shlib_event:
       case bp_thread_event:
       case bp_overlay_event:
+      case bp_entry_breakpoint:
 	if (opts.addressprint)
 	  {
 	    annotate_field (4);
@@ -4248,6 +4290,7 @@ allocate_bp_location (struct breakpoint *bpt, enum bptype bp_type)
     case bp_watchpoint_scope:
     case bp_call_dummy:
     case bp_shlib_event:
+    case bp_entry_breakpoint:
     case bp_thread_event:
     case bp_overlay_event:
     case bp_catch_load:
@@ -4309,6 +4352,8 @@ set_raw_breakpoint_without_location (enum bptype bptype)
   b->triggered_dll_pathname = NULL;
   b->forked_inferior_pid = null_ptid;
   b->exec_pathname = NULL;
+  b->syscall_to_be_caught = CATCHING_ANY_SYSCALL;
+  b->syscall_number = UNKNOWN_SYSCALL;
   b->ops = NULL;
   b->condition_not_parsed = 0;
 
@@ -4531,6 +4576,31 @@ disable_overlay_breakpoints (void)
     }
 }
 
+int
+create_entry_breakpoint ()
+{
+  CORE_ADDR taddr, entry_addr;
+  struct breakpoint *b;
+
+  taddr = entry_point_address ();
+  /* Make certain that the address points at real code, and not a
+     function descriptor.  */
+  entry_addr = gdbarch_convert_from_func_ptr_addr (current_gdbarch, taddr,
+                                                   &current_target);
+
+  /* Setting the breakpoint.  */
+  b = create_internal_breakpoint (entry_addr, bp_entry_breakpoint);
+
+  b->enable_state = bp_enabled;
+  b->disposition = disp_del;
+  /* addr_string has to be used or breakpoint_re_set will delete me.  */
+  b->addr_string = xstrprintf ("AT_ENTRY (0x%s)", paddr (entry_addr));
+
+  update_global_location_list (1);
+
+  return 1;
+}
+
 struct breakpoint *
 create_thread_event_breakpoint (CORE_ADDR address)
 {
@@ -4828,7 +4898,155 @@ static struct breakpoint_ops catch_vfork_breakpoint_ops =
   print_mention_catch_vfork
 };
 
-/* Create a new breakpoint of the bp_catchpoint kind and return it.
+/* Implement the "insert" breakpoint_ops method for syscall
+   catchpoints.  */
+
+static void
+insert_catch_syscall (struct breakpoint *b)
+{
+  target_insert_syscall_catchpoint (PIDGET (inferior_ptid));
+}
+
+/* Implement the "remove" breakpoint_ops method for syscall
+   catchpoints.  */
+
+static int
+remove_catch_syscall (struct breakpoint *b)
+{
+  return target_remove_syscall_catchpoint (PIDGET (inferior_ptid));
+}
+
+/* Implement the "breakpoint_hit" breakpoint_ops method for syscall
+   catchpoints.  */
+
+static int
+breakpoint_hit_catch_syscall (struct breakpoint *b)
+{
+  /* We must check if we are catching specific syscalls in this breakpoint.
+     If we are, then we must guarantee that the called syscall is the same
+     syscall we are catching.  */
+  int syscall_number = 0;
+
+  if (!inferior_has_called_syscall (inferior_ptid, &syscall_number))
+    return 0;
+
+  /* Now, checking if the syscall is the same.  */
+  if (b->syscall_to_be_caught != CATCHING_ANY_SYSCALL
+      && b->syscall_to_be_caught != syscall_number)
+    /* Not the same.  */
+    return 0;
+
+  /* It's the same syscall.  We can update the breakpoint struct
+     with the correct information.  */
+  b->syscall_number = syscall_number;
+
+  return 1;
+}
+
+/* Implement the "print_it" breakpoint_ops method for syscall
+   catchpoints.  */
+
+static enum print_stop_action
+print_it_catch_syscall (struct breakpoint *b)
+{
+  /* This is needed because we want to know in which state a
+     syscall is.  It can be in the TARGET_WAITKIND_SYSCALL_ENTRY
+     or TARGET_WAITKIND_SYSCALL_RETURN, and depending on it we
+     must print "called syscall" or "returned from syscall".  */
+  struct thread_info *th_info = find_thread_pid (inferior_ptid);
+  const char *syscall_name =
+    gdbarch_syscall_name_from_number (current_gdbarch, b->syscall_number);
+
+  annotate_catchpoint (b->number);
+  printf_filtered (_("\nCatchpoint %d ("), b->number);
+
+  if (th_info->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY)
+    printf_filtered (_("calling "));
+  else
+    printf_filtered (_("returned from "));
+
+  printf_filtered (_("syscall "));
+
+  if (syscall_name == NULL)
+    printf_filtered (_("%d"), b->syscall_number);
+  else
+    printf_filtered (_("'%s ()'"), syscall_name);
+
+  printf_filtered (_("), "));
+
+  return PRINT_SRC_AND_LOC;
+}
+
+/* Implement the "print_one" breakpoint_ops method for syscall
+   catchpoints.  */
+
+static void
+print_one_catch_syscall (struct breakpoint *b, CORE_ADDR *last_addr)
+{
+  struct value_print_options opts;
+  const char *syscall_name =
+    gdbarch_syscall_name_from_number (current_gdbarch, b->syscall_number);
+
+  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)
+    ui_out_field_skip (uiout, "addr");
+  annotate_field (5);
+  ui_out_text (uiout, "syscall \"");
+  if (b->syscall_number != UNKNOWN_SYSCALL)
+    {
+      if (syscall_name != NULL)
+        ui_out_field_string (uiout, "what", syscall_name);
+      else
+        ui_out_field_int (uiout, "what", b->syscall_number);
+    }
+  else
+    ui_out_field_string (uiout, "what", "<unknown syscall>");
+  ui_out_text (uiout, "\" ");
+}
+
+/* Implement the "print_mention" breakpoint_ops method for syscall
+   catchpoints.  */
+
+static void
+print_mention_catch_syscall (struct breakpoint *b)
+{
+  if (b->syscall_to_be_caught != CATCHING_ANY_SYSCALL)
+    printf_filtered (_("Catchpoint %d (syscall '%s ()')"),
+                     b->number,
+                     gdbarch_syscall_name_from_number (current_gdbarch,
+                                                   b->syscall_to_be_caught));
+  else
+    printf_filtered (_("Catchpoint %d (syscall)"),
+                     b->number);
+}
+
+/* The breakpoint_ops structure to be used in syscall catchpoints.  */
+
+static struct breakpoint_ops catch_syscall_breakpoint_ops =
+{
+  insert_catch_syscall,
+  remove_catch_syscall,
+  breakpoint_hit_catch_syscall,
+  print_it_catch_syscall,
+  print_one_catch_syscall,
+  print_mention_catch_syscall
+};
+
+/* Returns non-zero if 'b' is a syscall catchpoint.  */
+
+static int
+syscall_catchpoint_p (struct breakpoint *b)
+{
+  return (b->ops == &catch_syscall_breakpoint_ops);
+}
+
+/* Create a new breakpoint of the bp_catchpoint kind and return it,
+   but does NOT mention it nor update the global location list.
+   This is useful if you need to fill more fields in the
+   struct breakpoint before calling mention.
  
    If TEMPFLAG is non-zero, then make the breakpoint temporary.
    If COND_STRING is not NULL, then store it in the breakpoint.
@@ -4836,16 +5054,13 @@ static struct breakpoint_ops catch_vfork_breakpoint_ops =
    to the catchpoint.  */
 
 static struct breakpoint *
-create_catchpoint (int tempflag, char *cond_string,
-                   struct breakpoint_ops *ops)
+create_catchpoint_without_mention (int tempflag, char *cond_string,
+                                   struct breakpoint_ops *ops)
 {
   struct symtab_and_line sal;
   struct breakpoint *b;
 
   init_sal (&sal);
-  sal.pc = 0;
-  sal.symtab = NULL;
-  sal.line = 0;
 
   b = set_raw_breakpoint (sal, bp_catchpoint);
   set_breakpoint_count (breakpoint_count + 1);
@@ -4859,6 +5074,23 @@ create_catchpoint (int tempflag, char *cond_string,
   b->disposition = tempflag ? disp_del : disp_donttouch;
   b->ops = ops;
 
+  return b;
+}
+
+/* Create a new breakpoint of the bp_catchpoint kind and return it.
+ 
+   If TEMPFLAG is non-zero, then make the breakpoint temporary.
+   If COND_STRING is not NULL, then store it in the breakpoint.
+   OPS, if not NULL, is the breakpoint_ops structure associated
+   to the catchpoint.  */
+
+static struct breakpoint *
+create_catchpoint (int tempflag, char *cond_string,
+                   struct breakpoint_ops *ops)
+{
+  struct breakpoint *b =
+    create_catchpoint_without_mention (tempflag, cond_string, ops);
+
   mention (b);
   update_global_location_list (1);
 
@@ -4943,6 +5175,23 @@ static struct breakpoint_ops catch_exec_breakpoint_ops =
   print_mention_catch_exec
 };
 
+static void
+create_syscall_event_catchpoint (int tempflag, int syscall_number,
+                                 struct breakpoint_ops *ops)
+{
+  struct breakpoint *b =
+    create_catchpoint_without_mention (tempflag, NULL, ops);
+
+  b->syscall_to_be_caught = syscall_number;
+  /* We still don't know the syscall that will be caught :-).  */
+  b->syscall_number = UNKNOWN_SYSCALL;
+
+  /* Now, we have to mention the breakpoint and update the global
+     location list.  */
+  mention (b);
+  update_global_location_list (1);
+}
+
 static int
 hw_breakpoint_used_count (void)
 {
@@ -5164,6 +5413,7 @@ mention (struct breakpoint *b)
       case bp_shlib_event:
       case bp_thread_event:
       case bp_overlay_event:
+      case bp_entry_breakpoint:
 	break;
       }
 
@@ -6921,6 +7171,107 @@ catch_ada_exception_command (char *arg, int from_tty,
                                    from_tty);
 }
 
+/* Splits the argument using space as delimiter.
+   Returns the number of args.  */
+static int
+catch_syscall_split_args (char *arg, int *syscalls_numbers)
+{
+  int nargs = 0, i;
+  char *arg_p = arg;
+  char cur_name[128];
+  char c;
+
+  while (*arg_p != '\0')
+    {
+      int out = 0;
+      int syscall_number;
+      memset ((void *) cur_name, '\0', 128 * sizeof (char));
+
+      /* Skipping white-spaces.  */
+      while (isspace (*arg_p))
+        arg_p++;
+
+      for (i = 0; out == 0; i++)
+        {
+          c = *arg_p;
+          cur_name[i] = c;
+          if (isspace (c) || c == '\0')
+            {
+              out = 1;
+              cur_name[i] = '\0';
+            }
+          else
+            arg_p++;
+        }
+
+      /* Checking if the user provided a syscall name or a number.  */
+      if (isdigit (cur_name[0]))
+        {
+          /* We have a number.  Let's check if it's valid.  */
+          syscall_number = atoi (cur_name);
+
+          if (gdbarch_syscall_name_from_number (current_gdbarch,
+                                                syscall_number) == NULL)
+            error (_("The number '%d' does not represent a valid syscall."),
+                   syscall_number);
+        }
+      else
+        {
+          /* We have a name.  Let's check if it's valid and convert it
+             to a number.  */
+          syscall_number =
+            gdbarch_syscall_number_from_name (current_gdbarch, cur_name);
+
+          if (syscall_number == UNKNOWN_SYSCALL)
+            error (_("Invalid syscall name '%s'."), cur_name);
+        }
+      /* Ok, it's valid.  */
+      syscalls_numbers[nargs++] = syscall_number;
+    }
+
+    return nargs;
+}
+
+/* Implement the "catch syscall" command.  */
+
+static void
+catch_syscall_command_1 (char *arg, int from_tty, struct cmd_list_element *command)
+{
+  int tempflag;
+
+  /* Checking if the feature if supported.  */
+  if (gdbarch_get_syscall_number_p (current_gdbarch) == 0)
+    error (_("The feature 'catch syscall' is not supported on \
+this architeture yet."));
+
+  tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
+
+  ep_skip_leading_whitespace (&arg);
+
+  /* The allowed syntax is:
+     catch syscall
+     catch syscall <name | number> [<name | number> ... <name | number>]
+
+     The maximum number of arguments this command can take
+     is define by MAX_CATCH_SYSCALL_ARGS.
+
+     Let's check if there's a syscall name.  */
+
+  if (arg != NULL)
+    {
+      int syscalls_numbers[MAX_CATCH_SYSCALL_ARGS];
+      int nargs = catch_syscall_split_args (arg, syscalls_numbers);
+      int i;
+
+      for (i = 0; i < nargs; i++)
+        create_syscall_event_catchpoint (tempflag, syscalls_numbers[i],
+                                         &catch_syscall_breakpoint_ops);
+    }
+  else
+    create_syscall_event_catchpoint (tempflag, CATCHING_ANY_SYSCALL,
+                                     &catch_syscall_breakpoint_ops);
+}
+
 /* Implement the "catch assert" command.  */
 
 static void
@@ -7441,6 +7792,7 @@ delete_command (char *arg, int from_tty)
 	    b->type != bp_shlib_event &&
 	    b->type != bp_thread_event &&
 	    b->type != bp_overlay_event &&
+            b->type != bp_entry_breakpoint &&
 	    b->number >= 0)
 	  {
 	    breaks_to_delete = 1;
@@ -7458,6 +7810,7 @@ delete_command (char *arg, int from_tty)
 		b->type != bp_shlib_event &&
 		b->type != bp_thread_event &&
 		b->type != bp_overlay_event &&
+                b->type != bp_entry_breakpoint &&
 		b->number >= 0)
 	      delete_breakpoint (b);
 	  }
@@ -7766,6 +8119,9 @@ breakpoint_re_set_one (void *bint)
 	 Once it is set up, we do not want to touch it.  */
     case bp_thread_event:
 
+      /* Same for this one */
+    case bp_entry_breakpoint:
+
       /* Keep temporary breakpoints, which can be encountered when we step
          over a dlopen call and SOLIB_ADD is resetting the breakpoints.
          Otherwise these should have been blown away via the cleanup chain
@@ -8329,6 +8685,45 @@ single_step_breakpoint_inserted_here_p (CORE_ADDR pc)
   return 0;
 }
 
+/* Returns 0 if 'bp' is NOT a syscall catchpoint,
+   non-zero otherwise.  */
+static int
+is_syscall_catchpoint_enabled (struct breakpoint *bp)
+{
+  if (syscall_catchpoint_p (bp)
+      && bp->enable_state != bp_disabled
+      && bp->enable_state != bp_call_disabled)
+    return 1;
+  else
+    return 0;
+}
+
+int
+catch_syscall_enabled (void)
+{
+  struct breakpoint *bp;
+
+  ALL_BREAKPOINTS (bp)
+    if (is_syscall_catchpoint_enabled (bp))
+      return 1;
+
+  return 0;
+}
+
+int
+catching_syscall_number (int syscall_number)
+{
+  struct breakpoint *bp;
+
+  ALL_BREAKPOINTS (bp)
+    if (is_syscall_catchpoint_enabled (bp))
+      if (bp->syscall_to_be_caught == syscall_number
+          || bp->syscall_to_be_caught == CATCHING_ANY_SYSCALL)
+        return 1;
+
+  return 0;
+}
+
 \f
 /* This help string is used for the break, hbreak, tbreak and thbreak commands.
    It is defined as a macro to prevent duplication.
@@ -8361,6 +8756,7 @@ static void
 add_catch_command (char *name, char *docstring,
 		   void (*sfunc) (char *args, int from_tty,
 				  struct cmd_list_element *command),
+                   char ** (*completion_function) (char *text, char *word),
 		   void *user_data_catch,
 		   void *user_data_tcatch)
 {
@@ -8370,11 +8766,13 @@ add_catch_command (char *name, char *docstring,
 		     &catch_cmdlist);
   set_cmd_sfunc (command, sfunc);
   set_cmd_context (command, user_data_catch);
+  set_cmd_completer (command, completion_function);
 
   command = add_cmd (name, class_breakpoint, NULL, docstring,
 		     &tcatch_cmdlist);
   set_cmd_sfunc (command, sfunc);
   set_cmd_context (command, user_data_tcatch);
+  set_cmd_completer (command, completion_function);
 }
 
 void
@@ -8649,48 +9047,64 @@ Set temporary catchpoints to catch events."),
 Catch an exception, when caught.\n\
 With an argument, catch only exceptions with the given name."),
 		     catch_catch_command,
+                     NULL,
 		     CATCH_PERMANENT,
 		     CATCH_TEMPORARY);
   add_catch_command ("throw", _("\
 Catch an exception, when thrown.\n\
 With an argument, catch only exceptions with the given name."),
 		     catch_throw_command,
+                     NULL,
 		     CATCH_PERMANENT,
 		     CATCH_TEMPORARY);
   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,
 		     CATCH_PERMANENT,
 		     CATCH_TEMPORARY);
+  add_catch_command ("syscall", _("\
+Catch calls to syscalls.\n\
+With an argument, catch only calls of that syscall."),
+                         catch_syscall_command_1,
+                         catch_syscall_completer,
+                         CATCH_PERMANENT,
+                         CATCH_TEMPORARY);
   add_catch_command ("load", _("\
 Catch library loads.\n\
 With an argument, catch only loads of that library."),
 		     catch_load_command_1,
+                     NULL,
 		     CATCH_PERMANENT,
 		     CATCH_TEMPORARY);
   add_catch_command ("unload", _("\
 Catch library unloads.\n\
 With an argument, catch only unloads of that library."),
 		     catch_unload_command_1,
+                     NULL,
 		     CATCH_PERMANENT,
 		     CATCH_TEMPORARY);
   add_catch_command ("exception", _("\
 Catch Ada exceptions, when raised.\n\
 With an argument, catch only exceptions with the given name."),
 		     catch_ada_exception_command,
+                     NULL,
 		     CATCH_PERMANENT,
 		     CATCH_TEMPORARY);
   add_catch_command ("assert", _("\
 Catch failed Ada assertions, when raised.\n\
 With an argument, catch only exceptions with the given name."),
 		     catch_assert_command,
+                     NULL,
 		     CATCH_PERMANENT,
 		     CATCH_TEMPORARY);
 
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index a77b1b4..caecba9 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -33,7 +33,12 @@ struct block;
 
 #define	BREAKPOINT_MAX	16
 \f
-/* Type of breakpoint. */
+
+/* A number to represent wether we are catching any syscalls.  */
+
+#define CATCHING_ANY_SYSCALL (-1)
+
+/* Type of breakpoint.  */
 /* FIXME In the future, we should fold all other breakpoint-like things into
    here.  This includes:
 
@@ -119,6 +124,10 @@ enum bptype
     /* These breakpoints are used to implement the "catch unload" command
        on platforms whose dynamic linkers support such functionality.  */
     bp_catch_unload,
+
+    /* This type is used to signal an internal breakpoint located at
+       the AT_ENTRY address.  */
+    bp_entry_breakpoint,
   };
 
 /* States of enablement of breakpoint. */
@@ -460,6 +469,21 @@ struct breakpoint
        triggered.  */
     char *exec_pathname;
 
+    /* Syscall number used for the 'catch syscall' feature.
+       If no syscall has been called, its value is UNKNOWN_SYSCALL.
+       Otherwise, it holds the system call number in the target.
+
+       This field is only valid immediately after this catchpoint has
+       triggered.  */
+    int syscall_number;
+
+    /* This field is used when we are "filtering" the syscalls
+       (i.e., when the user types "catch syscall <SYSCALL_NAME>".
+
+       It stores the syscall number in case we are in the "filter mode",
+       or CATCHING_ANY_SYSCALL otherwise.  */
+    int syscall_to_be_caught;
+
     /* Methods associated with this breakpoint.  */
     struct breakpoint_ops *ops;
 
@@ -541,6 +565,10 @@ enum bpstat_what_main_action
        resume out of the dynamic linker's callback, stop and print.  */
     BPSTAT_WHAT_CHECK_SHLIBS_RESUME_FROM_HOOK,
 
+    /* This internal breakpoint is used syscall catchpoints only after the
+       shell and the dynamic linker have already ran.  */
+    BPSTAT_WHAT_ENTRY_BREAKPOINT,
+
     /* This is just used to keep track of how many enums there are.  */
     BPSTAT_WHAT_LAST
   };
@@ -810,6 +838,8 @@ extern void enable_watchpoints_after_interactive_call_stop (void);
 extern enum command_control_type commands_from_control_command
   (char *arg, struct command_line *cmd);
 
+extern void clear_syscall_catchpoints_info (void);
+
 extern void clear_breakpoint_hit_counts (void);
 
 extern int get_number (char **);
@@ -889,6 +919,27 @@ extern int breakpoints_always_inserted_mode (void);
    in our opinion won't ever trigger.  */
 extern void breakpoint_retire_moribund (void);
 
+/* Checks if we are catching syscalls or not.
+   Returns 0 if not, greater than 0 if we are.  */
+extern int catch_syscall_enabled (void);
+
+/* Checks if we are catching syscalls with the specific
+   syscall_number.  Used for "filtering" the catchpoints.
+   Returns 0 if not, greater than 0 if we are.  */
+extern int catching_syscall_number (int syscall_number);
+
+/* Function used to set an internal breakpoint at the AT_ENTRY
+   (a.k.a. the entry point of the inferior).
+
+   This is currently needed for us to know when to start setting
+   up catchpoints for syscalls in the inferior.  If we don't do that,
+   then we would set a "catch syscall" too early, which would
+   catch syscalls from ld.so and/or libc (and we don't want that).
+
+   Returns zero if there was an error setting this breakpoint,
+   or 1 if everything went OK.  */
+extern int create_entry_breakpoint (void);
+
 /* Tell a breakpoint to be quiet.  */
 extern void make_breakpoint_silent (struct breakpoint *);
 
diff --git a/gdb/completer.c b/gdb/completer.c
index e7ee817..84a93a8 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -22,6 +22,7 @@
 #include "expression.h"
 #include "filenames.h"		/* For DOSish file names.  */
 #include "language.h"
+#include "gdbarch.h" /* For current_gdbarch.  */
 
 #include "cli/cli-decode.h"
 
@@ -712,6 +713,15 @@ command_completer (char *text, char *word)
   return complete_line_internal (word, text, strlen (text), 1);
 }
 
+/* Complete syscalls names.  Used by "catch syscall".  */
+char **
+catch_syscall_completer (char *text, char *word)
+{
+  const char **list =
+    gdbarch_get_syscalls_names (current_gdbarch);
+  return (list == NULL) ? NULL : complete_on_enum (list, text, word);
+}
+
 /* Generate completions one by one for the completer.  Each time we are
    called return another potential completion to the caller.
    line_completion just completes on commands or passes the buck to the
diff --git a/gdb/completer.h b/gdb/completer.h
index 318594e..2e10136 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -31,6 +31,8 @@ extern char **location_completer (char *, char *);
 
 extern char **command_completer (char *, char *);
 
+extern char **catch_syscall_completer (char *text, char *word);
+
 extern char *get_gdb_completer_quote_characters (void);
 
 /* Exported to linespec.c */
diff --git a/gdb/defs.h b/gdb/defs.h
index 8d50f8a..9284a72 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -151,6 +151,9 @@ extern int dbx_commands;
 /* System root path, used to find libraries etc.  */
 extern char *gdb_sysroot;
 
+/* GDB datadir, used to store data files.  */
+extern char *gdb_datadir;
+
 /* Search path for separate debug files.  */
 extern char *debug_file_directory;
 
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index dd6ad7f..3ffcaf7 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -240,6 +240,10 @@ struct gdbarch
   gdbarch_target_signal_from_host_ftype *target_signal_from_host;
   gdbarch_target_signal_to_host_ftype *target_signal_to_host;
   gdbarch_record_special_symbol_ftype *record_special_symbol;
+  gdbarch_get_syscall_number_ftype *get_syscall_number;
+  gdbarch_syscall_name_from_number_ftype *syscall_name_from_number;
+  gdbarch_syscall_number_from_name_ftype *syscall_number_from_name;
+  gdbarch_get_syscalls_names_ftype *get_syscalls_names;
   int has_global_solist;
 };
 
@@ -372,6 +376,10 @@ struct gdbarch startup_gdbarch =
   default_target_signal_from_host,  /* target_signal_from_host */
   default_target_signal_to_host,  /* target_signal_to_host */
   0,  /* record_special_symbol */
+  0,  /* get_syscall_number */
+  0,  /* syscall_name_from_number */
+  0,  /* syscall_number_from_name */
+  0,  /* get_syscalls_names */
   0,  /* has_global_solist */
   /* startup_gdbarch() */
 };
@@ -625,6 +633,10 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of target_signal_from_host, invalid_p == 0 */
   /* Skip verify of target_signal_to_host, invalid_p == 0 */
   /* Skip verify of record_special_symbol, has predicate */
+  /* Skip verify of get_syscall_number, has predicate */
+  /* Skip verify of syscall_name_from_number, has predicate */
+  /* Skip verify of syscall_number_from_name, has predicate */
+  /* Skip verify of get_syscalls_names, has predicate */
   /* Skip verify of has_global_solist, invalid_p == 0 */
   buf = ui_file_xstrdup (log, &dummy);
   make_cleanup (xfree, buf);
@@ -835,6 +847,18 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: get_longjmp_target = <0x%lx>\n",
                       (long) gdbarch->get_longjmp_target);
   fprintf_unfiltered (file,
+                      "gdbarch_dump: gdbarch_get_syscall_number_p() = %d\n",
+                      gdbarch_get_syscall_number_p (gdbarch));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: get_syscall_number = <0x%lx>\n",
+                      (long) gdbarch->get_syscall_number);
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: gdbarch_get_syscalls_names_p() = %d\n",
+                      gdbarch_get_syscalls_names_p (gdbarch));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: get_syscalls_names = <0x%lx>\n",
+                      (long) gdbarch->get_syscalls_names);
+  fprintf_unfiltered (file,
                       "gdbarch_dump: has_global_solist = %s\n",
                       plongest (gdbarch->has_global_solist));
   fprintf_unfiltered (file,
@@ -1057,6 +1081,18 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: static_transform_name = <0x%lx>\n",
                       (long) gdbarch->static_transform_name);
   fprintf_unfiltered (file,
+                      "gdbarch_dump: gdbarch_syscall_name_from_number_p() = %d\n",
+                      gdbarch_syscall_name_from_number_p (gdbarch));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: syscall_name_from_number = <0x%lx>\n",
+                      (long) gdbarch->syscall_name_from_number);
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: gdbarch_syscall_number_from_name_p() = %d\n",
+                      gdbarch_syscall_number_from_name_p (gdbarch));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: syscall_number_from_name = <0x%lx>\n",
+                      (long) gdbarch->syscall_number_from_name);
+  fprintf_unfiltered (file,
                       "gdbarch_dump: target_desc = %s\n",
                       plongest ((long) gdbarch->target_desc));
   fprintf_unfiltered (file,
@@ -3244,6 +3280,102 @@ set_gdbarch_record_special_symbol (struct gdbarch *gdbarch,
 }
 
 int
+gdbarch_get_syscall_number_p (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  return gdbarch->get_syscall_number != NULL;
+}
+
+LONGEST
+gdbarch_get_syscall_number (struct gdbarch *gdbarch, ptid_t ptid)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->get_syscall_number != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_get_syscall_number called\n");
+  return gdbarch->get_syscall_number (gdbarch, ptid);
+}
+
+void
+set_gdbarch_get_syscall_number (struct gdbarch *gdbarch,
+                                gdbarch_get_syscall_number_ftype get_syscall_number)
+{
+  gdbarch->get_syscall_number = get_syscall_number;
+}
+
+int
+gdbarch_syscall_name_from_number_p (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  return gdbarch->syscall_name_from_number != NULL;
+}
+
+const char *
+gdbarch_syscall_name_from_number (struct gdbarch *gdbarch, int syscall_number)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->syscall_name_from_number != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_syscall_name_from_number called\n");
+  return gdbarch->syscall_name_from_number (gdbarch, syscall_number);
+}
+
+void
+set_gdbarch_syscall_name_from_number (struct gdbarch *gdbarch,
+                                      gdbarch_syscall_name_from_number_ftype syscall_name_from_number)
+{
+  gdbarch->syscall_name_from_number = syscall_name_from_number;
+}
+
+int
+gdbarch_syscall_number_from_name_p (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  return gdbarch->syscall_number_from_name != NULL;
+}
+
+int
+gdbarch_syscall_number_from_name (struct gdbarch *gdbarch, const char *syscall_name)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->syscall_number_from_name != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_syscall_number_from_name called\n");
+  return gdbarch->syscall_number_from_name (gdbarch, syscall_name);
+}
+
+void
+set_gdbarch_syscall_number_from_name (struct gdbarch *gdbarch,
+                                      gdbarch_syscall_number_from_name_ftype syscall_number_from_name)
+{
+  gdbarch->syscall_number_from_name = syscall_number_from_name;
+}
+
+int
+gdbarch_get_syscalls_names_p (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  return gdbarch->get_syscalls_names != NULL;
+}
+
+const char **
+gdbarch_get_syscalls_names (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->get_syscalls_names != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_get_syscalls_names called\n");
+  return gdbarch->get_syscalls_names (gdbarch);
+}
+
+void
+set_gdbarch_get_syscalls_names (struct gdbarch *gdbarch,
+                                gdbarch_get_syscalls_names_ftype get_syscalls_names)
+{
+  gdbarch->get_syscalls_names = get_syscalls_names;
+}
+
+int
 gdbarch_has_global_solist (struct gdbarch *gdbarch)
 {
   gdb_assert (gdbarch != NULL);
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 35f8a36..9cb6e84 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -811,6 +811,42 @@ typedef void (gdbarch_record_special_symbol_ftype) (struct gdbarch *gdbarch, str
 extern void gdbarch_record_special_symbol (struct gdbarch *gdbarch, struct objfile *objfile, asymbol *sym);
 extern void set_gdbarch_record_special_symbol (struct gdbarch *gdbarch, gdbarch_record_special_symbol_ftype *record_special_symbol);
 
+/* Functions for the 'catch syscall' feature.
+   Get architecture-specific system calls information from registers. */
+
+extern int gdbarch_get_syscall_number_p (struct gdbarch *gdbarch);
+
+typedef LONGEST (gdbarch_get_syscall_number_ftype) (struct gdbarch *gdbarch, ptid_t ptid);
+extern LONGEST gdbarch_get_syscall_number (struct gdbarch *gdbarch, ptid_t ptid);
+extern void set_gdbarch_get_syscall_number (struct gdbarch *gdbarch, gdbarch_get_syscall_number_ftype *get_syscall_number);
+
+/* Translate a syscall number to its corresponding name. */
+
+extern int gdbarch_syscall_name_from_number_p (struct gdbarch *gdbarch);
+
+typedef const char * (gdbarch_syscall_name_from_number_ftype) (struct gdbarch *gdbarch, int syscall_number);
+extern const char * gdbarch_syscall_name_from_number (struct gdbarch *gdbarch, int syscall_number);
+extern void set_gdbarch_syscall_name_from_number (struct gdbarch *gdbarch, gdbarch_syscall_name_from_number_ftype *syscall_name_from_number);
+
+/* Translate a syscall name to its corresponding number.
+  
+   This function must return the syscall number if found, or
+   UNKNOWN_SYSCALL if not found. */
+
+extern int gdbarch_syscall_number_from_name_p (struct gdbarch *gdbarch);
+
+typedef int (gdbarch_syscall_number_from_name_ftype) (struct gdbarch *gdbarch, const char *syscall_name);
+extern int gdbarch_syscall_number_from_name (struct gdbarch *gdbarch, const char *syscall_name);
+extern void set_gdbarch_syscall_number_from_name (struct gdbarch *gdbarch, gdbarch_syscall_number_from_name_ftype *syscall_number_from_name);
+
+/* Returns the array containing the syscalls names for the architecture. */
+
+extern int gdbarch_get_syscalls_names_p (struct gdbarch *gdbarch);
+
+typedef const char ** (gdbarch_get_syscalls_names_ftype) (struct gdbarch *gdbarch);
+extern const char ** gdbarch_get_syscalls_names (struct gdbarch *gdbarch);
+extern void set_gdbarch_get_syscalls_names (struct gdbarch *gdbarch, gdbarch_get_syscalls_names_ftype *get_syscalls_names);
+
 /* True if the list of shared libraries is one and only for all
    processes, as opposed to a list of shared libraries per inferior.
    When this property is true, GDB assumes that since shared libraries
@@ -820,6 +856,9 @@ extern void set_gdbarch_record_special_symbol (struct gdbarch *gdbarch, gdbarch_
 extern int gdbarch_has_global_solist (struct gdbarch *gdbarch);
 extern void set_gdbarch_has_global_solist (struct gdbarch *gdbarch, int has_global_solist);
 
+/* Definition for an unknown syscall, used basically in error-cases. */
+#define UNKNOWN_SYSCALL (-1)
+
 extern struct gdbarch_tdep *gdbarch_tdep (struct gdbarch *gdbarch);
 
 
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 79ca862..2e65481 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -708,6 +708,23 @@ m:int:target_signal_to_host:enum target_signal ts:ts::default_target_signal_to_h
 # Record architecture-specific information from the symbol table.
 M:void:record_special_symbol:struct objfile *objfile, asymbol *sym:objfile, sym
 
+# Functions for the 'catch syscall' feature.
+
+# Get architecture-specific system calls information from registers.
+M:LONGEST:get_syscall_number:ptid_t ptid:ptid
+
+# Translate a syscall number to its corresponding name.
+M:const char *:syscall_name_from_number:int syscall_number:syscall_number
+
+# Translate a syscall name to its corresponding number.
+#
+# This function must return the syscall number if found, or
+# UNKNOWN_SYSCALL if not found.
+M:int:syscall_number_from_name:const char *syscall_name:syscall_name
+
+# Returns the array containing the syscalls names for the architecture.
+M:const char **:get_syscalls_names:void:
+
 # True if the list of shared libraries is one and only for all
 # processes, as opposed to a list of shared libraries per inferior.
 # When this property is true, GDB assumes that since shared libraries
@@ -895,6 +912,9 @@ done
 # close it off
 cat <<EOF
 
+/* Definition for an unknown syscall, used basically in error-cases. */
+#define UNKNOWN_SYSCALL (-1)
+
 extern struct gdbarch_tdep *gdbarch_tdep (struct gdbarch *gdbarch);
 
 
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 55c848d..5d89ced 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -173,6 +173,13 @@ struct thread_info
 
   /* Private data used by the target vector implementation.  */
   struct private_thread_info *private;
+
+  /* Signal wether we are in a SYSCALL_ENTRY or
+     in a SYSCALL_RETURN event.
+     Values:
+     - TARGET_WAITKIND_SYSCALL_ENTRY
+     - TARGET_WAITKIND_SYSCALL_RETURN */
+  int syscall_state;
 };
 
 /* Create an empty thread list, or empty the existing one.  */
diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 8da25f4..058cb34 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -144,6 +144,21 @@ inf_child_remove_exec_catchpoint (int pid)
   return 0;
 }
 
+static void
+inf_child_insert_syscall_catchpoint (int pid)
+{
+  /* This version of Unix doesn't support notification of syscall
+     events.  */
+}
+
+static int
+inf_child_remove_syscall_catchpoint (int pid)
+{
+  /* This version of Unix doesn't support notification of syscall
+     events.  */
+  return 0;
+}
+
 static int
 inf_child_can_run (void)
 {
@@ -187,6 +202,8 @@ inf_child_target (void)
   t->to_follow_fork = inf_child_follow_fork;
   t->to_insert_exec_catchpoint = inf_child_insert_exec_catchpoint;
   t->to_remove_exec_catchpoint = inf_child_remove_exec_catchpoint;
+  t->to_insert_syscall_catchpoint = inf_child_insert_syscall_catchpoint;
+  t->to_remove_syscall_catchpoint = inf_child_remove_syscall_catchpoint;
   t->to_can_run = inf_child_can_run;
   t->to_pid_to_exec_file = inf_child_pid_to_exec_file;
   t->to_stratum = process_stratum;
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 57af79a..65e7acd 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -355,13 +355,19 @@ static void
 inf_ptrace_resume (ptid_t ptid, int step, enum target_signal signal)
 {
   pid_t pid = ptid_get_pid (ptid);
-  int request = PT_CONTINUE;
+  int request;
 
   if (pid == -1)
     /* Resume all threads.  Traditionally ptrace() only supports
        single-threaded processes, so simply resume the inferior.  */
     pid = ptid_get_pid (inferior_ptid);
 
+  if (target_passed_by_entrypoint () > 0
+      && catch_syscall_enabled () > 0)
+    request = PT_SYSCALL;
+  else
+    request = PT_CONTINUE;
+
   if (step)
     {
       /* If this system does not support PT_STEP, a higher level
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 50e8fff..744d32c 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -466,6 +466,11 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
   init_wait_for_inferior ();
   clear_breakpoint_hit_counts ();
 
+  /* If we already caught a syscall catchpoint, then reset its
+     syscall_number information because we are starting all over
+     again.  */
+  clear_syscall_catchpoints_info ();
+
   /* Clean up any leftovers from other runs.  Some other things from
      this function should probably be moved into target_pre_inferior.  */
   target_pre_inferior (from_tty);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 30d914d..843155c 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1042,7 +1042,7 @@ a command like `return' or `jump' to continue execution."));
         }
     }
 
-  /* If there were any forks/vforks/execs that were caught and are
+  /* If there were any forks/vforks/execs/syscalls that were caught and are
      now to be followed, then do so.  */
   switch (pending_follow.kind)
     {
@@ -1058,6 +1058,11 @@ a command like `return' or `jump' to continue execution."));
       pending_follow.kind = TARGET_WAITKIND_SPURIOUS;
       break;
 
+    case TARGET_WAITKIND_SYSCALL_ENTRY:
+    case TARGET_WAITKIND_SYSCALL_RETURN:
+      pending_follow.kind = TARGET_WAITKIND_SPURIOUS;
+      break;
+
     default:
       break;
     }
@@ -1471,7 +1476,7 @@ init_wait_for_inferior (void)
 
   breakpoint_init_inferior (inf_starting);
 
-  /* The first resume is not following a fork/vfork/exec. */
+  /* The first resume is not following a fork/vfork/exec/syscall.  */
   pending_follow.kind = TARGET_WAITKIND_SPURIOUS;	/* I.e., none. */
 
   clear_proceed_status ();
@@ -2049,6 +2054,50 @@ ensure_not_running (void)
     error_is_running ();
 }
 
+/* Auxiliary function that handles syscall entry/return events.
+   It returns 1 if the inferior should keep going (and GDB
+   should ignore the event), or 0 if the event deserves to be
+   processed.  */
+static int
+deal_with_syscall_event (struct execution_control_state *ecs)
+{
+  int syscall_number = gdbarch_get_syscall_number (current_gdbarch,
+                                                   ecs->ptid);
+  if (catch_syscall_enabled () > 0
+      && catching_syscall_number (syscall_number) > 0)
+    {
+      ecs->event_thread->stop_signal = TARGET_SIGNAL_TRAP;
+      pending_follow.kind = ecs->ws.kind;
+
+      if (!ptid_equal (ecs->ptid, inferior_ptid))
+        {
+          context_switch (ecs->ptid);
+          reinit_frame_cache ();
+        }
+
+      stop_pc = read_pc ();
+
+      ecs->event_thread->stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid);
+
+      ecs->random_signal = !bpstat_explains_signal (ecs->event_thread->stop_bpstat);
+
+      /* If no catchpoint triggered for this, then keep going.  */
+      if (ecs->random_signal)
+        {
+          ecs->event_thread->stop_signal = TARGET_SIGNAL_0;
+          keep_going (ecs);
+          return 1;
+        }
+      return 0;
+    }
+  else
+    {
+      resume (0, TARGET_SIGNAL_0);
+      prepare_to_wait (ecs);
+      return 1;
+    }
+}
+
 /* Given an execution control state that has been freshly filled in
    by an event from the inferior, figure out what it means and take
    appropriate action.  */
@@ -2337,9 +2386,11 @@ handle_inferior_event (struct execution_control_state *ecs)
     case TARGET_WAITKIND_SYSCALL_ENTRY:
       if (debug_infrun)
         fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_SYSCALL_ENTRY\n");
-      resume (0, TARGET_SIGNAL_0);
-      prepare_to_wait (ecs);
-      return;
+      /* Getting the current syscall number */
+      if (deal_with_syscall_event (ecs) != 0)
+        return;
+      goto process_event_stop_test;
+      break;
 
       /* Before examining the threads further, step this thread to
          get it entirely out of the syscall.  (We get notice of the
@@ -2349,9 +2400,10 @@ handle_inferior_event (struct execution_control_state *ecs)
     case TARGET_WAITKIND_SYSCALL_RETURN:
       if (debug_infrun)
         fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_SYSCALL_RETURN\n");
-      target_resume (ecs->ptid, 1, TARGET_SIGNAL_0);
-      prepare_to_wait (ecs);
-      return;
+      if (deal_with_syscall_event (ecs) != 0)
+        return;
+      goto process_event_stop_test;
+      break;
 
     case TARGET_WAITKIND_STOPPED:
       if (debug_infrun)
@@ -3199,6 +3251,16 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n");
 	}
 	break;
 
+      case BPSTAT_WHAT_ENTRY_BREAKPOINT:
+        /* We hit the AT_ENTRY breakpoint, and now we have to enable
+           the PTRACE_O_TRACESYSGOOD option in the inferior *if* we
+           are catching syscalls.  */
+        if (debug_infrun)
+          fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_ENTRY_BREAKPOINT\n");
+        target_enable_tracesysgood (ecs->ptid);
+        ecs->event_thread->stepping_over_breakpoint = 1;
+        break;
+
       case BPSTAT_WHAT_LAST:
 	/* Not a real code, but listed here to shut up gcc -Wall.  */
 
@@ -4914,6 +4976,25 @@ inferior_has_execd (ptid_t pid, char **execd_pathname)
   return 1;
 }
 
+int
+inferior_has_called_syscall (ptid_t pid, int *syscall_number)
+{
+  struct target_waitstatus last;
+  ptid_t last_ptid;
+
+  get_last_target_status (&last_ptid, &last);
+
+  if (last.kind != TARGET_WAITKIND_SYSCALL_ENTRY &&
+      last.kind != TARGET_WAITKIND_SYSCALL_RETURN)
+    return 0;
+
+  if (!ptid_equal (last_ptid, pid))
+    return 0;
+
+  *syscall_number = last.value.syscall_number;
+  return 1;
+}
+
 /* Oft used ptids */
 ptid_t null_ptid;
 ptid_t minus_one_ptid;
diff --git a/gdb/main.c b/gdb/main.c
index 15a6558..ebb19c6 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -62,6 +62,9 @@ int dbx_commands = 0;
 /* System root path, used to find libraries etc.  */
 char *gdb_sysroot = 0;
 
+/* GDB datadir, used to store data files.  */
+char *gdb_datadir = 0;
+
 struct ui_file *gdb_stdout;
 struct ui_file *gdb_stderr;
 struct ui_file *gdb_stdlog;
@@ -268,6 +271,40 @@ captured_main (void *data)
 	}
     }
 
+#ifdef GDB_DATADIR_RELOCATABLE
+  gdb_datadir = make_relative_prefix (argv[0], BINDIR, GDB_DATADIR);
+  if (gdb_datadir)
+    {
+      struct stat s;
+      int res = 0;
+
+      if (stat (gdb_datadir, &s) == 0)
+	if (S_ISDIR (s.st_mode))
+	  res = 1;
+
+      if (res == 0)
+	{
+	  xfree (gdb_datadir);
+	  gdb_datadir = xstrdup (GDB_DATADIR);
+	}
+    }
+  else
+    gdb_datadir = xstrdup (GDB_DATADIR);
+#else
+  gdb_datadir = xstrdup (GDB_DATADIR);
+#endif /* GDB_DATADIR_RELOCATABLE */
+
+  /* Canonicalize the GDB's datadir path.  */
+  if (*gdb_datadir)
+    {
+      char *canon_debug = lrealpath (gdb_datadir);
+      if (canon_debug)
+	{
+	  xfree (gdb_datadir);
+	  gdb_datadir = canon_debug;
+	}
+    }
+
   /* There will always be an interpreter.  Either the one passed into
      this captured main, or one specified by the user at start up, or
      the console.  Initialize the interpreter to the one requested by 
diff --git a/gdb/maint.c b/gdb/maint.c
index 365e374..3aae317 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -882,4 +882,12 @@ When enabled GDB is profiled."),
 			   show_maintenance_profile_p,
 			   &maintenance_set_cmdlist,
 			   &maintenance_show_cmdlist);
+  add_setshow_filename_cmd ("gdb_datadir", class_maintenance,
+                           &gdb_datadir, _("Set GDB's datadir path."),
+                           _("Show GDB's datadir path."),
+                           _("\
+When set, GDB uses the specified path to search for data files."),
+                           NULL, NULL,
+                           &maintenance_set_cmdlist,
+                           &maintenance_show_cmdlist);
 }
diff --git a/gdb/target.c b/gdb/target.c
index fa9a941..e66c430 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -438,6 +438,10 @@ update_current_target (void)
       /* Do not inherit to_follow_fork.  */
       INHERIT (to_insert_exec_catchpoint, t);
       INHERIT (to_remove_exec_catchpoint, t);
+      INHERIT (to_passed_by_entrypoint, t);
+      INHERIT (to_insert_syscall_catchpoint, t);
+      INHERIT (to_remove_syscall_catchpoint, t);
+      INHERIT (to_enable_tracesysgood, t);
       INHERIT (to_has_exited, t);
       INHERIT (to_mourn_inferior, t);
       INHERIT (to_can_run, t);
@@ -596,9 +600,21 @@ update_current_target (void)
   de_fault (to_insert_exec_catchpoint,
 	    (void (*) (int))
 	    tcomplain);
+  de_fault (to_passed_by_entrypoint,
+            (int (*) (void))
+            tcomplain);
   de_fault (to_remove_exec_catchpoint,
 	    (int (*) (int))
 	    tcomplain);
+  de_fault (to_insert_syscall_catchpoint,
+	    (void (*) (int))
+	    tcomplain);
+  de_fault (to_remove_syscall_catchpoint,
+	    (int (*) (int))
+	    tcomplain);
+  de_fault (to_enable_tracesysgood,
+            (void (*) (ptid_t))
+            tcomplain);
   de_fault (to_has_exited,
 	    (int (*) (int, int, int *))
 	    return_zero);
@@ -2562,6 +2578,12 @@ debug_to_wait (ptid_t ptid, struct target_waitstatus *status)
     case TARGET_WAITKIND_EXECD:
       fprintf_unfiltered (gdb_stdlog, "execd\n");
       break;
+    case TARGET_WAITKIND_SYSCALL_ENTRY:
+      fprintf_unfiltered (gdb_stdlog, "entered syscall\n");
+      break;
+    case TARGET_WAITKIND_SYSCALL_RETURN:
+      fprintf_unfiltered (gdb_stdlog, "exited syscall\n");
+      break;
     case TARGET_WAITKIND_SPURIOUS:
       fprintf_unfiltered (gdb_stdlog, "spurious\n");
       break;
diff --git a/gdb/target.h b/gdb/target.h
index a010d2d..1b935be 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -139,14 +139,15 @@ struct target_waitstatus
   {
     enum target_waitkind kind;
 
-    /* Forked child pid, execd pathname, exit status or signal number.  */
+    /* Forked child pid, execd pathname, exit status, signal number or
+       syscall name.  */
     union
       {
 	int integer;
 	enum target_signal sig;
 	ptid_t related_pid;
 	char *execd_pathname;
-	int syscall_id;
+	int syscall_number;
       }
     value;
   };
@@ -397,6 +398,10 @@ struct target_ops
     int (*to_follow_fork) (struct target_ops *, int);
     void (*to_insert_exec_catchpoint) (int);
     int (*to_remove_exec_catchpoint) (int);
+    int (*to_passed_by_entrypoint) (void);
+    void (*to_insert_syscall_catchpoint) (int);
+    int (*to_remove_syscall_catchpoint) (int);
+    void (*to_enable_tracesysgood) (ptid_t);
     int (*to_has_exited) (int, int, int *);
     void (*to_mourn_inferior) (void);
     int (*to_can_run) (void);
@@ -731,6 +736,8 @@ extern int inferior_has_vforked (ptid_t pid, ptid_t *child_pid);
 
 extern int inferior_has_execd (ptid_t pid, char **execd_pathname);
 
+extern int inferior_has_called_syscall (ptid_t pid, int *syscall_number);
+
 /* From exec.c */
 
 extern void print_section_info (struct target_ops *, bfd *);
@@ -890,6 +897,24 @@ int target_follow_fork (int follow_child);
 #define target_remove_exec_catchpoint(pid) \
      (*current_target.to_remove_exec_catchpoint) (pid)
 
+/* Has the inferior already passed through its entrypoint? */
+#define target_passed_by_entrypoint() \
+     (*current_target.to_passed_by_entrypoint) ()
+
+/* Syscall catch functions */
+
+#define target_insert_syscall_catchpoint(pid) \
+     (*current_target.to_insert_syscall_catchpoint) (pid)
+
+#define target_remove_syscall_catchpoint(pid) \
+     (*current_target.to_remove_syscall_catchpoint) (pid)
+
+/* Enable PTRACE_O_TRACESYSGOOD in the inferior.
+   This is mainly used for the "catch syscall" feature.  */
+
+#define target_enable_tracesysgood(ptid) \
+     (*current_target.to_enable_tracesysgood) (ptid)
+
 /* Returns TRUE if PID has exited.  And, also sets EXIT_STATUS to the
    exit code of PID, if any.  */
 

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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent  part
  2008-11-04  4:32 [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part Sérgio Durigan Júnior
@ 2008-11-04 16:17 ` Pedro Alves
  2008-11-07  3:30   ` Sérgio Durigan Júnior
  2008-11-04 17:57 ` Tom Tromey
  2008-11-04 21:13 ` Eli Zaretskii
  2 siblings, 1 reply; 57+ messages in thread
From: Pedro Alves @ 2008-11-04 16:17 UTC (permalink / raw)
  To: gdb-patches; +Cc: Sérgio Durigan Júnior

On Tuesday 04 November 2008 04:31:19, Sérgio Durigan Júnior wrote:
> +static enum print_stop_action
> +print_it_catch_syscall (struct breakpoint *b)
> +{
> +  /* This is needed because we want to know in which state a
> +     syscall is.  It can be in the TARGET_WAITKIND_SYSCALL_ENTRY
> +     or TARGET_WAITKIND_SYSCALL_RETURN, and depending on it we
> +     must print "called syscall" or "returned from syscall".  */
> +  struct thread_info *th_info = find_thread_pid (inferior_ptid);
> +  const char *syscall_name =
> +    gdbarch_syscall_name_from_number (current_gdbarch, b->syscall_number);
> +
> +  annotate_catchpoint (b->number);
> +  printf_filtered (_("\nCatchpoint %d ("), b->number);
> +
> +  if (th_info->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY)
> +    printf_filtered (_("calling "));
> +  else
> +    printf_filtered (_("returned from "));

This bit left me wondering about the below.  Take these with a
grain of salt, please :-)

syscall_state has been placed in struct thread_info, and linux-nat.c
toggles it between entry/exit, but that's mainly because on linux, the
same trap is sent in both cases.  In the ttrace case (inf-ttrace.c), for
example, you have distinct TTEVT_SYSCALL_ENTRY and TTEVT_SYSCALL_RETURN
events at the target level.  Shouldn't we be doing the same on linux?  That
is, move 'syscall_state' to 'struct lwp_info', thus making it
private to the linux-nat.c implementation, and have the core side
always distinguish them from the TARGET_WAITKIND_SYSCALL_* type
returned from target_wait?  It looks weird for the target side to
be writing to a thread_info directly.  I always tend to ponder how I'd
do these things in gdbserver to validate target_ops designs --- I guess
I wouldn't be able to tweak gdb's common code from there.  :-)

Was it because you need to access it in print_it_catch_syscall?  You
could get it from the last target event like you do in
breakpoint_hit_catch_syscall, I guess.

(I guess the ideal solution would be for a bpstat to be able
to hold extra catchpoint hit state without re-querying the
last target event, but that would require an extra redesign of
the breakpoint hit -> bpstat building.)

Also, I'm not 100% sure, but I think you can crash in
linux-nat.c:linux_handle_extended_wait if an lwp happens to hit a syscall
you're catching before it's corresponding thread has been added to the
thread list in linux-thread-db.c.

Also, while we're on to speaking of these matters, would it make sense
to be able to catch only syscall entry or syscall return events
at the UI level?  That is, separate "catch syscall entry",
"catch syscall return" or some such?

-- 
Pedro Alves


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent  part
  2008-11-04  4:32 [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part Sérgio Durigan Júnior
  2008-11-04 16:17 ` Pedro Alves
@ 2008-11-04 17:57 ` Tom Tromey
  2008-11-04 21:55   ` Thiago Jung Bauermann
  2008-11-07  3:39   ` Sérgio Durigan Júnior
  2008-11-04 21:13 ` Eli Zaretskii
  2 siblings, 2 replies; 57+ messages in thread
From: Tom Tromey @ 2008-11-04 17:57 UTC (permalink / raw)
  To: Sérgio Durigan Júnior; +Cc: gdb-patches

>>>>> "Sérgio" == Sérgio Durigan Júnior <sergiodj@linux.vnet.ibm.com> writes:

Sérgio> +/* The maximum number of arguments the user can provide to
Sérgio> +   the 'catch syscall' command.  */
Sérgio> +#define MAX_CATCH_SYSCALL_ARGS 10

Do we need a maximum?  GNU style is not to have them.

Sérgio> +    printf_filtered (_("'%s ()'"), syscall_name);

I don't think the '()' is needed here.

Sérgio> +static void
Sérgio> +print_mention_catch_syscall (struct breakpoint *b)
Sérgio> +{
Sérgio> +  if (b->syscall_to_be_caught != CATCHING_ANY_SYSCALL)
Sérgio> +    printf_filtered (_("Catchpoint %d (syscall '%s ()')"),

Same here.

Sérgio> +/* Splits the argument using space as delimiter.
Sérgio> +   Returns the number of args.  */
Sérgio> +static int
Sérgio> +catch_syscall_split_args (char *arg, int *syscalls_numbers)

Sérgio> +      memset ((void *) cur_name, '\0', 128 * sizeof (char));

I don't think this is needed.
Also, sizeof(char)==1 by definition.

Sérgio> +      for (i = 0; out == 0; i++)
Sérgio> +        {
Sérgio> +          c = *arg_p;
Sérgio> +          cur_name[i] = c;
Sérgio> +          if (isspace (c) || c == '\0')
Sérgio> +            {
Sérgio> +              out = 1;
Sérgio> +              cur_name[i] = '\0';

I'd say, remove "out", make it an infinite loop, and use a 'break' in
the exit condition.

Sérgio> +static void
Sérgio> +catch_syscall_command_1 (char *arg, int from_tty, struct cmd_list_element *command)

I think you need a line break before the 'struct' here.

Sérgio> +      for (i = 0; i < nargs; i++)
Sérgio> +        create_syscall_event_catchpoint (tempflag, syscalls_numbers[i],
Sérgio> +                                         &catch_syscall_breakpoint_ops);

This makes a separate catchpoint for each argument to "catch syscall".

I think it would be more useful to make a single catchpoint.  A single
catchpoint gives the user a way to set commands, conditions, etc, for
a whole range of syscalls at once.  It is analogous, I think, to
having a breakpoint with multiple locations.

What do you think of that?

It would mean some changes in the logic and some changes in the data
structure -- but nothing too major.  Usually a catchpoint would have a
small number of syscalls, so I'd say that just using a linked list
would be fine.

Sérgio> +/* Complete syscalls names.  Used by "catch syscall".  */
Sérgio> +char **
Sérgio> +catch_syscall_completer (char *text, char *word)
Sérgio> +{
Sérgio> +  const char **list =
Sérgio> +    gdbarch_get_syscalls_names (current_gdbarch);
Sérgio> +  return (list == NULL) ? NULL : complete_on_enum (list, text, word);
Sérgio> +}

I think you should just put this in breakpoint.c and make it static.
My reasoning is that it is likely that only this one particular
command will need this completion function.

Tom


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part
  2008-11-04  4:32 [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part Sérgio Durigan Júnior
  2008-11-04 16:17 ` Pedro Alves
  2008-11-04 17:57 ` Tom Tromey
@ 2008-11-04 21:13 ` Eli Zaretskii
  2008-11-04 22:12   ` Thiago Jung Bauermann
  2 siblings, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2008-11-04 21:13 UTC (permalink / raw)
  To: Sérgio Durigan Júnior; +Cc: gdb-patches

> From: =?ISO-8859-1?Q?S=E9rgio?= Durigan =?ISO-8859-1?Q?J=FAnior?= <sergiodj@linux.vnet.ibm.com>
> Date: Tue, 04 Nov 2008 02:31:19 -0200

Thanks, but I cannot say I like the non-portable assumptions of this
design.  For example:

> +void
> +clear_syscall_catchpoints_info (void)
> +{
> +  struct breakpoint *b;
> +
> +  ALL_BREAKPOINTS (b)
> +    if (syscall_catchpoint_p (b))
> +      b->syscall_number = UNKNOWN_SYSCALL;
> +}     ^^^^^^^^^^^^^^^^^

Who said that a syscall is necessarily defined by some number?

More generally, let's say I'd like to implement support for this on
Windows -- how would I need to go about it?

> +      /* We are in a entry breakpoint.  */
> +      entry_breakpoint,
> +

This comment is too obscure, IMO.  Please make it more explicit; I
think you want to say that it's an entry to a syscall, right?

> +  const char *syscall_name =
> +    gdbarch_syscall_name_from_number (current_gdbarch, b->syscall_number);

Does this mean the notion that a syscall is a number have crept into
gdbarch, which is supposed to be the most architecture-independent
part of GDB?

> +      /* Checking if the user provided a syscall name or a number.  */
> +      if (isdigit (cur_name[0]))

Is the assumption that no name will ever begin with a digit
universally valid?

> +            error (_("The number '%d' does not represent a valid syscall."),
                                                            ^^^^^^^^^^^^^^^
"a known syscall", I'd say.

> +          if (syscall_number == UNKNOWN_SYSCALL)
> +            error (_("Invalid syscall name '%s'."), cur_name);
                         ^^^^^^^
"Unknown" is more accurate.

>  static void
> @@ -7441,6 +7792,7 @@ delete_command (char *arg, int from_tty)
>  	    b->type != bp_shlib_event &&
>  	    b->type != bp_thread_event &&
>  	    b->type != bp_overlay_event &&
> +            b->type != bp_entry_breakpoint &&

The previous lines used TABs and spaces for indentation, while your
line uses only spaces.

> +  add_catch_command ("syscall", _("\
> +Catch calls to syscalls.\n\

"call to syscalls" is awkward.  I suggest to drop the "calls to" part.

> +With an argument, catch only calls of that syscall."),

Ditto, suggest to drop the "calls of" part.

> +  add_setshow_filename_cmd ("gdb_datadir", class_maintenance,
> +                           &gdb_datadir, _("Set GDB's datadir path."),
> +                           _("Show GDB's datadir path."),
> +                           _("\
> +When set, GDB uses the specified path to search for data files."),
> +                           NULL, NULL,
> +                           &maintenance_set_cmdlist,
> +                           &maintenance_show_cmdlist);

This should be described in the manual.


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-04 17:57 ` Tom Tromey
@ 2008-11-04 21:55   ` Thiago Jung Bauermann
  2008-11-04 22:33     ` Tom Tromey
  2008-11-07  3:39   ` Sérgio Durigan Júnior
  1 sibling, 1 reply; 57+ messages in thread
From: Thiago Jung Bauermann @ 2008-11-04 21:55 UTC (permalink / raw)
  To: tromey; +Cc: Sérgio Durigan Júnior, gdb-patches

El mar, 04-11-2008 a las 10:56 -0700, Tom Tromey escribió:
> Sérgio> +      for (i = 0; i < nargs; i++)
> Sérgio> +        create_syscall_event_catchpoint (tempflag, syscalls_numbers[i],
> Sérgio> +                                         &catch_syscall_breakpoint_ops);
> 
> This makes a separate catchpoint for each argument to "catch syscall".
> 
> I think it would be more useful to make a single catchpoint.  A single
> catchpoint gives the user a way to set commands, conditions, etc, for
> a whole range of syscalls at once.  It is analogous, I think, to
> having a breakpoint with multiple locations.
> 
> What do you think of that?

I think a better analogy is a breakpoint with a condition. Ideally, I
think you would do that by setting a catchpoint for all syscalls (which
Sérgio's patch supports) and then put conditions to tell which syscalls
you are interested in. At least for me, it sounds more natural.

Except that there's no way to do that. :-) Perhaps we could create a
convenience function which would return the syscall name?

> It would mean some changes in the logic and some changes in the data
> structure -- but nothing too major.  Usually a catchpoint would have a
> small number of syscalls, so I'd say that just using a linked list
> would be fine.

With my suggestion, the logic would mostly stay the same, with a small
addition to evaluate an expression when the catchpoint triggers, IIUC.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-04 21:13 ` Eli Zaretskii
@ 2008-11-04 22:12   ` Thiago Jung Bauermann
  2008-11-04 22:22     ` Eli Zaretskii
  2008-11-04 22:31     ` Pedro Alves
  0 siblings, 2 replies; 57+ messages in thread
From: Thiago Jung Bauermann @ 2008-11-04 22:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Sérgio Durigan Júnior, gdb-patches

El mar, 04-11-2008 a las 23:12 +0200, Eli Zaretskii escribió:
> Who said that a syscall is necessarily defined by some number?

I assumed every OS used numbers to define syscalls ...

> More generally, let's say I'd like to implement support for this on
> Windows -- how would I need to go about it?

... but from what you are saying it seems that in Windows it's
different. What's the proper datatype to represent a syscall there?

> > +      /* Checking if the user provided a syscall name or a number.  */
> > +      if (isdigit (cur_name[0]))
> 
> Is the assumption that no name will ever begin with a digit
> universally valid?

Syscall names need to be valid function names in at least the most
common programming languages. I'm far from being a specialist, but isn't
it a very common (or universal?) restriction that function names have to
start with a non-digit character?
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part
  2008-11-04 22:12   ` Thiago Jung Bauermann
@ 2008-11-04 22:22     ` Eli Zaretskii
  2008-11-04 22:35       ` Daniel Jacobowitz
  2008-11-04 22:31     ` Pedro Alves
  1 sibling, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2008-11-04 22:22 UTC (permalink / raw)
  To: Thiago Jung Bauermann; +Cc: sergiodj, gdb-patches

> From: Thiago Jung Bauermann <bauerman@br.ibm.com>
> Cc: =?ISO-8859-1?Q?S=E9rgio?= Durigan =?ISO-8859-1?Q?J=FAnior?= <sergiodj@linux.vnet.ibm.com>,
>         gdb-patches@sourceware.org
> Date: Tue, 04 Nov 2008 20:11:27 -0200
> 
> > More generally, let's say I'd like to implement support for this on
> > Windows -- how would I need to go about it?
> 
> ... but from what you are saying it seems that in Windows it's
> different. What's the proper datatype to represent a syscall there?

A symbol, I think.

> > > +      /* Checking if the user provided a syscall name or a number.  */
> > > +      if (isdigit (cur_name[0]))
> > 
> > Is the assumption that no name will ever begin with a digit
> > universally valid?
> 
> Syscall names need to be valid function names in at least the most
> common programming languages. I'm far from being a specialist, but isn't
> it a very common (or universal?) restriction that function names have to
> start with a non-digit character?

But why assume that, instead of just checking that the argument is
all-digits?  It's so easy (e.g., use `strtoul') that shortcuts like
this one are not needed.


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-04 22:12   ` Thiago Jung Bauermann
  2008-11-04 22:22     ` Eli Zaretskii
@ 2008-11-04 22:31     ` Pedro Alves
  2008-11-05  4:10       ` Eli Zaretskii
  1 sibling, 1 reply; 57+ messages in thread
From: Pedro Alves @ 2008-11-04 22:31 UTC (permalink / raw)
  To: gdb-patches
  Cc: Thiago Jung Bauermann, Eli Zaretskii, Sérgio Durigan Júnior

On Tuesday 04 November 2008 22:11:27, Thiago Jung Bauermann wrote:
> El mar, 04-11-2008 a las 23:12 +0200, Eli Zaretskii escribió:
> > Who said that a syscall is necessarily defined by some number?
> 
> I assumed every OS used numbers to define syscalls ...
> 
> > More generally, let's say I'd like to implement support for this on
> > Windows -- how would I need to go about it?
> 
> ... but from what you are saying it seems that in Windows it's
> different. What's the proper datatype to represent a syscall there?

Depends on what you're calling a syscall on Windows.

If talking about userland->kernel calls, similarly to this
new feature, an integer.

 http://www.metasploit.com/users/opcode/syscalls.html
 http://www.codeguru.com/cpp/w-p/system/devicedriverdevelopment/article.php/c8035

strace-like tracers on Windows are usually more interested in
tracing calls to all kinds of dlls, and they usually do so by
playing games with the import tables, I believe.

-- 
Pedro Alves


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-04 21:55   ` Thiago Jung Bauermann
@ 2008-11-04 22:33     ` Tom Tromey
  2008-11-05 19:05       ` Tom Tromey
  0 siblings, 1 reply; 57+ messages in thread
From: Tom Tromey @ 2008-11-04 22:33 UTC (permalink / raw)
  To: Thiago Jung Bauermann; +Cc: Sérgio Durigan Júnior, gdb-patches

>>>>> "Thiago" == Thiago Jung Bauermann <bauerman@br.ibm.com> writes:

Thiago> I think a better analogy is a breakpoint with a
Thiago> condition. Ideally, I think you would do that by setting a
Thiago> catchpoint for all syscalls (which Sérgio's patch supports)
Thiago> and then put conditions to tell which syscalls you are
Thiago> interested in. At least for me, it sounds more natural.

Thiago> Except that there's no way to do that. :-) Perhaps we could create a
Thiago> convenience function which would return the syscall name?

I think that would satisfy me for time being.  If we're going that
route, then as far as I'm concerned it can even be deferred until we
wire up the Python bits in this area.

However, as with the 'thread' (and Ada 'task') qualifier, the best
approach depends a bit on the underlying OS' capabilities.  That is,
if there is a ptrace-like thing that handles the condition checking in
the kernel, then it would be nice to expose that, instead of doing the
check in gdb.  I will try to find out if this is a planned froggy
feature.

Tom


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent  part
  2008-11-04 22:22     ` Eli Zaretskii
@ 2008-11-04 22:35       ` Daniel Jacobowitz
  2008-11-05  4:19         ` Eli Zaretskii
  0 siblings, 1 reply; 57+ messages in thread
From: Daniel Jacobowitz @ 2008-11-04 22:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Thiago Jung Bauermann, sergiodj, gdb-patches

On Wed, Nov 05, 2008 at 12:21:05AM +0200, Eli Zaretskii wrote:
> > ... but from what you are saying it seems that in Windows it's
> > different. What's the proper datatype to represent a syscall there?
> 
> A symbol, I think.

As far as I can tell, Windows has system calls just like other OS's
do; as of Windows NT they were triggered by "int 2e" and the syscall
number went in %eax.

There's plenty of other levels of potential traceability in a Windows
program than the system call, but I think that's the obvious one to
map "catch syscall" on to.  Although, I don't believe GDB will be able
to catch Windows syscalls; I don't see a way to do it, anyway, but
there's plenty I don't know about Windows.

-- 
Daniel Jacobowitz
CodeSourcery


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-04 22:31     ` Pedro Alves
@ 2008-11-05  4:10       ` Eli Zaretskii
  2008-11-05 12:29         ` Pedro Alves
  2008-11-05 13:32         ` Mark Kettenis
  0 siblings, 2 replies; 57+ messages in thread
From: Eli Zaretskii @ 2008-11-05  4:10 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, bauerman, sergiodj

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Tue, 4 Nov 2008 22:30:27 +0000
> Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>,
>  Eli Zaretskii <eliz@gnu.org>,
>  =?utf-8?q?S=C3=A9rgio_Durigan_J=C3=BAnior?= <sergiodj@linux.vnet.ibm.com>
> 
> On Tuesday 04 November 2008 22:11:27, Thiago Jung Bauermann wrote:
> > El mar, 04-11-2008 a las 23:12 +0200, Eli Zaretskii escribió:
> > > Who said that a syscall is necessarily defined by some number?
> > 
> > I assumed every OS used numbers to define syscalls ...
> > 
> > > More generally, let's say I'd like to implement support for this on
> > > Windows -- how would I need to go about it?
> > 
> > ... but from what you are saying it seems that in Windows it's
> > different. What's the proper datatype to represent a syscall there?
> 
> Depends on what you're calling a syscall on Windows.
> 
> If talking about userland->kernel calls, similarly to this
> new feature, an integer.
> 
>  http://www.metasploit.com/users/opcode/syscalls.html
>  http://www.codeguru.com/cpp/w-p/system/devicedriverdevelopment/article.php/c8035
> 
> strace-like tracers on Windows are usually more interested in
> tracing calls to all kinds of dlls, and they usually do so by
> playing games with the import tables, I believe.

I was thinking about the latter, as that is what is usually
interesting.


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part
  2008-11-04 22:35       ` Daniel Jacobowitz
@ 2008-11-05  4:19         ` Eli Zaretskii
  2008-11-05 13:34           ` Sérgio Durigan Júnior
  2008-11-05 14:55           ` Daniel Jacobowitz
  0 siblings, 2 replies; 57+ messages in thread
From: Eli Zaretskii @ 2008-11-05  4:19 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: bauerman, sergiodj, gdb-patches

> Date: Tue, 4 Nov 2008 17:34:21 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>,
> 	sergiodj@linux.vnet.ibm.com, gdb-patches@sourceware.org
> 
> On Wed, Nov 05, 2008 at 12:21:05AM +0200, Eli Zaretskii wrote:
> > > ... but from what you are saying it seems that in Windows it's
> > > different. What's the proper datatype to represent a syscall there?
> > 
> > A symbol, I think.
> 
> As far as I can tell, Windows has system calls just like other OS's
> do; as of Windows NT they were triggered by "int 2e" and the syscall
> number went in %eax.

This level is IMO uninteresting, because many important system
services on Windows are not in the kernel space.

I'd like us very much to have some higher-level abstraction of a
syscall in target-independent code, than just a number.  Can we do
that, please?


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-05  4:10       ` Eli Zaretskii
@ 2008-11-05 12:29         ` Pedro Alves
  2008-11-05 18:38           ` Eli Zaretskii
  2008-11-05 13:32         ` Mark Kettenis
  1 sibling, 1 reply; 57+ messages in thread
From: Pedro Alves @ 2008-11-05 12:29 UTC (permalink / raw)
  To: gdb-patches, Eli Zaretskii; +Cc: bauerman, sergiodj

A Wednesday 05 November 2008 04:09:28, Eli Zaretskii escreveu:
> > From: Pedro Alves <pedro@codesourcery.com>
> > Date: Tue, 4 Nov 2008 22:30:27 +0000
> > Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>,
> >  Eli Zaretskii <eliz@gnu.org>,
> >  =?utf-8?q?S=C3=A9rgio_Durigan_J=C3=BAnior?= <sergiodj@linux.vnet.ibm.com>
> > 
> > On Tuesday 04 November 2008 22:11:27, Thiago Jung Bauermann wrote:
> > > El mar, 04-11-2008 a las 23:12 +0200, Eli Zaretskii escribió:
> > > > Who said that a syscall is necessarily defined by some number?
> > > 
> > > I assumed every OS used numbers to define syscalls ...
> > > 
> > > > More generally, let's say I'd like to implement support for this on
> > > > Windows -- how would I need to go about it?
> > > 
> > > ... but from what you are saying it seems that in Windows it's
> > > different. What's the proper datatype to represent a syscall there?
> > 
> > Depends on what you're calling a syscall on Windows.
> > 
> > If talking about userland->kernel calls, similarly to this
> > new feature, an integer.
> > 
> >  http://www.metasploit.com/users/opcode/syscalls.html
> >  http://www.codeguru.com/cpp/w-p/system/devicedriverdevelopment/article.php/c8035
> > 

> > strace-like tracers on Windows are usually more interested in
> > tracing calls to all kinds of dlls, and they usually do so by
> > playing games with the import tables, I believe.
> 
> I was thinking about the latter, as that is what is usually
> interesting.

Yes, but that falls into api-tracing land, which is a bit different
from syscall tracing.  When using a debugger, if you're going to be
attached to the inferior anyway, it doesn't look like it's much
interesting to have it as a separate feature, and to go play with
import tables.  You can already set breakpoints in the functions you're
interested, and you can already inspect the stack frame with
symbolic data when they're hit, and automatically tell the inferior to
continue.  Perhaps having more scripting power would make
it even easier to use, but well, there's going to be python
for that.

If someone would want to implement "catch syscall" for native
windows, I'd say that some form to catch real syscalls is what
would make more sense, because that's what you can't do with a
regular breakpoint.

-- 
Pedro Alves


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-05  4:10       ` Eli Zaretskii
  2008-11-05 12:29         ` Pedro Alves
@ 2008-11-05 13:32         ` Mark Kettenis
  1 sibling, 0 replies; 57+ messages in thread
From: Mark Kettenis @ 2008-11-05 13:32 UTC (permalink / raw)
  To: eliz; +Cc: pedro, gdb-patches, bauerman, sergiodj

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1741 bytes --]

> Date: Wed, 05 Nov 2008 06:09:28 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > From: Pedro Alves <pedro@codesourcery.com>
> > Date: Tue, 4 Nov 2008 22:30:27 +0000
> > Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>,
> >  Eli Zaretskii <eliz@gnu.org>,
> >  =?utf-8?q?S=C3=A9rgio_Durigan_J=C3=BAnior?= <sergiodj@linux.vnet.ibm.com>
> > 
> > On Tuesday 04 November 2008 22:11:27, Thiago Jung Bauermann wrote:
> > > El mar, 04-11-2008 a las 23:12 +0200, Eli Zaretskii escribió:
> > > > Who said that a syscall is necessarily defined by some number?
> > > 
> > > I assumed every OS used numbers to define syscalls ...
> > > 
> > > > More generally, let's say I'd like to implement support for this on
> > > > Windows -- how would I need to go about it?
> > > 
> > > ... but from what you are saying it seems that in Windows it's
> > > different. What's the proper datatype to represent a syscall there?
> > 
> > Depends on what you're calling a syscall on Windows.
> > 
> > If talking about userland->kernel calls, similarly to this
> > new feature, an integer.
> > 
> >  http://www.metasploit.com/users/opcode/syscalls.html
> >  http://www.codeguru.com/cpp/w-p/system/devicedriverdevelopment/article.php/c8035
> > 
> > strace-like tracers on Windows are usually more interested in
> > tracing calls to all kinds of dlls, and they usually do so by
> > playing games with the import tables, I believe.
> 
> I was thinking about the latter, as that is what is usually
> interesting.

I won't stop you from implementing this as a windows-specific feature,
but I think we should reserve "catch syscall" for the case where you
actually hand over control to the kernel (and therefore wouldn't be
able to single-step instructions anymore).

Mark


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-05  4:19         ` Eli Zaretskii
@ 2008-11-05 13:34           ` Sérgio Durigan Júnior
  2008-11-05 18:42             ` Eli Zaretskii
  2008-11-08 19:31             ` Mark Kettenis
  2008-11-05 14:55           ` Daniel Jacobowitz
  1 sibling, 2 replies; 57+ messages in thread
From: Sérgio Durigan Júnior @ 2008-11-05 13:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Daniel Jacobowitz, bauerman, gdb-patches

On Wed, 2008-11-05 at 06:18 +0200, Eli Zaretskii wrote:

> I'd like us very much to have some higher-level abstraction of a
> syscall in target-independent code, than just a number.  Can we do
> that, please?

I'm sorry about my bad design assumptions, but that just sounded good
for me by the time I was developing. I think we can do the higher-level
abstraction that you are asking, but I'd like you to please describe in
more details how this abstraction would be, or even better, if that's
not asking too much you could take the code I did and implement
something better :-).

I'm sorry about these assumptions I've made, again.

Regards,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-05  4:19         ` Eli Zaretskii
  2008-11-05 13:34           ` Sérgio Durigan Júnior
@ 2008-11-05 14:55           ` Daniel Jacobowitz
  2008-11-05 18:43             ` Eli Zaretskii
  1 sibling, 1 reply; 57+ messages in thread
From: Daniel Jacobowitz @ 2008-11-05 14:55 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: bauerman, sergiodj, gdb-patches

On Wed, Nov 05, 2008 at 06:18:24AM +0200, Eli Zaretskii wrote:
> This level is IMO uninteresting, because many important system
> services on Windows are not in the kernel space.

But those aren't system calls; the terminology I'm accustomed to is
quite specific, a system call is a trap from user to kernel mode.
Should we make the description of 'catch syscall' more clear to
exclude other OS facilities?

Trapping on other kinds of Windows events would be a nice feature,
but I don't think it's this same feature.  We have other kinds of
catchpoints (e.g. for Ada exceptions); I think that's the right place
to add Windows system services, if someone contributes an
implementation to catch them.

-- 
Daniel Jacobowitz
CodeSourcery


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-05 12:29         ` Pedro Alves
@ 2008-11-05 18:38           ` Eli Zaretskii
  2008-11-05 18:57             ` Pedro Alves
  0 siblings, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2008-11-05 18:38 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, bauerman, sergiodj

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Wed, 5 Nov 2008 12:28:29 +0000
> Cc: bauerman@br.ibm.com,  sergiodj@linux.vnet.ibm.com
> 
> > > strace-like tracers on Windows are usually more interested in
> > > tracing calls to all kinds of dlls, and they usually do so by
> > > playing games with the import tables, I believe.
> > 
> > I was thinking about the latter, as that is what is usually
> > interesting.
> 
> Yes, but that falls into api-tracing land, which is a bit different
> from syscall tracing.

It's different in implementation details, but very similar in essence,
as far as Joe Random Hacker is concerned.  From the Windows
programmer's POV, Windows APIs _are_ syscalls.

> When using a debugger, if you're going to be attached to the
> inferior anyway, it doesn't look like it's much interesting to have
> it as a separate feature

Sorry, I don't understand why.  If you thought about setting a
breakpoint on a Windows API call, then this is not a trivial matter,
especially if you are not a Windows internals guru.

> If someone would want to implement "catch syscall" for native
> windows, I'd say that some form to catch real syscalls is what
> would make more sense, because that's what you can't do with a
> regular breakpoint.

I find it hard to believe that catching Int 2Eh calls would be
interesting to most Windows programmers.  Unlike Linux, the Windows
kernel implements only a tiny portion of useful services, the rest is
mostly user-space DLLs.  A radically different architecture calls for
a radically different interpretation of what is a ``syscall''.


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part
  2008-11-05 13:34           ` Sérgio Durigan Júnior
@ 2008-11-05 18:42             ` Eli Zaretskii
  2008-11-08 19:31             ` Mark Kettenis
  1 sibling, 0 replies; 57+ messages in thread
From: Eli Zaretskii @ 2008-11-05 18:42 UTC (permalink / raw)
  To: Sérgio Durigan Júnior; +Cc: drow, bauerman, gdb-patches

> From: =?ISO-8859-1?Q?S=E9rgio?= Durigan =?ISO-8859-1?Q?J=FAnior?= <sergiodj@linux.vnet.ibm.com>
> Cc: Daniel Jacobowitz <drow@false.org>, bauerman@br.ibm.com,
>         gdb-patches@sourceware.org
> Date: Wed, 05 Nov 2008 11:33:41 -0200
> 
> I think we can do the higher-level abstraction that you are asking,
> but I'd like you to please describe in more details how this
> abstraction would be

I think a better interface would be to specify a syscall by its name,
a string, not its number.  The target-side code will then do TRT with
the string; the Linux target will convert that to a number and use
that (and probably all other Posix platforms will do that as well).
Higher-level GDB code, such as breakpoint.c, should never be exposed
to the fact that a syscall is identified by a number.

> or even better, if that's not asking too much you could take the
> code I did and implement something better :-).

Unfortunately, I don't have nearly enough time to work on this.  Which
makes my arguments much less convincing, I know.


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part
  2008-11-05 14:55           ` Daniel Jacobowitz
@ 2008-11-05 18:43             ` Eli Zaretskii
  2008-11-05 18:59               ` Daniel Jacobowitz
  2008-11-06 23:03               ` Mark Kettenis
  0 siblings, 2 replies; 57+ messages in thread
From: Eli Zaretskii @ 2008-11-05 18:43 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: bauerman, sergiodj, gdb-patches

> Date: Wed, 5 Nov 2008 09:54:49 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: bauerman@br.ibm.com, sergiodj@linux.vnet.ibm.com,
> 	gdb-patches@sourceware.org
> 
> Should we make the description of 'catch syscall' more clear to
> exclude other OS facilities?

We could, but that would be second best, IMO.  It would be better to
make the abstraction less Unix-centric.


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-05 18:38           ` Eli Zaretskii
@ 2008-11-05 18:57             ` Pedro Alves
  2008-11-05 19:10               ` Eli Zaretskii
  0 siblings, 1 reply; 57+ messages in thread
From: Pedro Alves @ 2008-11-05 18:57 UTC (permalink / raw)
  To: gdb-patches, Eli Zaretskii; +Cc: bauerman, sergiodj

On ednesday 05 November 2008 18:37:26, Eli Zaretskii wrote
> > Yes, but that falls into api-tracing land, which is a bit different
> > from syscall tracing.
> 
> It's different in implementation details, but very similar in essence,
> as far as Joe Random Hacker is concerned.  From the Windows
> programmer's POV, Windows APIs _are_ syscalls.

Sorry, here's where I think I'm missing something.

What's wrong with "break ReadFile" ?  What would you gain with
"catch syscall ReadFile", and how would that be different from
"b foo_function_in_a_random_so" on non-Windows systems?

-- 
Pedro Alves


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent  part
  2008-11-05 18:43             ` Eli Zaretskii
@ 2008-11-05 18:59               ` Daniel Jacobowitz
  2008-11-05 19:11                 ` Eli Zaretskii
  2008-11-06 23:03               ` Mark Kettenis
  1 sibling, 1 reply; 57+ messages in thread
From: Daniel Jacobowitz @ 2008-11-05 18:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: bauerman, sergiodj, gdb-patches

On Wed, Nov 05, 2008 at 08:43:02PM +0200, Eli Zaretskii wrote:
> We could, but that would be second best, IMO.  It would be better to
> make the abstraction less Unix-centric.

I don't think that Windows API calls belong in "catch syscall".  It's
not like other platforms don't provide system services in userspace
too; but that's a different debugger feature than this one.

I wouldn't call the abstraction Unix-centric.  Classic Mac OS did the
same thing; I believe the Amiga OS did too.  A better example might be
Hurd, which uses system calls in the same way - but primarily to
facilitate inter-program communication to the servers providing system
services.  There's scope for a debugger to trace system services
separately from system calls.

-- 
Daniel Jacobowitz
CodeSourcery


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-04 22:33     ` Tom Tromey
@ 2008-11-05 19:05       ` Tom Tromey
  2008-11-05 19:13         ` Sérgio Durigan Júnior
  2008-11-07  3:41         ` Sérgio Durigan Júnior
  0 siblings, 2 replies; 57+ messages in thread
From: Tom Tromey @ 2008-11-05 19:05 UTC (permalink / raw)
  To: Thiago Jung Bauermann; +Cc: Sérgio Durigan Júnior, gdb-patches

>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:

Tom> However, as with the 'thread' (and Ada 'task') qualifier, the best
Tom> approach depends a bit on the underlying OS' capabilities.  That is,
Tom> if there is a ptrace-like thing that handles the condition checking in
Tom> the kernel, then it would be nice to expose that, instead of doing the
Tom> check in gdb.  I will try to find out if this is a planned froggy
Tom> feature.

I asked, and apparently this is already in froggy.  The filtering is
implemented kernel-side.

So, I would like it if this part were changed as I suggested
up-thread.  We're starting to look at porting gdb to froggy and it
would be nice to have this in place from the beginning.

Sérgio, I'm willing to write a patch against your branch for this, if
you want.  Just let me know.

thanks,
Tom


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-05 18:57             ` Pedro Alves
@ 2008-11-05 19:10               ` Eli Zaretskii
  2008-11-05 19:34                 ` Pedro Alves
  0 siblings, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2008-11-05 19:10 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, bauerman, sergiodj

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Wed, 5 Nov 2008 18:56:26 +0000
> Cc: bauerman@br.ibm.com,
>  sergiodj@linux.vnet.ibm.com
> 
> What's wrong with "break ReadFile" ?

Does it work for you?  And if the breakpoint breaks, can you easily
see the arguments of the call?


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part
  2008-11-05 18:59               ` Daniel Jacobowitz
@ 2008-11-05 19:11                 ` Eli Zaretskii
  0 siblings, 0 replies; 57+ messages in thread
From: Eli Zaretskii @ 2008-11-05 19:11 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: bauerman, sergiodj, gdb-patches

> Date: Wed, 5 Nov 2008 13:58:56 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: bauerman@br.ibm.com, sergiodj@linux.vnet.ibm.com, 	gdb-patches@sourceware.org
> 
> On Wed, Nov 05, 2008 at 08:43:02PM +0200, Eli Zaretskii wrote:
> > We could, but that would be second best, IMO.  It would be better to
> > make the abstraction less Unix-centric.
> 
> I don't think that Windows API calls belong in "catch syscall".

You made it perfectly clear that this is what you think, but we will
have to disagree.

Regardless, I still think that the breakpoint.c notion of a syscall
should be a higher abstraction than just a number.


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-05 19:05       ` Tom Tromey
@ 2008-11-05 19:13         ` Sérgio Durigan Júnior
  2008-11-07  3:41         ` Sérgio Durigan Júnior
  1 sibling, 0 replies; 57+ messages in thread
From: Sérgio Durigan Júnior @ 2008-11-05 19:13 UTC (permalink / raw)
  To: tromey; +Cc: Thiago Jung Bauermann, gdb-patches

On Wed, 2008-11-05 at 12:04 -0700, Tom Tromey wrote:

> Sérgio, I'm willing to write a patch against your branch for this, if
> you want.  Just let me know.

Please, be my guest :-).

Regards,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-05 19:10               ` Eli Zaretskii
@ 2008-11-05 19:34                 ` Pedro Alves
  2008-11-05 20:36                   ` Eli Zaretskii
  0 siblings, 1 reply; 57+ messages in thread
From: Pedro Alves @ 2008-11-05 19:34 UTC (permalink / raw)
  To: gdb-patches, Eli Zaretskii; +Cc: bauerman, sergiodj

On Wednesday 05 November 2008 19:08:49, Eli Zaretskii wrote:
> > From: Pedro Alves <pedro@codesourcery.com>
> > Date: Wed, 5 Nov 2008 18:56:26 +0000
> > Cc: bauerman@br.ibm.com,
> >  sergiodj@linux.vnet.ibm.com
> > 
> > What's wrong with "break ReadFile" ?
> 
> Does it work for you?  

...  it does.  It's a function exported from a regular dll, just like
all other functions exported from other dlls.  I can only infer that
you haven't gone through the trouble of trying it yourself.

> And if the breakpoint breaks, can you easily 
> see the arguments of the call?
> 

No, but that's not different from breaking on any other function
in any other system when you don't have debug info for it.
To solve this, the real solution is to make GDB make use
of MSFT's PDB debug info, not to somehow make up a new method
to extract arguments from plain regular code.

This is *no different* from breaking on the "read" function in libc.so
on linux, without debug info for libc.so.  Eventually the libc
'read' function calls the real 'read' syscall, just like on Windows.

-- 
Pedro Alves


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-05 19:34                 ` Pedro Alves
@ 2008-11-05 20:36                   ` Eli Zaretskii
  2008-11-05 21:10                     ` Pedro Alves
  0 siblings, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2008-11-05 20:36 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, bauerman, sergiodj

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Wed, 5 Nov 2008 19:33:28 +0000
> Cc: bauerman@br.ibm.com,
>  sergiodj@linux.vnet.ibm.com
> 
> Eventually the libc 'read' function calls the real 'read' syscall,
> just like on Windows.

I don't think `ReadFile', the Windows equivalent of `read', calls Int
2Eh to read a file.  If you know differently, please tell the details.


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-05 20:36                   ` Eli Zaretskii
@ 2008-11-05 21:10                     ` Pedro Alves
  2008-11-06  4:27                       ` Eli Zaretskii
  0 siblings, 1 reply; 57+ messages in thread
From: Pedro Alves @ 2008-11-05 21:10 UTC (permalink / raw)
  To: gdb-patches, Eli Zaretskii; +Cc: bauerman, sergiodj

On Wednesday 05 November 2008 20:34:24, Eli Zaretskii wrote:
> I don't think `ReadFile', the Windows equivalent of `read', calls Int
> 2Eh to read a file.  If you know differently, please tell the details.

I don't understand what we're arguing about.  I'm not against making
it so that a syscall is identified by string instead of number, if
it's such a hard design decision that makes it impossible to
change things later on.

In NT the Win32 API functions are regular functions that are
implemented on top of OS services.  The kernel knows nothing about the
win32 API.  E.g., the ReadFile function is a wrapper around
the user land NtReadFile, which itself is what does the syscall.  You
can write NT programs without touching the win32 api.  Heck,
cygwin.dll is moving away from it.

There are thousands of win32 functions, spread across a big
number of dlls that the user could want to break on, in the use
case we're talking about.  Which of those would you consider
candidates to place a breakpoint for "catch syscall"?  All of them?

Yes, we could probably implement "catch syscall" on Windows by
placing a breakpoint on each of these functions:

 http://www.metasploit.com/users/opcode/syscalls.html

... this to me is the list of functions that makes sense to
break at with "catch syscall".  In this case, the win32 specific
code to implement the feature would probably map the numbers 
to the function names as well --- the set is bounded.
But, as you say, most Windows developers aren't that
interested in these.

I believe that what you want (and I'd like to have it too), is
the ability to easily break on all functions of a given Dll.  Something
like 'rbreak -public kernel32.dll!' (I believe minimal symbols for dlls
we don't have debug info for, are prefixed with the dll name like
that, by extracting the function names from the import table, but
I'm not sure where that's user visible).  Maybe even fold that ability
to the 'break' command, and bind all locations to a simple
breakpoint with multiple locations.  I don't see why this couldn't
be implemented on unix as well, for any '.so'. -- but this isn't
catching a "system call".  I think it would it look strange to do
catch syscall "MyDll.dll" to catch all "system functions" in
MyDll.dll, for example.

-- 
Pedro Alves


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-05 21:10                     ` Pedro Alves
@ 2008-11-06  4:27                       ` Eli Zaretskii
  2008-11-06 14:32                         ` Pedro Alves
  0 siblings, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2008-11-06  4:27 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, bauerman, sergiodj

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Wed, 5 Nov 2008 21:09:29 +0000
> Cc: bauerman@br.ibm.com,
>  sergiodj@linux.vnet.ibm.com
> 
> On Wednesday 05 November 2008 20:34:24, Eli Zaretskii wrote:
> > I don't think `ReadFile', the Windows equivalent of `read', calls Int
> > 2Eh to read a file.  If you know differently, please tell the details.
> 
> I don't understand what we're arguing about.

You said (or so I thought) that instead of watching the call to
ReadFile, one can watch some equivalent call to Int 2Eh which ReadFile
issues eventually, just like `read' does on Unix:

> Eventually the libc 'read' function calls the real 'read' syscall,
> just like on Windows.

I'm saying that I don't think such a function of Int 2Eh exists,
because I think only lower-level sector-oriented disk read commands
are implemented as software interrupts, and all the higher level
processing needed for reading the file are not in kernel space.

> In NT the Win32 API functions are regular functions that are
> implemented on top of OS services.  The kernel knows nothing about the
> win32 API.  E.g., the ReadFile function is a wrapper around
> the user land NtReadFile, which itself is what does the syscall.

But NtReadFile is also a function, not an Int 2Eh syscall, right?


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-06  4:27                       ` Eli Zaretskii
@ 2008-11-06 14:32                         ` Pedro Alves
  2008-11-07  9:59                           ` Eli Zaretskii
  0 siblings, 1 reply; 57+ messages in thread
From: Pedro Alves @ 2008-11-06 14:32 UTC (permalink / raw)
  To: gdb-patches, Eli Zaretskii; +Cc: bauerman, sergiodj

On Thursday 06 November 2008 04:25:38, Eli Zaretskii wrote:

> You said (or so I thought) that instead of watching the call to
> ReadFile, one can watch some equivalent call to Int 2Eh which ReadFile
> issues eventually, just like `read' does on Unix:

I said it ends up calling the syscall (as in, it calls some function, that
calls some function, that ..., that ends up calling the syscall), but
I didn't say we can "watch it".  AFAIK, there's no debug event to trap
on it.

> 
> > Eventually the libc 'read' function calls the real 'read' syscall,
> > just like on Windows.
> 
> I'm saying that I don't think such a function of Int 2Eh exists,
> because I think only lower-level sector-oriented disk read commands
> are implemented as software interrupts, and all the higher level
> processing needed for reading the file are not in kernel space.
> 
> > In NT the Win32 API functions are regular functions that are
> > implemented on top of OS services.  The kernel knows nothing about the
> > win32 API.  E.g., the ReadFile function is a wrapper around
> > the user land NtReadFile, which itself is what does the syscall.
> 
> But NtReadFile is also a function, not an Int 2Eh syscall, right?
> 

I thought that was clear from
"user land NtReadFile, which itself is what does the syscall".
I pointed at an url explaining how syscalls are done on NT.

Here's another one, which should be more clear:

 http://www.ddj.com/184410109

And linked from it, you'll find the disassembly of NtCreateFile:

 http://www.ddj.com/showArticle.jhtml?documentID=ddj9701e&pgno=2

The point I am making, is that what you wanted to bind to "catch syscall"
is in fact api-tracing --- tracing normal function calls, which you can do
with breakpoints, and isn't specific to Windows.  I mentioned or implied 
that most users would want to trace Win32 api calls, but those are
not "system calls".  I gave the example of ReadFile.

I mentioned that probably, you could get away with putting breakpoints
on the ntdll functions that themselves do the syscalls, but I didn't
say that's the only way.  There could be other better ways.

Please, let's stop this "I said you said" nonsense.

-- 
Pedro Alves


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part
  2008-11-05 18:43             ` Eli Zaretskii
  2008-11-05 18:59               ` Daniel Jacobowitz
@ 2008-11-06 23:03               ` Mark Kettenis
  1 sibling, 0 replies; 57+ messages in thread
From: Mark Kettenis @ 2008-11-06 23:03 UTC (permalink / raw)
  To: eliz; +Cc: drow, bauerman, sergiodj, gdb-patches

> X-Spam-Check-By: sourceware.org
> Date: Wed, 05 Nov 2008 20:43:02 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Date: Wed, 5 Nov 2008 09:54:49 -0500
> > From: Daniel Jacobowitz <drow@false.org>
> > Cc: bauerman@br.ibm.com, sergiodj@linux.vnet.ibm.com,
> > 	gdb-patches@sourceware.org
> > 
> > Should we make the description of 'catch syscall' more clear to
> > exclude other OS facilities?
> 
> We could, but that would be second best, IMO.  It would be better to
> make the abstraction less Unix-centric.

Come on.  Syscall tracing *is* low level stuff and making it less
Unix-centric makes it less useful.


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-04 16:17 ` Pedro Alves
@ 2008-11-07  3:30   ` Sérgio Durigan Júnior
  2008-11-07 12:12     ` Pedro Alves
  0 siblings, 1 reply; 57+ messages in thread
From: Sérgio Durigan Júnior @ 2008-11-07  3:30 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Hi Pedro,

On Tue, 2008-11-04 at 16:17 +0000, Pedro Alves wrote:
> On Tuesday 04 November 2008 04:31:19, Sérgio Durigan Júnior wrote:
> > +static enum print_stop_action
> > +print_it_catch_syscall (struct breakpoint *b)
> > +{
> > +  /* This is needed because we want to know in which state a
> > +     syscall is.  It can be in the TARGET_WAITKIND_SYSCALL_ENTRY
> > +     or TARGET_WAITKIND_SYSCALL_RETURN, and depending on it we
> > +     must print "called syscall" or "returned from syscall".  */
> > +  struct thread_info *th_info = find_thread_pid (inferior_ptid);
> > +  const char *syscall_name =
> > +    gdbarch_syscall_name_from_number (current_gdbarch, b->syscall_number);
> > +
> > +  annotate_catchpoint (b->number);
> > +  printf_filtered (_("\nCatchpoint %d ("), b->number);
> > +
> > +  if (th_info->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY)
> > +    printf_filtered (_("calling "));
> > +  else
> > +    printf_filtered (_("returned from "));
> 
> This bit left me wondering about the below.  Take these with a
> grain of salt, please :-)

Of course :-). And I'm sorry for taking so long to respond.

> syscall_state has been placed in struct thread_info, and linux-nat.c
> toggles it between entry/exit, but that's mainly because on linux, the
> same trap is sent in both cases.  In the ttrace case (inf-ttrace.c), for
> example, you have distinct TTEVT_SYSCALL_ENTRY and TTEVT_SYSCALL_RETURN
> events at the target level.  Shouldn't we be doing the same on linux?  That
> is, move 'syscall_state' to 'struct lwp_info', thus making it
> private to the linux-nat.c implementation, and have the core side
> always distinguish them from the TARGET_WAITKIND_SYSCALL_* type
> returned from target_wait?  It looks weird for the target side to
> be writing to a thread_info directly.  I always tend to ponder how I'd
> do these things in gdbserver to validate target_ops designs --- I guess
> I wouldn't be able to tweak gdb's common code from there.  :-)
> 
> Was it because you need to access it in print_it_catch_syscall?  You
> could get it from the last target event like you do in
> breakpoint_hit_catch_syscall, I guess.

Ok, I'll try to put the syscall_state in 'struct lwp_info'. Honestly, I
don't remember now why I've chosen to put this variable inside
thread_info, but of course you're way more capable of telling me how to
make my design be more clever (and look more like GDB) :-).

> Also, I'm not 100% sure, but I think you can crash in
> linux-nat.c:linux_handle_extended_wait if an lwp happens to hit a syscall
> you're catching before it's corresponding thread has been added to the
> thread list in linux-thread-db.c.

I'm not sure I'm able to answer this, but that's a problem that I wasn't
considering. Thanks for the hint.

> Also, while we're on to speaking of these matters, would it make sense
> to be able to catch only syscall entry or syscall return events
> at the UI level?  That is, separate "catch syscall entry",
> "catch syscall return" or some such?

IMHO, we could postpone this for now. Honestly I'm pretty satisfied with
the feature right now, but of course my opinion shouldn't count that
much ;-).

Thanks for your comments, I'll fix my code as soon as I can.

Regards, 

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-04 17:57 ` Tom Tromey
  2008-11-04 21:55   ` Thiago Jung Bauermann
@ 2008-11-07  3:39   ` Sérgio Durigan Júnior
  2008-11-07 18:21     ` Tom Tromey
  1 sibling, 1 reply; 57+ messages in thread
From: Sérgio Durigan Júnior @ 2008-11-07  3:39 UTC (permalink / raw)
  To: tromey; +Cc: gdb-patches

Hey Tom!

On Tue, 2008-11-04 at 10:56 -0700, Tom Tromey wrote:
> >>>>> "Sérgio" == Sérgio Durigan Júnior <sergiodj@linux.vnet.ibm.com> writes:
> 
> Sérgio> +/* The maximum number of arguments the user can provide to
> Sérgio> +   the 'catch syscall' command.  */
> Sérgio> +#define MAX_CATCH_SYSCALL_ARGS 10
> 
> Do we need a maximum?  GNU style is not to have them.

I don't know if we need :-). I just decided to put it there :-P. But of
course I can take it off.

> 
> Sérgio> +    printf_filtered (_("'%s ()'"), syscall_name);
> 
> I don't think the '()' is needed here.

OK.

> Sérgio> +static void
> Sérgio> +print_mention_catch_syscall (struct breakpoint *b)
> Sérgio> +{
> Sérgio> +  if (b->syscall_to_be_caught != CATCHING_ANY_SYSCALL)
> Sérgio> +    printf_filtered (_("Catchpoint %d (syscall '%s ()')"),
> 
> Same here.

Roger that.

> Sérgio> +/* Splits the argument using space as delimiter.
> Sérgio> +   Returns the number of args.  */
> Sérgio> +static int
> Sérgio> +catch_syscall_split_args (char *arg, int *syscalls_numbers)
> 
> Sérgio> +      memset ((void *) cur_name, '\0', 128 * sizeof (char));
> 
> I don't think this is needed.
> Also, sizeof(char)==1 by definition.

Yeah, I know hehe. It just happens that I like to be "explicit", so
you'll likely find "sizeof (char)" in every code I make :-)... But
there's no problem for me to take it off.

> Sérgio> +      for (i = 0; out == 0; i++)
> Sérgio> +        {
> Sérgio> +          c = *arg_p;
> Sérgio> +          cur_name[i] = c;
> Sérgio> +          if (isspace (c) || c == '\0')
> Sérgio> +            {
> Sérgio> +              out = 1;
> Sérgio> +              cur_name[i] = '\0';
> 
> I'd say, remove "out", make it an infinite loop, and use a 'break' in
> the exit condition.

Right.

> Sérgio> +static void
> Sérgio> +catch_syscall_command_1 (char *arg, int from_tty, struct cmd_list_element *command)
> 
> I think you need a line break before the 'struct' here.

Forgot about that. Sorry.

> Sérgio> +      for (i = 0; i < nargs; i++)
> Sérgio> +        create_syscall_event_catchpoint (tempflag, syscalls_numbers[i],
> Sérgio> +                                         &catch_syscall_breakpoint_ops);
> 
> This makes a separate catchpoint for each argument to "catch syscall".
> 
> I think it would be more useful to make a single catchpoint.  A single
> catchpoint gives the user a way to set commands, conditions, etc, for
> a whole range of syscalls at once.  It is analogous, I think, to
> having a breakpoint with multiple locations.

I have a question right here, then. Is the "breakpoint with multiple
locations" implemented this way? I'm sorry for my ignorance on this :-(

> 
> What do you think of that?
> 
> It would mean some changes in the logic and some changes in the data
> structure -- but nothing too major.  Usually a catchpoint would have a
> small number of syscalls, so I'd say that just using a linked list
> would be fine.

I was talking to Thiago about this, and I don't know if we need this
*right now*, for this patch. I think it's good the way it is, and as you
said, it won't take much to modify things in order to make it work the
way you want :-). Also, currently the "catch syscall" command doesn't
allow the user to set commands, conditions, etc. So for now it's only
"catch syscall [name|number]".

What do you think?

> Sérgio> +/* Complete syscalls names.  Used by "catch syscall".  */
> Sérgio> +char **
> Sérgio> +catch_syscall_completer (char *text, char *word)
> Sérgio> +{
> Sérgio> +  const char **list =
> Sérgio> +    gdbarch_get_syscalls_names (current_gdbarch);
> Sérgio> +  return (list == NULL) ? NULL : complete_on_enum (list, text, word);
> Sérgio> +}
> 
> I think you should just put this in breakpoint.c and make it static.
> My reasoning is that it is likely that only this one particular
> command will need this completion function.

Right, I agree. Consider it done.

Thanks for your comments :-).

Regards,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-05 19:05       ` Tom Tromey
  2008-11-05 19:13         ` Sérgio Durigan Júnior
@ 2008-11-07  3:41         ` Sérgio Durigan Júnior
  1 sibling, 0 replies; 57+ messages in thread
From: Sérgio Durigan Júnior @ 2008-11-07  3:41 UTC (permalink / raw)
  To: tromey; +Cc: Thiago Jung Bauermann, gdb-patches

Hi Tom :-),

On Wed, 2008-11-05 at 12:04 -0700, Tom Tromey wrote:
> >>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:
> 
> Tom> However, as with the 'thread' (and Ada 'task') qualifier, the best
> Tom> approach depends a bit on the underlying OS' capabilities.  That is,
> Tom> if there is a ptrace-like thing that handles the condition checking in
> Tom> the kernel, then it would be nice to expose that, instead of doing the
> Tom> check in gdb.  I will try to find out if this is a planned froggy
> Tom> feature.
> 
> I asked, and apparently this is already in froggy.  The filtering is
> implemented kernel-side.
> 
> So, I would like it if this part were changed as I suggested
> up-thread.  We're starting to look at porting gdb to froggy and it
> would be nice to have this in place from the beginning.

Hmm, ok. So please, disconsider my last e-mail (where I say that I think
things are OK for now). I'll implement this as you asked.

Regards,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-06 14:32                         ` Pedro Alves
@ 2008-11-07  9:59                           ` Eli Zaretskii
  2008-11-07 10:10                             ` Pedro Alves
  0 siblings, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2008-11-07  9:59 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, bauerman, sergiodj

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Thu, 6 Nov 2008 14:26:45 +0000
> Cc: bauerman@br.ibm.com,  sergiodj@linux.vnet.ibm.com
> 
> Please, let's stop this "I said you said" nonsense.

I'm sorry, but you asked what we were arguing about, so I tried to
explain.


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-07  9:59                           ` Eli Zaretskii
@ 2008-11-07 10:10                             ` Pedro Alves
  0 siblings, 0 replies; 57+ messages in thread
From: Pedro Alves @ 2008-11-07 10:10 UTC (permalink / raw)
  To: gdb-patches, Eli Zaretskii; +Cc: bauerman, sergiodj

On Friday 07 November 2008 09:58:16, Eli Zaretskii wrote:

> I'm sorry, but you asked what we were arguing about, so I tried to
> explain.

You're right, and I'm sorry for jumping on you.

-- 
Pedro Alves


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part
  2008-11-07  3:30   ` Sérgio Durigan Júnior
@ 2008-11-07 12:12     ` Pedro Alves
  2008-11-07 13:30       ` Daniel Jacobowitz
  2008-11-08 15:35       ` Sérgio Durigan Júnior
  0 siblings, 2 replies; 57+ messages in thread
From: Pedro Alves @ 2008-11-07 12:12 UTC (permalink / raw)
  To: Sérgio Durigan Júnior; +Cc: gdb-patches

On Friday 07 November 2008 03:29:25, Sérgio Durigan Júnior wrote:

> Ok, I'll try to put the syscall_state in 'struct lwp_info'. Honestly, I
> don't remember now why I've chosen to put this variable inside
> thread_info, but of course you're way more capable of telling me how to
> make my design be more clever (and look more like GDB) :-).

Nah, you're doing yourself just fine.  :-)

An alternative would be to add a new TARGET_WAITKIND_SYSCALL, for
targets that don't distinguish entry/exit, but this leaves me yet
to wonder:

If there any other way to distinguish entry/exit other than a toggle?
Toggles are prone to be fallible.  E.g., if you *attach* to a program that
is doing a long syscall, and then start tracing that syscall,
is it possible that you hit the syscall exit first, so your toggle will
be inverted?  That is, you'll report a syscall entry, when in fact, it
was a syscall exit, and so on for the following syscalls of the same lwp.

-- 
Pedro Alves


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-07 12:12     ` Pedro Alves
@ 2008-11-07 13:30       ` Daniel Jacobowitz
  2008-11-08 15:35       ` Sérgio Durigan Júnior
  1 sibling, 0 replies; 57+ messages in thread
From: Daniel Jacobowitz @ 2008-11-07 13:30 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Sérgio Durigan Júnior, gdb-patches

On Fri, Nov 07, 2008 at 12:06:31PM +0000, Pedro Alves wrote:
> If there any other way to distinguish entry/exit other than a toggle?
> Toggles are prone to be fallible.  E.g., if you *attach* to a program that
> is doing a long syscall, and then start tracing that syscall,
> is it possible that you hit the syscall exit first, so your toggle will
> be inverted?  That is, you'll report a syscall entry, when in fact, it
> was a syscall exit, and so on for the following syscalls of the same lwp.

There's code in strace to sort this out, though I do not remember
precisely how it works.  Yes, in current kernels we are limited to a
toggle.  There's PTRACE_O_TRACESYSGOOD (which I presume the patch
uses); that marks syscall traps specially, but does not differentiate
entry and exit.

-- 
Daniel Jacobowitz
CodeSourcery


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part
  2008-11-07  3:39   ` Sérgio Durigan Júnior
@ 2008-11-07 18:21     ` Tom Tromey
  0 siblings, 0 replies; 57+ messages in thread
From: Tom Tromey @ 2008-11-07 18:21 UTC (permalink / raw)
  To: Sérgio Durigan Júnior; +Cc: gdb-patches

>>>>> "Sérgio" == Sérgio Durigan Júnior <sergiodj@linux.vnet.ibm.com> writes:

Sérgio> +      memset ((void *) cur_name, '\0', 128 * sizeof (char));

Tom> I don't think this is needed.
Tom> Also, sizeof(char)==1 by definition.

Sérgio> Yeah, I know hehe. It just happens that I like to be "explicit", so
Sérgio> you'll likely find "sizeof (char)" in every code I make :-)... But
Sérgio> there's no problem for me to take it off.

Just to be clear, here I meant that the whole memset appears
unnecessary.

Tom> I think it would be more useful to make a single catchpoint.  A single
Tom> catchpoint gives the user a way to set commands, conditions, etc, for
Tom> a whole range of syscalls at once.  It is analogous, I think, to
Tom> having a breakpoint with multiple locations.

Sérgio> I have a question right here, then. Is the "breakpoint with multiple
Sérgio> locations" implemented this way? I'm sorry for my ignorance on this :-(

Yeah.  That might not be the best example, since there is also
"rbreak", which makes lots of separate breakpoints.

Sérgio> I was talking to Thiago about this, and I don't know if we need this
Sérgio> *right now*, for this patch. I think it's good the way it is, and as you
Sérgio> said, it won't take much to modify things in order to make it work the
Sérgio> way you want :-). Also, currently the "catch syscall" command doesn't
Sérgio> allow the user to set commands, conditions, etc. So for now it's only
Sérgio> "catch syscall [name|number]".

Sérgio> What do you think?

I'd still like it, for a long term UI reason.  If "catch syscall"
makes separate catch points for each syscall, then when we move to
froggy we won't be able to take advantage of the kernel-side
filtering, because that would be a change to how the command works.
We would probably have to invent a new user command.

Another option would be to make "catch syscall" only take a single
argument for now.  Then we could extend it gracefully in the future.

Also, can't you set conditions and commands on syscall catchpoints
using the "condition" and "commands" commands?  I would have thought
that came for free -- but I didn't try it and I didn't read the code
closely enough to know for sure.

Sérgio> Thanks for your comments :-).

Thanks for writing this and for persevering :)

Tom


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-07 12:12     ` Pedro Alves
  2008-11-07 13:30       ` Daniel Jacobowitz
@ 2008-11-08 15:35       ` Sérgio Durigan Júnior
  1 sibling, 0 replies; 57+ messages in thread
From: Sérgio Durigan Júnior @ 2008-11-08 15:35 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Olá Pedro,

On Fri, 2008-11-07 at 12:06 +0000, Pedro Alves wrote: 
> If there any other way to distinguish entry/exit other than a toggle?
> Toggles are prone to be fallible.  E.g., if you *attach* to a program that
> is doing a long syscall, and then start tracing that syscall,
> is it possible that you hit the syscall exit first, so your toggle will
> be inverted?  That is, you'll report a syscall entry, when in fact, it
> was a syscall exit, and so on for the following syscalls of the same lwp.

The way the patch is designed now, GDB will have exactly this behaviour
(inverting syscall entry/exit). If you can think of a better way to do
it so that GDB doesn't get confused, I'd be glad to implement.

Thanks,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-11-05 13:34           ` Sérgio Durigan Júnior
  2008-11-05 18:42             ` Eli Zaretskii
@ 2008-11-08 19:31             ` Mark Kettenis
  1 sibling, 0 replies; 57+ messages in thread
From: Mark Kettenis @ 2008-11-08 19:31 UTC (permalink / raw)
  To: sergiodj; +Cc: eliz, drow, bauerman, gdb-patches

> From: =?ISO-8859-1?Q?S=E9rgio?= Durigan =?ISO-8859-1?Q?J=FAnior?= <sergiodj@linux.vnet.ibm.com>
> Date: Wed, 05 Nov 2008 11:33:41 -0200
> 
> On Wed, 2008-11-05 at 06:18 +0200, Eli Zaretskii wrote:
> 
> > I'd like us very much to have some higher-level abstraction of a
> > syscall in target-independent code, than just a number.  Can we do
> > that, please?
> 
> I'm sorry about my bad design assumptions, but that just sounded good
> for me by the time I was developing. I think we can do the higher-level
> abstraction that you are asking, but I'd like you to please describe in
> more details how this abstraction would be, or even better, if that's
> not asking too much you could take the code I did and implement
> something better :-).
> 
> I'm sorry about these assumptions I've made, again.

I think the assumption that system calls are numbered is pretty
reasonable.  I have yet to encounter an OS that doesn't have them.


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part
  2008-10-16 15:17                   ` Daniel Jacobowitz
@ 2008-10-16 16:28                     ` Joel Brobecker
  0 siblings, 0 replies; 57+ messages in thread
From: Joel Brobecker @ 2008-10-16 16:28 UTC (permalink / raw)
  To: Sérgio Durigan Júnior, gdb-patches

> On Thu, Oct 16, 2008 at 08:36:50AM -0400, Daniel Jacobowitz wrote:
> > On Thu, Oct 16, 2008 at 12:35:50AM -0200, Sérgio Durigan Júnior wrote:
> > > Well, I took your patch and ran the testsuite here for both PPC and
> > > PPC64 archs. Things seem to be OK! I wasn't able to reproduce this
> > > regression you told; (I don't think so, but just in case...) maybe it's
> > > an x86 issue? Or maybe this particular test is non-deterministic?
> > 
> > I know I've seen random failures from this test before.  I'll try the
> > patch on x86.
> 
> Looks fine here too.

(relief). Thank you, guys. Patch now checked in. I intend to convert
the remaining ones when I have a moment.

Thanks again,
-- 
Joel


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-10-16 12:37                 ` Daniel Jacobowitz
@ 2008-10-16 15:17                   ` Daniel Jacobowitz
  2008-10-16 16:28                     ` Joel Brobecker
  0 siblings, 1 reply; 57+ messages in thread
From: Daniel Jacobowitz @ 2008-10-16 15:17 UTC (permalink / raw)
  To: Sérgio Durigan Júnior, Joel Brobecker, gdb-patches

On Thu, Oct 16, 2008 at 08:36:50AM -0400, Daniel Jacobowitz wrote:
> On Thu, Oct 16, 2008 at 12:35:50AM -0200, Sérgio Durigan Júnior wrote:
> > Well, I took your patch and ran the testsuite here for both PPC and
> > PPC64 archs. Things seem to be OK! I wasn't able to reproduce this
> > regression you told; (I don't think so, but just in case...) maybe it's
> > an x86 issue? Or maybe this particular test is non-deterministic?
> 
> I know I've seen random failures from this test before.  I'll try the
> patch on x86.

Looks fine here too.

-- 
Daniel Jacobowitz
CodeSourcery


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-10-16  3:35               ` Sérgio Durigan Júnior
@ 2008-10-16 12:37                 ` Daniel Jacobowitz
  2008-10-16 15:17                   ` Daniel Jacobowitz
  0 siblings, 1 reply; 57+ messages in thread
From: Daniel Jacobowitz @ 2008-10-16 12:37 UTC (permalink / raw)
  To: Sérgio Durigan Júnior; +Cc: Joel Brobecker, gdb-patches

On Thu, Oct 16, 2008 at 12:35:50AM -0200, Sérgio Durigan Júnior wrote:
> Well, I took your patch and ran the testsuite here for both PPC and
> PPC64 archs. Things seem to be OK! I wasn't able to reproduce this
> regression you told; (I don't think so, but just in case...) maybe it's
> an x86 issue? Or maybe this particular test is non-deterministic?

I know I've seen random failures from this test before.  I'll try the
patch on x86.

-- 
Daniel Jacobowitz
CodeSourcery


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-10-15  5:40             ` Joel Brobecker
@ 2008-10-16  3:35               ` Sérgio Durigan Júnior
  2008-10-16 12:37                 ` Daniel Jacobowitz
  0 siblings, 1 reply; 57+ messages in thread
From: Sérgio Durigan Júnior @ 2008-10-16  3:35 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

Hi Joel,

On Tue, 2008-10-14 at 22:40 -0700, Joel Brobecker wrote:

> So, Sergio, here is what I'm proposing: If you are in a hurry, then
> can you investigate the reason for the regressions? It shouldn't be
> too difficult.  Once fixed and approved, you can then start building
> your patch on top of mine.  If you're not in a hurry, then I'll pick
> it up again when I have some free time again.

Thanks a lot for your patch! I know you must be totally busy now, so
thanks again for taking your time to do this.

Well, I took your patch and ran the testsuite here for both PPC and
PPC64 archs. Things seem to be OK! I wasn't able to reproduce this
regression you told; (I don't think so, but just in case...) maybe it's
an x86 issue? Or maybe this particular test is non-deterministic?

Anyway, if you (or anybody) could give me more pointers to this I'd
appreciate :-).

Regards,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part
  2008-10-12  2:26           ` Sérgio Durigan Júnior
@ 2008-10-15  5:40             ` Joel Brobecker
  2008-10-16  3:35               ` Sérgio Durigan Júnior
  0 siblings, 1 reply; 57+ messages in thread
From: Joel Brobecker @ 2008-10-15  5:40 UTC (permalink / raw)
  To: Sérgio Durigan Júnior; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 3338 bytes --]

Thanks, Daniel, for the thorough review. I was only hoping for a quick
look at the principle, so thanks again.

Here is a new version of the patch, with all corrections made, including
a typo in a comment that Pedro noticed and emailed me about. I was going
to check it in, but I re-ran the testsuite, and this time, I'm having
unexpected timeouts in multi-forks.exp. Bad luck, something personal
came up, and I won't have time to look further into this.

> So, Daniel already reviewed your patch :-). As my rework on catch
> syscall heavily depends on your patch, I'm waiting until it gets pushed
> upstream so that I can start to modify things here.

So, Sergio, here is what I'm proposing: If you are in a hurry, then
can you investigate the reason for the regressions? It shouldn't be
too difficult.  Once fixed and approved, you can then start building
your patch on top of mine.  If you're not in a hurry, then I'll pick
it up again when I have some free time again.

2008-10-14  Joel Brobecker  <brobecker@adacore.com>

        * breakpoint.h (enum bptype): New enum bp_catchpoint.
        Delete bp_catch_fork and bp_catch_vfork.
        (struct breakpoint_ops): Add new methods "insert", "remove"
        and "breakpoint_hit".
        * breakpoint.c (create_fork_vfork_event_catchpoint)
        (create_fork_event_catchpoint, create_vfork_event_catchpoint): Remove.
        (insert_catchpoint): Remove handling of bp_catch_fork and
        bp_catch_vfork catchpoints, and handle them as bp_catchpoint
        catchpoints instead.
        (insert_bp_location, update_breakpoints_after_exec)
        (remove_breakpoint, bpstat_check_location, bpstat_what)
        (allocate_bp_location): Likewise.
        (print_it_typical, print_one_breakpoint_location, mention): Remove
        handling of bp_catch_fork and bp_catch_vfork breakpoints.
        (ep_is_catchpoint, user_settable_breakpoint)
        (breakpoint_address_is_meaningful, adjust_breakpoint_address)
        (breakpoint_re_set_one, disable_command, enable_command):
        Remove use of bp_catch_fork and bp_catch_vfork.  Add handling of
        bp_catchpoint breakpoints.
        (insert_catch_fork, remove_catch_fork, breakpoint_hit_catch_fork)
        (print_it_catch_fork, print_one_catch_fork, print_mention_catch_fork):
        New functions.
        (catch_fork_breakpoint_ops): New static constant.
        (insert_catch_vfork, remove_catch_vfork, breakpoint_hit_catch_vfork)
        (print_it_catch_vfork, print_one_catch_vfork)
        (print_mention_catch_vfork): New functions.
        (catch_vfork_breakpoint_ops): New static constant.
        (create_catchpoint, create_fork_vfork_event_catchpoint): New functions.
        (catch_fork_command_1): Use create_fork_vfork_event_catchpoint
        to create the fork and vfork catchpoints.
        (gnu_v3_exception_catchpoint_ops): Set new breakpoint_ops fields.
        * ada-lang.c (catch_exception_breakpoint_ops): Set new breakpoint_ops
        fields.
        (catch_exception_unhandled_breakpoint_ops): Likewise.
        (catch_assert_breakpoint_ops): Likewise.

2008-10-06  Joel Brobecker  <brobecker@adacore.com>

        * gdb.base/foll-fork.exp: Adjust the expected output to match
        the new description for fork/vfork catchpoints in the "info
        breakpoints" output.

Cheers,
-- 
Joel

[-- Attachment #2: catchpoint.diff --]
[-- Type: text/plain, Size: 21203 bytes --]

Index: breakpoint.h
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.h,v
retrieving revision 1.80
diff -u -p -r1.80 breakpoint.h
--- breakpoint.h	14 Oct 2008 20:49:02 -0000	1.80
+++ breakpoint.h	15 Oct 2008 04:33:21 -0000
@@ -110,6 +110,8 @@ enum bptype
 
     bp_overlay_event, 
 
+    bp_catchpoint,
+
     /* These breakpoints are used to implement the "catch load" command
        on platforms whose dynamic linkers support such functionality.  */
     bp_catch_load,
@@ -124,8 +126,6 @@ enum bptype
        kernels which can raise an event when a fork or exec occurs, as
        opposed to the debugger setting breakpoints on functions named
        "fork" or "exec".) */
-    bp_catch_fork,
-    bp_catch_vfork,
     bp_catch_exec,
   };
 
@@ -315,6 +315,19 @@ struct bp_location
 
 struct breakpoint_ops 
 {
+  /* Insert the breakpoint or activate the catchpoint.  Should raise
+     an exception if the operation failed.  */
+  void (*insert) (struct breakpoint *);
+
+  /* Remove the breakpoint/catchpoint that was previously inserted
+     with the "insert" method above.  Return non-zero if the operation
+     succeeded.  */
+  int (*remove) (struct breakpoint *);
+
+  /* Return non-zero if the debugger should tell the user that this
+     breakpoint was hit.  */
+  int (*breakpoint_hit) (struct breakpoint *);
+
   /* The normal print routine for this breakpoint, called when we
      hit it.  */
   enum print_stop_action (*print_it) (struct breakpoint *);
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.353
diff -u -p -r1.353 breakpoint.c
--- breakpoint.c	14 Oct 2008 20:49:02 -0000	1.353
+++ breakpoint.c	15 Oct 2008 04:33:24 -0000
@@ -159,10 +159,6 @@ static void awatch_command (char *, int)
 
 static void do_enable_breakpoint (struct breakpoint *, enum bpdisp);
 
-static void create_fork_vfork_event_catchpoint (int tempflag,
-						char *cond_string,
-						enum bptype bp_kind);
-
 static void stop_command (char *arg, int from_tty);
 
 static void stopin_command (char *arg, int from_tty);
@@ -797,11 +793,9 @@ insert_catchpoint (struct ui_out *uo, vo
 
   switch (b->type)
     {
-    case bp_catch_fork:
-      target_insert_fork_catchpoint (PIDGET (inferior_ptid));
-      break;
-    case bp_catch_vfork:
-      target_insert_vfork_catchpoint (PIDGET (inferior_ptid));
+    case bp_catchpoint:
+      gdb_assert (b->ops != NULL && b->ops->insert != NULL);
+      b->ops->insert (b);
       break;
     case bp_catch_exec:
       target_insert_exec_catchpoint (PIDGET (inferior_ptid));
@@ -1244,8 +1238,7 @@ Note: automatically using hardware break
       bpt->inserted = (val != -1);
     }
 
-  else if (bpt->owner->type == bp_catch_fork
-	   || bpt->owner->type == bp_catch_vfork
+  else if (bpt->owner->type == bp_catchpoint
 	   || bpt->owner->type == bp_catch_exec)
     {
       struct gdb_exception e = catch_exception (uiout, insert_catchpoint,
@@ -1501,17 +1494,18 @@ update_breakpoints_after_exec (void)
 	continue;
       }
 
-    /* Don't delete an exec catchpoint, because else the inferior
-       won't stop when it ought!
+    if (b->type == bp_catchpoint)
+      {
+        /* For now, none of the bp_catchpoint breakpoints need to
+           do anything at this point.  In the future, if some of
+           the catchpoints need to something, we will need to add
+           a new method, and call this method from here.  */
+        continue;
+      }
 
-       Similarly, we probably ought to keep vfork catchpoints, 'cause
-       on this target, we may not be able to stop when the vfork is
-       seen, but only when the subsequent exec is seen.  (And because
-       deleting fork catchpoints here but not vfork catchpoints will
-       seem mysterious to users, keep those too.)  */
-    if ((b->type == bp_catch_exec) ||
-	(b->type == bp_catch_vfork) ||
-	(b->type == bp_catch_fork))
+    /* Don't delete an exec catchpoint, because else the inferior
+       won't stop when it ought!  */
+    if (b->type == bp_catch_exec)
       {
 	continue;
       }
@@ -1686,21 +1680,24 @@ remove_breakpoint (struct bp_location *b
 	warning (_("Could not remove hardware watchpoint %d."),
 		 b->owner->number);
     }
-  else if ((b->owner->type == bp_catch_fork ||
-	    b->owner->type == bp_catch_vfork ||
-	    b->owner->type == bp_catch_exec)
+  else if (b->owner->type == bp_catchpoint
+           && breakpoint_enabled (b->owner)
+           && !b->duplicate)
+    {
+      gdb_assert (b->owner->ops != NULL && b->owner->ops->remove != NULL);
+
+      val = b->owner->ops->remove (b->owner);
+      if (val)
+	return val;
+      b->inserted = (is == mark_inserted);
+    }
+  else if (b->owner->type == bp_catch_exec
 	   && breakpoint_enabled (b->owner)
 	   && !b->duplicate)
     {
       val = -1;
       switch (b->owner->type)
 	{
-	case bp_catch_fork:
-	  val = target_remove_fork_catchpoint (PIDGET (inferior_ptid));
-	  break;
-	case bp_catch_vfork:
-	  val = target_remove_vfork_catchpoint (PIDGET (inferior_ptid));
-	  break;
 	case bp_catch_exec:
 	  val = target_remove_exec_catchpoint (PIDGET (inferior_ptid));
 	  break;
@@ -1948,11 +1945,9 @@ breakpoint_thread_match (CORE_ADDR pc, p
 int
 ep_is_catchpoint (struct breakpoint *ep)
 {
-  return
-    (ep->type == bp_catch_load)
+  return (ep->type == bp_catchpoint)
+    || (ep->type == bp_catch_load)
     || (ep->type == bp_catch_unload)
-    || (ep->type == bp_catch_fork)
-    || (ep->type == bp_catch_vfork)
     || (ep->type == bp_catch_exec);
 
   /* ??rehrauer: Add more kinds here, as are implemented... */
@@ -2347,22 +2342,6 @@ print_it_typical (bpstat bs)
       return PRINT_SRC_AND_LOC;
       break;
 
-    case bp_catch_fork:
-      annotate_catchpoint (b->number);
-      printf_filtered (_("\nCatchpoint %d (forked process %d), "),
-		       b->number, 
-		       ptid_get_pid (b->forked_inferior_pid));
-      return PRINT_SRC_AND_LOC;
-      break;
-
-    case bp_catch_vfork:
-      annotate_catchpoint (b->number);
-      printf_filtered (_("\nCatchpoint %d (vforked process %d), "),
-		       b->number, 
-		       ptid_get_pid (b->forked_inferior_pid));
-      return PRINT_SRC_AND_LOC;
-      break;
-
     case bp_catch_exec:
       annotate_catchpoint (b->number);
       printf_filtered (_("\nCatchpoint %d (exec'd %s), "),
@@ -2789,8 +2768,7 @@ bpstat_check_location (const struct bp_l
       && b->type != bp_read_watchpoint
       && b->type != bp_access_watchpoint
       && b->type != bp_hardware_breakpoint
-      && b->type != bp_catch_fork
-      && b->type != bp_catch_vfork
+      && b->type != bp_catchpoint
       && b->type != bp_catch_exec)	/* a non-watchpoint bp */
     {
       if (bl->address != bp_addr) 	/* address doesn't match */
@@ -2823,7 +2801,7 @@ bpstat_check_location (const struct bp_l
 	  && !section_is_mapped (bl->section))
 	return 0;
     }
-  
+
   /* Is this a catchpoint of a load or unload?  If so, did we
      get a load or unload of the specified library?  If not,
      ignore it. */
@@ -2851,16 +2829,13 @@ bpstat_check_location (const struct bp_l
       )
     return 0;
 
-  if ((b->type == bp_catch_fork)
-      && !inferior_has_forked (inferior_ptid,
-			       &b->forked_inferior_pid))
-    return 0;
-  
-  if ((b->type == bp_catch_vfork)
-      && !inferior_has_vforked (inferior_ptid,
-				&b->forked_inferior_pid))
-    return 0;
-  
+  if (b->type == bp_catchpoint)
+    {
+      gdb_assert (b->ops != NULL && b->ops->breakpoint_hit != NULL);
+      if (!b->ops->breakpoint_hit (b))
+        return 0;
+    }
+     
   if ((b->type == bp_catch_exec)
       && !inferior_has_execd (inferior_ptid, &b->exec_pathname))
     return 0;
@@ -3389,8 +3364,7 @@ bpstat_what (bpstat bs)
 	  else
 	    bs_class = no_effect;
 	  break;
-	case bp_catch_fork:
-	case bp_catch_vfork:
+	case bp_catchpoint:
 	case bp_catch_exec:
 	  if (bs->stop)
 	    {
@@ -3569,10 +3543,9 @@ print_one_breakpoint_location (struct br
     {bp_shlib_event, "shlib events"},
     {bp_thread_event, "thread events"},
     {bp_overlay_event, "overlay events"},
+    {bp_catchpoint, "catchpoint"},
     {bp_catch_load, "catch load"},
     {bp_catch_unload, "catch unload"},
-    {bp_catch_fork, "catch fork"},
-    {bp_catch_vfork, "catch vfork"},
     {bp_catch_exec, "catch exec"}
   };
   
@@ -3707,23 +3680,6 @@ print_one_breakpoint_location (struct br
 	  }
 	break;
 
-      case bp_catch_fork:
-      case bp_catch_vfork:
-	/* 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 (addressprint)
-	  ui_out_field_skip (uiout, "addr");
-	annotate_field (5);
-	if (!ptid_equal (b->forked_inferior_pid, null_ptid))
-	  {
-	    ui_out_text (uiout, "process ");
-	    ui_out_field_int (uiout, "what",
-			      ptid_get_pid (b->forked_inferior_pid));
-	    ui_out_spaces (uiout, 1);
-	  }
-	break;
-
       case bp_catch_exec:
 	/* Field 4, the address, is omitted (which makes the columns
 	   not line up too nicely with the headers, but the effect
@@ -3935,10 +3891,9 @@ static int
 user_settable_breakpoint (const struct breakpoint *b)
 {
   return (b->type == bp_breakpoint
+	  || b->type == bp_catchpoint
 	  || b->type == bp_catch_load
 	  || b->type == bp_catch_unload
-	  || b->type == bp_catch_fork
-	  || b->type == bp_catch_vfork
 	  || b->type == bp_catch_exec
 	  || b->type == bp_hardware_breakpoint
 	  || b->type == bp_watchpoint
@@ -4146,9 +4101,8 @@ set_default_breakpoint (int valid, CORE_
       bp_hardware_watchpoint
       bp_read_watchpoint
       bp_access_watchpoint
-      bp_catch_exec
-      bp_catch_fork
-      bp_catch_vork */
+      bp_catchpoint
+      bp_catch_exec */
 
 static int
 breakpoint_address_is_meaningful (struct breakpoint *bpt)
@@ -4159,9 +4113,8 @@ breakpoint_address_is_meaningful (struct
 	  && type != bp_hardware_watchpoint
 	  && type != bp_read_watchpoint
 	  && type != bp_access_watchpoint
-	  && type != bp_catch_exec
-	  && type != bp_catch_fork
-	  && type != bp_catch_vfork);
+	  && type != bp_catchpoint
+	  && type != bp_catch_exec);
 }
 
 /* Rescan breakpoints at the same address and section as BPT,
@@ -4276,8 +4229,7 @@ adjust_breakpoint_address (CORE_ADDR bpa
            || bptype == bp_hardware_watchpoint
            || bptype == bp_read_watchpoint
            || bptype == bp_access_watchpoint
-           || bptype == bp_catch_fork
-           || bptype == bp_catch_vfork
+           || bptype == bp_catchpoint
            || bptype == bp_catch_exec)
     {
       /* Watchpoints and the various bp_catch_* eventpoints should not
@@ -4344,8 +4296,7 @@ allocate_bp_location (struct breakpoint 
       loc->loc_type = bp_loc_hardware_watchpoint;
       break;
     case bp_watchpoint:
-    case bp_catch_fork:
-    case bp_catch_vfork:
+    case bp_catchpoint:
     case bp_catch_exec:
       loc->loc_type = bp_loc_other;
       break;
@@ -4746,45 +4697,210 @@ disable_breakpoints_in_unloaded_shlib (s
   }
 }
 
+/* FORK & VFORK catchpoints.  */
+
+/* Implement the "insert" breakpoint_ops method for fork catchpoints.  */
+
 static void
-create_fork_vfork_event_catchpoint (int tempflag, char *cond_string,
-				    enum bptype bp_kind)
+insert_catch_fork (struct breakpoint *b)
+{
+  target_insert_fork_catchpoint (PIDGET (inferior_ptid));
+}
+
+/* Implement the "remove" breakpoint_ops method for fork catchpoints.  */
+
+static int
+remove_catch_fork (struct breakpoint *b)
+{
+  return target_remove_fork_catchpoint (PIDGET (inferior_ptid));
+}
+
+/* Implement the "breakpoint_hit" breakpoint_ops method for fork
+   catchpoints.  */
+
+static int
+breakpoint_hit_catch_fork (struct breakpoint *b)
+{
+  return inferior_has_forked (inferior_ptid, &b->forked_inferior_pid);
+}
+
+/* Implement the "print_it" breakpoint_ops method for fork catchpoints.  */
+
+static enum print_stop_action
+print_it_catch_fork (struct breakpoint *b)
+{
+  annotate_catchpoint (b->number);
+  printf_filtered (_("\nCatchpoint %d (forked process %d), "),
+		   b->number, ptid_get_pid (b->forked_inferior_pid));
+  return PRINT_SRC_AND_LOC;
+}
+
+/* Implement the "print_one" breakpoint_ops method for fork catchpoints.  */
+
+static void
+print_one_catch_fork (struct breakpoint *b, CORE_ADDR *last_addr)
+{
+  /* 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 (addressprint)
+    ui_out_field_skip (uiout, "addr");
+  annotate_field (5);
+  ui_out_text (uiout, "fork");
+  if (!ptid_equal (b->forked_inferior_pid, null_ptid))
+    {
+      ui_out_text (uiout, ", process ");
+      ui_out_field_int (uiout, "what",
+                        ptid_get_pid (b->forked_inferior_pid));
+      ui_out_spaces (uiout, 1);
+    }
+}
+
+/* Implement the "print_mention" breakpoint_ops method for fork
+   catchpoints.  */
+
+static void
+print_mention_catch_fork (struct breakpoint *b)
+{
+  printf_filtered (_("Catchpoint %d (fork)"), b->number);
+}
+
+/* The breakpoint_ops structure to be used in fork catchpoints.  */
+
+static struct breakpoint_ops catch_fork_breakpoint_ops =
+{
+  insert_catch_fork,
+  remove_catch_fork,
+  breakpoint_hit_catch_fork,
+  print_it_catch_fork,
+  print_one_catch_fork,
+  print_mention_catch_fork
+};
+
+/* Implement the "insert" breakpoint_ops method for vfork catchpoints.  */
+
+static void
+insert_catch_vfork (struct breakpoint *b)
+{
+  target_insert_vfork_catchpoint (PIDGET (inferior_ptid));
+}
+
+/* Implement the "remove" breakpoint_ops method for vfork catchpoints.  */
+
+static int
+remove_catch_vfork (struct breakpoint *b)
+{
+  return target_remove_vfork_catchpoint (PIDGET (inferior_ptid));
+}
+
+/* Implement the "breakpoint_hit" breakpoint_ops method for vfork
+   catchpoints.  */
+
+static int
+breakpoint_hit_catch_vfork (struct breakpoint *b)
+{
+  return inferior_has_vforked (inferior_ptid, &b->forked_inferior_pid);
+}
+
+/* Implement the "print_it" breakpoint_ops method for vfork catchpoints.  */
+
+static enum print_stop_action
+print_it_catch_vfork (struct breakpoint *b)
+{
+  annotate_catchpoint (b->number);
+  printf_filtered (_("\nCatchpoint %d (vforked process %d), "),
+		   b->number, ptid_get_pid (b->forked_inferior_pid));
+  return PRINT_SRC_AND_LOC;
+}
+
+/* Implement the "print_one" breakpoint_ops method for vfork catchpoints.  */
+
+static void
+print_one_catch_vfork (struct breakpoint *b, CORE_ADDR *last_addr)
+{
+  /* 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 (addressprint)
+    ui_out_field_skip (uiout, "addr");
+  annotate_field (5);
+  ui_out_text (uiout, "vfork");
+  if (!ptid_equal (b->forked_inferior_pid, null_ptid))
+    {
+      ui_out_text (uiout, ", process ");
+      ui_out_field_int (uiout, "what",
+                        ptid_get_pid (b->forked_inferior_pid));
+      ui_out_spaces (uiout, 1);
+    }
+}
+
+/* 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);
+}
+
+/* The breakpoint_ops structure to be used in vfork catchpoints.  */
+
+static struct breakpoint_ops catch_vfork_breakpoint_ops =
+{
+  insert_catch_vfork,
+  remove_catch_vfork,
+  breakpoint_hit_catch_vfork,
+  print_it_catch_vfork,
+  print_one_catch_vfork,
+  print_mention_catch_vfork
+};
+
+/* Create a new breakpoint of the bp_catchpoint kind and return it.
+ 
+   If TEMPFLAG is non-zero, then make the breakpoint temporary.
+   If COND_STRING is not NULL, then store it in the breakpoint.
+   OPS, if not NULL, is the breakpoint_ops structure associated
+   to the catchpoint.  */
+
+static struct breakpoint *
+create_catchpoint (int tempflag, char *cond_string,
+                   struct breakpoint_ops *ops)
 {
   struct symtab_and_line sal;
   struct breakpoint *b;
-  int thread = -1;		/* All threads. */
 
   init_sal (&sal);
   sal.pc = 0;
   sal.symtab = NULL;
   sal.line = 0;
 
-  b = set_raw_breakpoint (sal, bp_kind);
+  b = set_raw_breakpoint (sal, bp_catchpoint);
   set_breakpoint_count (breakpoint_count + 1);
   b->number = breakpoint_count;
+
   b->cond_string = (cond_string == NULL) ? 
     NULL : savestring (cond_string, strlen (cond_string));
-  b->thread = thread;
+  b->thread = -1;
   b->addr_string = NULL;
   b->enable_state = bp_enabled;
   b->disposition = tempflag ? disp_del : disp_donttouch;
-  b->forked_inferior_pid = null_ptid;
-  update_global_location_list (1);
-
+  b->ops = ops;
 
   mention (b);
-}
+  update_global_location_list (1);
 
-static void
-create_fork_event_catchpoint (int tempflag, char *cond_string)
-{
-  create_fork_vfork_event_catchpoint (tempflag, cond_string, bp_catch_fork);
+  return b;
 }
 
 static void
-create_vfork_event_catchpoint (int tempflag, char *cond_string)
+create_fork_vfork_event_catchpoint (int tempflag, char *cond_string,
+                                    struct breakpoint_ops *ops)
 {
-  create_fork_vfork_event_catchpoint (tempflag, cond_string, bp_catch_vfork);
+  struct breakpoint *b = create_catchpoint (tempflag, cond_string, ops);
+
+  /* FIXME: We should put this information in a breakpoint private data
+     area.  */
+  b->forked_inferior_pid = null_ptid;
 }
 
 static void
@@ -5020,12 +5136,6 @@ mention (struct breakpoint *b)
 			 (b->dll_pathname != NULL) ? 
 			 b->dll_pathname : "<any library>");
 	break;
-      case bp_catch_fork:
-      case bp_catch_vfork:
-	printf_filtered (_("Catchpoint %d (%s)"),
-			 b->number,
-			 (b->type == bp_catch_fork) ? "fork" : "vfork");
-	break;
       case bp_catch_exec:
 	printf_filtered (_("Catchpoint %d (exec)"),
 			 b->number);
@@ -6446,11 +6556,13 @@ catch_fork_command_1 (char *arg, int fro
     {
     case catch_fork_temporary:
     case catch_fork_permanent:
-      create_fork_event_catchpoint (tempflag, cond_string);
+      create_fork_vfork_event_catchpoint (tempflag, cond_string,
+                                          &catch_fork_breakpoint_ops);
       break;
     case catch_vfork_temporary:
     case catch_vfork_permanent:
-      create_vfork_event_catchpoint (tempflag, cond_string);
+      create_fork_vfork_event_catchpoint (tempflag, cond_string,
+                                          &catch_vfork_breakpoint_ops);
       break;
     default:
       error (_("unsupported or unknown fork kind; cannot catch it"));
@@ -6647,6 +6759,9 @@ print_mention_exception_catchpoint (stru
 }
 
 static struct breakpoint_ops gnu_v3_exception_catchpoint_ops = {
+  NULL, /* insert */
+  NULL, /* remove */
+  NULL, /* breakpoint_hit */
   print_exception_catchpoint,
   print_one_exception_catchpoint,
   print_mention_exception_catchpoint
@@ -7613,8 +7728,7 @@ breakpoint_re_set_one (void *bint)
       /* We needn't really do anything to reset these, since the mask
          that requests them is unaffected by e.g., new libraries being
          loaded. */
-    case bp_catch_fork:
-    case bp_catch_vfork:
+    case bp_catchpoint:
     case bp_catch_exec:
       break;
 
@@ -7874,10 +7988,9 @@ disable_command (char *args, int from_tt
 		 bpt->number);
 	continue;
       case bp_breakpoint:
+      case bp_catchpoint:
       case bp_catch_load:
       case bp_catch_unload:
-      case bp_catch_fork:
-      case bp_catch_vfork:
       case bp_catch_exec:
       case bp_hardware_breakpoint:
       case bp_watchpoint:
@@ -8008,10 +8121,9 @@ enable_command (char *args, int from_tty
 		 bpt->number);
 	continue;
       case bp_breakpoint:
+      case bp_catchpoint:
       case bp_catch_load:
       case bp_catch_unload:
-      case bp_catch_fork:
-      case bp_catch_vfork:
       case bp_catch_exec:
       case bp_hardware_breakpoint:
       case bp_watchpoint:
Index: ada-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/ada-lang.c,v
retrieving revision 1.178
diff -u -p -r1.178 ada-lang.c
--- ada-lang.c	7 Oct 2008 14:07:10 -0000	1.178
+++ ada-lang.c	15 Oct 2008 04:52:30 -0000
@@ -10152,6 +10152,9 @@ print_mention_catch_exception (struct br
 
 static struct breakpoint_ops catch_exception_breakpoint_ops =
 {
+  NULL, /* insert */
+  NULL, /* remove */
+  NULL, /* breakpoint_hit */
   print_it_catch_exception,
   print_one_catch_exception,
   print_mention_catch_exception
@@ -10178,6 +10181,9 @@ print_mention_catch_exception_unhandled 
 }
 
 static struct breakpoint_ops catch_exception_unhandled_breakpoint_ops = {
+  NULL, /* insert */
+  NULL, /* remove */
+  NULL, /* breakpoint_hit */
   print_it_catch_exception_unhandled,
   print_one_catch_exception_unhandled,
   print_mention_catch_exception_unhandled
@@ -10204,6 +10210,9 @@ print_mention_catch_assert (struct break
 }
 
 static struct breakpoint_ops catch_assert_breakpoint_ops = {
+  NULL, /* insert */
+  NULL, /* remove */
+  NULL, /* breakpoint_hit */
   print_it_catch_assert,
   print_one_catch_assert,
   print_mention_catch_assert

[-- Attachment #3: catchpoint-ex.diff --]
[-- Type: text/plain, Size: 1820 bytes --]

diff -r c9da7894e8d5 gdb/testsuite/gdb.base/foll-fork.exp
--- a/gdb/testsuite/gdb.base/foll-fork.exp	Sun Oct 05 16:06:42 2008 -0700
+++ b/gdb/testsuite/gdb.base/foll-fork.exp	Mon Oct 06 13:05:03 2008 -0400
@@ -158,14 +158,11 @@ proc catch_fork_child_follow {} {
    # Verify that the catchpoint is mentioned in an "info breakpoints",
    # and further that the catchpoint mentions no process id.
    #
-   send_gdb "info breakpoints\n"
-   gdb_expect {
-     -re ".*catch fork.*keep y.*$gdb_prompt $"\
-                     {pass "info shows catchpoint without pid"}
-     -re ".*catch fork.*process .*$gdb_prompt $"\
-                     {fail "info shows catchpoint without pid"}
-     -re "$gdb_prompt $" {fail "info shows catchpoint without pid"}
-     timeout         {fail "(timeout) info shows catchpoint without pid"}
+   set test_name "info shows catchpoint without pid"
+   gdb_test_multiple "info breakpoints" "$test_name" {
+     -re ".*catchpoint.*keep y.*fork\[\r\n\]+$gdb_prompt $" {
+       pass "$test_name"
+     }
    }
 
    send_gdb "continue\n"
@@ -179,12 +176,11 @@ proc catch_fork_child_follow {} {
    # Verify that the catchpoint is mentioned in an "info breakpoints",
    # and further that the catchpoint managed to capture a process id.
    #
-   send_gdb "info breakpoints\n"
-   gdb_expect {
-     -re ".*catch fork .*process \[0-9\]+.*$gdb_prompt $"\
-                     {pass "info shows catchpoint pid"}
-     -re "$gdb_prompt $" {fail "info shows catchpoint pid"}
-     timeout         {fail "(timeout) info shows catchpoint pid"}
+   set test_name "info shows catchpoint without pid"
+   gdb_test_multiple "info breakpoints" "$test_name" {
+     -re ".*catchpoint.*keep y.*fork, process.*$gdb_prompt $" {
+       pass "$test_name"
+     }
    }
 
    send_gdb "set follow child\n"

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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-10-06 17:22         ` Joel Brobecker
  2008-10-10 13:12           ` Daniel Jacobowitz
  2008-10-10 15:28           ` Sérgio Durigan Júnior
@ 2008-10-12  2:26           ` Sérgio Durigan Júnior
  2008-10-15  5:40             ` Joel Brobecker
  2 siblings, 1 reply; 57+ messages in thread
From: Sérgio Durigan Júnior @ 2008-10-12  2:26 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

Hi Joel,

On Mon, 2008-10-06 at 13:21 -0400, Joel Brobecker wrote:
>   2. I think it should be possible to split the fork/vfork ops
>      out into a separate file.  But the fork/vfork bp_ops use
>      some static routines from breakpoint.c, and I don't necessarily
>      want these routines to be exported.  One solution would be to
>      include a .c file from inside breakpoint.c.  Not sure that
>      this is very elegant either.  For now, I elected to stay focused
>      on the conversion.  Just some thoughts maybe for later.

IMHO this is not an elegant choice, though I don't have any better
solution in mind now :-(

> OK to commit? I think on principle Daniel agreed, but the devil is
> often in the details...

So, Daniel already reviewed your patch :-). As my rework on catch
syscall heavily depends on your patch, I'm waiting until it gets pushed
upstream so that I can start to modify things here.

Thank you,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-10-06 17:22         ` Joel Brobecker
  2008-10-10 13:12           ` Daniel Jacobowitz
@ 2008-10-10 15:28           ` Sérgio Durigan Júnior
  2008-10-12  2:26           ` Sérgio Durigan Júnior
  2 siblings, 0 replies; 57+ messages in thread
From: Sérgio Durigan Júnior @ 2008-10-10 15:28 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

Hi Joel,

On Mon, 2008-10-06 at 13:21 -0400, Joel Brobecker wrote:
> > Meanwhile, I'll try to understand and improve things by my own :-).
> 
> Here is a patch that converts fork/vfork catchpoints to using
> breakpoint_ops. I introduces 3 new "methods" for inserting/removing/
> and breakpoint_hit, and should give you an idea of what the code
> should look like. For your feature, let's see if others agree to
> check it in. If it goes in, then you can leverage on the new kind.

I have executed a regression test with your patch, and it does not cause
any regressions for both PPC and PPC64. Thought it would be nice to
tell :-).

Regards,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-10-06 17:22         ` Joel Brobecker
@ 2008-10-10 13:12           ` Daniel Jacobowitz
  2008-10-10 15:28           ` Sérgio Durigan Júnior
  2008-10-12  2:26           ` Sérgio Durigan Júnior
  2 siblings, 0 replies; 57+ messages in thread
From: Daniel Jacobowitz @ 2008-10-10 13:12 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: S?rgio Durigan J?nior, gdb-patches

On Mon, Oct 06, 2008 at 01:21:36PM -0400, Joel Brobecker wrote:
> 2008-10-06  Joel Brobecker  <brobecker@adacore.com>
> 
>         * breakpoint.h (enum bptype): New enum bp_catchpoint.
>         Delete bp_catch_fork and bp_catch_vfork.
>         * breakpoint.c: Implement the catch fork/vfork feature
>         using the bp_catchpoint bptype enum and the breakpoint_ops
>         structure.  Remove the use of bp_catch_fork and bp_catch_vfork
>         breakpoint kinds.

I think this is sufficiently non-mechanical that we should be more
descriptive in the changelog.

> @@ -1505,6 +1498,14 @@ update_breakpoints_after_exec (void)
>  	continue;
>        }
>  
> +    if (b->type == bp_catchpoint)
> +      {
> +        /* For now, none of the bp_catchpoint breakpoints need to
> +           do anything at this point.  In the future, if some of
> +           the catchpoints need to something, we will need to add
> +           a new method, and call this method from here.  */
> +      }
> +
>      /* Don't delete an exec catchpoint, because else the inferior
>         won't stop when it ought!
>  
> @@ -1513,9 +1514,7 @@ update_breakpoints_after_exec (void)
>         seen, but only when the subsequent exec is seen.  (And because
>         deleting fork catchpoints here but not vfork catchpoints will
>         seem mysterious to users, keep those too.)  */
> -    if ((b->type == bp_catch_exec) ||
> -	(b->type == bp_catch_vfork) ||
> -	(b->type == bp_catch_fork))
> +    if (b->type == bp_catch_exec)
>        {
>  	continue;
>        }

This bit changes behavior (and the old comment doesn't make sense any
more).

> -	case bp_catch_fork:
> -	case bp_catch_vfork:
> +        case bp_catchpoint:

Spaces/tabs?

> @@ -3939,10 +3900,9 @@ user_settable_breakpoint (const struct b
>  user_settable_breakpoint (const struct breakpoint *b)
>  {
>    return (b->type == bp_breakpoint
> +          || b->type == bp_catchpoint
>  	  || b->type == bp_catch_load
>  	  || b->type == bp_catch_unload
> -	  || b->type == bp_catch_fork
> -	  || b->type == bp_catch_vfork

Spaces/tabs again.

> @@ -4163,9 +4123,8 @@ breakpoint_address_is_meaningful (struct
>  	  && type != bp_hardware_watchpoint
>  	  && type != bp_read_watchpoint
>  	  && type != bp_access_watchpoint
> -	  && type != bp_catch_exec
> -	  && type != bp_catch_fork
> -	  && type != bp_catch_vfork);
> +          && type != bp_catchpoint
> +	  && type != bp_catch_exec);
>  }
>  
>  /* Rescan breakpoints at the same address and section as BPT,

Likewise.

Anyway, that was all trivial.  It looks good to me.

-- 
Daniel Jacobowitz
CodeSourcery


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part
  2008-10-04 23:04       ` Sérgio Durigan Júnior
@ 2008-10-06 17:22         ` Joel Brobecker
  2008-10-10 13:12           ` Daniel Jacobowitz
                             ` (2 more replies)
  0 siblings, 3 replies; 57+ messages in thread
From: Joel Brobecker @ 2008-10-06 17:22 UTC (permalink / raw)
  To: Sérgio Durigan Júnior; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 2225 bytes --]

> Meanwhile, I'll try to understand and improve things by my own :-).

Here is a patch that converts fork/vfork catchpoints to using
breakpoint_ops. I introduces 3 new "methods" for inserting/removing/
and breakpoint_hit, and should give you an idea of what the code
should look like. For your feature, let's see if others agree to
check it in. If it goes in, then you can leverage on the new kind.

I would have gone ahead and converted exec as as well as shared-library
load/unload, since it's fairly mechanical and would be a welcome cleanup,
but I'm a bit short on time, and I want to continue submitting other
changes we made.  If this change looks ok to the others as well, I'll
try to convert more during my evenings after dinner.

2008-10-06  Joel Brobecker  <brobecker@adacore.com>

        * breakpoint.h (enum bptype): New enum bp_catchpoint.
        Delete bp_catch_fork and bp_catch_vfork.
        * breakpoint.c: Implement the catch fork/vfork feature
        using the bp_catchpoint bptype enum and the breakpoint_ops
        structure.  Remove the use of bp_catch_fork and bp_catch_vfork
        breakpoint kinds.

2008-10-06  Joel Brobecker  <brobecker@adacore.com>

        * gdb.base/foll-fork.exp: Adjust the expected output to match
        the new description for fork/vfork catchpoints in the "info
        breakpoints" output.

Tested on x86-linux.

A couple of side remarks:
  
  1. I think it would be useful to introduce a private-data field
     that the bp_ops functions would access if necessary.  We would
     be able to stuff the forked_inferior_pid in it, for instance.
     Not done here, to avoid mixing issues in the same patch.

  2. I think it should be possible to split the fork/vfork ops
     out into a separate file.  But the fork/vfork bp_ops use
     some static routines from breakpoint.c, and I don't necessarily
     want these routines to be exported.  One solution would be to
     include a .c file from inside breakpoint.c.  Not sure that
     this is very elegant either.  For now, I elected to stay focused
     on the conversion.  Just some thoughts maybe for later.

OK to commit? I think on principle Daniel agreed, but the devil is
often in the details...

-- 
Joel

[-- Attachment #2: catchpoint-fork.diff --]
[-- Type: text/plain, Size: 22440 bytes --]

diff -r c9da7894e8d5 gdb/ada-lang.c
--- a/gdb/ada-lang.c	Sun Oct 05 16:06:42 2008 -0700
+++ b/gdb/ada-lang.c	Mon Oct 06 13:05:03 2008 -0400
@@ -10145,6 +10145,9 @@ print_mention_catch_exception (struct br
 
 static struct breakpoint_ops catch_exception_breakpoint_ops =
 {
+  NULL, /* insert */
+  NULL, /* remove */
+  NULL, /* breakpoint_hit */
   print_it_catch_exception,
   print_one_catch_exception,
   print_mention_catch_exception
@@ -10171,6 +10174,9 @@ print_mention_catch_exception_unhandled 
 }
 
 static struct breakpoint_ops catch_exception_unhandled_breakpoint_ops = {
+  NULL, /* insert */
+  NULL, /* remove */
+  NULL, /* breakpoint_hit */
   print_it_catch_exception_unhandled,
   print_one_catch_exception_unhandled,
   print_mention_catch_exception_unhandled
@@ -10197,6 +10203,9 @@ print_mention_catch_assert (struct break
 }
 
 static struct breakpoint_ops catch_assert_breakpoint_ops = {
+  NULL, /* insert */
+  NULL, /* remove */
+  NULL, /* breakpoint_hit */
   print_it_catch_assert,
   print_one_catch_assert,
   print_mention_catch_assert
diff -r c9da7894e8d5 gdb/breakpoint.c
--- a/gdb/breakpoint.c	Sun Oct 05 16:06:42 2008 -0700
+++ b/gdb/breakpoint.c	Mon Oct 06 13:05:03 2008 -0400
@@ -159,10 +159,6 @@ static void awatch_command (char *, int)
 
 static void do_enable_breakpoint (struct breakpoint *, enum bpdisp);
 
-static void create_fork_vfork_event_catchpoint (int tempflag,
-						char *cond_string,
-						enum bptype bp_kind);
-
 static void stop_command (char *arg, int from_tty);
 
 static void stopin_command (char *arg, int from_tty);
@@ -801,11 +797,9 @@ insert_catchpoint (struct ui_out *uo, vo
 
   switch (b->type)
     {
-    case bp_catch_fork:
-      target_insert_fork_catchpoint (PIDGET (inferior_ptid));
-      break;
-    case bp_catch_vfork:
-      target_insert_vfork_catchpoint (PIDGET (inferior_ptid));
+    case bp_catchpoint:
+      gdb_assert (b->ops != NULL && b->ops->insert != NULL);
+      b->ops->insert (b);
       break;
     case bp_catch_exec:
       target_insert_exec_catchpoint (PIDGET (inferior_ptid));
@@ -1248,8 +1242,7 @@ Note: automatically using hardware break
       bpt->inserted = (val != -1);
     }
 
-  else if (bpt->owner->type == bp_catch_fork
-	   || bpt->owner->type == bp_catch_vfork
+  else if (bpt->owner->type == bp_catchpoint
 	   || bpt->owner->type == bp_catch_exec)
     {
       struct gdb_exception e = catch_exception (uiout, insert_catchpoint,
@@ -1505,6 +1498,14 @@ update_breakpoints_after_exec (void)
 	continue;
       }
 
+    if (b->type == bp_catchpoint)
+      {
+        /* For now, none of the bp_catchpoint breakpoints need to
+           do anything at this point.  In the future, if some of
+           the catchpoints need to something, we will need to add
+           a new method, and call this method from here.  */
+      }
+
     /* Don't delete an exec catchpoint, because else the inferior
        won't stop when it ought!
 
@@ -1513,9 +1514,7 @@ update_breakpoints_after_exec (void)
        seen, but only when the subsequent exec is seen.  (And because
        deleting fork catchpoints here but not vfork catchpoints will
        seem mysterious to users, keep those too.)  */
-    if ((b->type == bp_catch_exec) ||
-	(b->type == bp_catch_vfork) ||
-	(b->type == bp_catch_fork))
+    if (b->type == bp_catch_exec)
       {
 	continue;
       }
@@ -1690,21 +1689,24 @@ remove_breakpoint (struct bp_location *b
 	warning (_("Could not remove hardware watchpoint %d."),
 		 b->owner->number);
     }
-  else if ((b->owner->type == bp_catch_fork ||
-	    b->owner->type == bp_catch_vfork ||
-	    b->owner->type == bp_catch_exec)
+  else if (b->owner->type == bp_catchpoint
+           && breakpoint_enabled (b->owner)
+           && !b->duplicate)
+    {
+      gdb_assert (b->owner->ops != NULL && b->owner->ops->remove != NULL);
+
+      val = b->owner->ops->remove (b->owner);
+      if (val)
+	return val;
+      b->inserted = (is == mark_inserted);
+    }
+  else if (b->owner->type == bp_catch_exec
 	   && breakpoint_enabled (b->owner)
 	   && !b->duplicate)
     {
       val = -1;
       switch (b->owner->type)
 	{
-	case bp_catch_fork:
-	  val = target_remove_fork_catchpoint (PIDGET (inferior_ptid));
-	  break;
-	case bp_catch_vfork:
-	  val = target_remove_vfork_catchpoint (PIDGET (inferior_ptid));
-	  break;
 	case bp_catch_exec:
 	  val = target_remove_exec_catchpoint (PIDGET (inferior_ptid));
 	  break;
@@ -1952,11 +1954,9 @@ int
 int
 ep_is_catchpoint (struct breakpoint *ep)
 {
-  return
-    (ep->type == bp_catch_load)
+  return (ep->type == bp_catchpoint)
+    || (ep->type == bp_catch_load)
     || (ep->type == bp_catch_unload)
-    || (ep->type == bp_catch_fork)
-    || (ep->type == bp_catch_vfork)
     || (ep->type == bp_catch_exec);
 
   /* ??rehrauer: Add more kinds here, as are implemented... */
@@ -2348,22 +2348,6 @@ print_it_typical (bpstat bs)
       printf_filtered (_("\nCatchpoint %d (unloaded %s), "),
 		       b->number,
 		       b->triggered_dll_pathname);
-      return PRINT_SRC_AND_LOC;
-      break;
-
-    case bp_catch_fork:
-      annotate_catchpoint (b->number);
-      printf_filtered (_("\nCatchpoint %d (forked process %d), "),
-		       b->number, 
-		       ptid_get_pid (b->forked_inferior_pid));
-      return PRINT_SRC_AND_LOC;
-      break;
-
-    case bp_catch_vfork:
-      annotate_catchpoint (b->number);
-      printf_filtered (_("\nCatchpoint %d (vforked process %d), "),
-		       b->number, 
-		       ptid_get_pid (b->forked_inferior_pid));
       return PRINT_SRC_AND_LOC;
       break;
 
@@ -2793,8 +2777,7 @@ bpstat_check_location (const struct bp_l
       && b->type != bp_read_watchpoint
       && b->type != bp_access_watchpoint
       && b->type != bp_hardware_breakpoint
-      && b->type != bp_catch_fork
-      && b->type != bp_catch_vfork
+      && b->type != bp_catchpoint
       && b->type != bp_catch_exec)	/* a non-watchpoint bp */
     {
       if (bl->address != bp_addr) 	/* address doesn't match */
@@ -2827,7 +2810,7 @@ bpstat_check_location (const struct bp_l
 	  && !section_is_mapped (bl->section))
 	return 0;
     }
-  
+
   /* Is this a catchpoint of a load or unload?  If so, did we
      get a load or unload of the specified library?  If not,
      ignore it. */
@@ -2855,16 +2838,13 @@ bpstat_check_location (const struct bp_l
       )
     return 0;
 
-  if ((b->type == bp_catch_fork)
-      && !inferior_has_forked (inferior_ptid,
-			       &b->forked_inferior_pid))
-    return 0;
-  
-  if ((b->type == bp_catch_vfork)
-      && !inferior_has_vforked (inferior_ptid,
-				&b->forked_inferior_pid))
-    return 0;
-  
+  if (b->type == bp_catchpoint)
+    {
+      gdb_assert (b->ops != NULL && b->ops->breakpoint_hit != NULL);
+      if (!b->ops->breakpoint_hit (b))
+        return 0;
+    }
+     
   if ((b->type == bp_catch_exec)
       && !inferior_has_execd (inferior_ptid, &b->exec_pathname))
     return 0;
@@ -3393,8 +3373,7 @@ bpstat_what (bpstat bs)
 	  else
 	    bs_class = no_effect;
 	  break;
-	case bp_catch_fork:
-	case bp_catch_vfork:
+        case bp_catchpoint:
 	case bp_catch_exec:
 	  if (bs->stop)
 	    {
@@ -3573,10 +3552,9 @@ print_one_breakpoint_location (struct br
     {bp_shlib_event, "shlib events"},
     {bp_thread_event, "thread events"},
     {bp_overlay_event, "overlay events"},
+    {bp_catchpoint, "catchpoint"},
     {bp_catch_load, "catch load"},
     {bp_catch_unload, "catch unload"},
-    {bp_catch_fork, "catch fork"},
-    {bp_catch_vfork, "catch vfork"},
     {bp_catch_exec, "catch exec"}
   };
   
@@ -3708,23 +3686,6 @@ print_one_breakpoint_location (struct br
 	    ui_out_text (uiout, "library \"");
 	    ui_out_field_string (uiout, "what", b->dll_pathname);
 	    ui_out_text (uiout, "\" ");
-	  }
-	break;
-
-      case bp_catch_fork:
-      case bp_catch_vfork:
-	/* 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 (addressprint)
-	  ui_out_field_skip (uiout, "addr");
-	annotate_field (5);
-	if (!ptid_equal (b->forked_inferior_pid, null_ptid))
-	  {
-	    ui_out_text (uiout, "process ");
-	    ui_out_field_int (uiout, "what",
-			      ptid_get_pid (b->forked_inferior_pid));
-	    ui_out_spaces (uiout, 1);
 	  }
 	break;
 
@@ -3939,10 +3900,9 @@ user_settable_breakpoint (const struct b
 user_settable_breakpoint (const struct breakpoint *b)
 {
   return (b->type == bp_breakpoint
+          || b->type == bp_catchpoint
 	  || b->type == bp_catch_load
 	  || b->type == bp_catch_unload
-	  || b->type == bp_catch_fork
-	  || b->type == bp_catch_vfork
 	  || b->type == bp_catch_exec
 	  || b->type == bp_hardware_breakpoint
 	  || b->type == bp_watchpoint
@@ -4150,8 +4110,8 @@ set_default_breakpoint (int valid, CORE_
       bp_hardware_watchpoint
       bp_read_watchpoint
       bp_access_watchpoint
+      bp_catchpoint
       bp_catch_exec
-      bp_catch_fork
       bp_catch_vork */
 
 static int
@@ -4163,9 +4123,8 @@ breakpoint_address_is_meaningful (struct
 	  && type != bp_hardware_watchpoint
 	  && type != bp_read_watchpoint
 	  && type != bp_access_watchpoint
-	  && type != bp_catch_exec
-	  && type != bp_catch_fork
-	  && type != bp_catch_vfork);
+          && type != bp_catchpoint
+	  && type != bp_catch_exec);
 }
 
 /* Rescan breakpoints at the same address and section as BPT,
@@ -4280,8 +4239,7 @@ adjust_breakpoint_address (CORE_ADDR bpa
            || bptype == bp_hardware_watchpoint
            || bptype == bp_read_watchpoint
            || bptype == bp_access_watchpoint
-           || bptype == bp_catch_fork
-           || bptype == bp_catch_vfork
+           || bptype == bp_catchpoint
            || bptype == bp_catch_exec)
     {
       /* Watchpoints and the various bp_catch_* eventpoints should not
@@ -4348,8 +4306,7 @@ allocate_bp_location (struct breakpoint 
       loc->loc_type = bp_loc_hardware_watchpoint;
       break;
     case bp_watchpoint:
-    case bp_catch_fork:
-    case bp_catch_vfork:
+    case bp_catchpoint:
     case bp_catch_exec:
       loc->loc_type = bp_loc_other;
       break;
@@ -4750,45 +4707,210 @@ disable_breakpoints_in_unloaded_shlib (s
   }
 }
 
-static void
-create_fork_vfork_event_catchpoint (int tempflag, char *cond_string,
-				    enum bptype bp_kind)
-{
-  struct symtab_and_line sal;
-  struct breakpoint *b;
-  int thread = -1;		/* All threads. */
+/* FORK & VFORK catchpoints.  */
+
+/* Implement the "insert" breakpoint_ops method for fork catchpoints.  */
+
+static void
+insert_catch_fork (struct breakpoint *b)
+{
+  target_insert_fork_catchpoint (PIDGET (inferior_ptid));
+}
+
+/* Implement the "remove" breakpoint_ops method for fork catchpoints.  */
+
+static int
+remove_catch_fork (struct breakpoint *b)
+{
+  return target_remove_fork_catchpoint (PIDGET (inferior_ptid));
+}
+
+/* Implement the "breakpoint_hit" breakpoint_ops method for fork
+   catchpoints.  */
+
+static int
+breakpoint_hit_catch_fork (struct breakpoint *b)
+{
+  return inferior_has_forked (inferior_ptid, &b->forked_inferior_pid);
+}
+
+/* Implement the "print_it" breakpoint_ops method for fork catchpoints.  */
+
+static enum print_stop_action
+print_it_catch_fork (struct breakpoint *b)
+{
+  annotate_catchpoint (b->number);
+  printf_filtered (_("\nCatchpoint %d (forked process %d), "),
+		   b->number, ptid_get_pid (b->forked_inferior_pid));
+  return PRINT_SRC_AND_LOC;
+}
+
+/* Implement the "print_one" breakpoint_ops method for fork catchpoints.  */
+
+static void
+print_one_catch_fork (struct breakpoint *b, CORE_ADDR *last_addr)
+{
+  /* 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 (addressprint)
+    ui_out_field_skip (uiout, "addr");
+  annotate_field (5);
+  ui_out_text (uiout, "fork");
+  if (!ptid_equal (b->forked_inferior_pid, null_ptid))
+    {
+      ui_out_text (uiout, ", process ");
+      ui_out_field_int (uiout, "what",
+                        ptid_get_pid (b->forked_inferior_pid));
+      ui_out_spaces (uiout, 1);
+    }
+}
+
+/* Implement the "print_mention" breakpoint_ops method for fork
+   catchpoints.  */
+
+static void
+print_mention_catch_fork (struct breakpoint *b)
+{
+  printf_filtered (_("Catchpoint %d (fork)"), b->number);
+}
+
+/* The breakpoint_ops structure to be used in fork catchpoints.  */
+
+static struct breakpoint_ops catch_fork_breakpoint_ops =
+{
+  insert_catch_fork,
+  remove_catch_fork,
+  breakpoint_hit_catch_fork,
+  print_it_catch_fork,
+  print_one_catch_fork,
+  print_mention_catch_fork
+};
+
+/* Implement the "insert" breakpoint_ops method for vfork catchpoints.  */
+
+static void
+insert_catch_vfork (struct breakpoint *b)
+{
+  target_insert_vfork_catchpoint (PIDGET (inferior_ptid));
+}
+
+/* Implement the "remove" breakpoint_ops method for vfork catchpoints.  */
+
+static int
+remove_catch_vfork (struct breakpoint *b)
+{
+  return target_remove_vfork_catchpoint (PIDGET (inferior_ptid));
+}
+
+/* Implement the "breakpoint_hit" breakpoint_ops method for vfork
+   catchpoints.  */
+
+static int
+breakpoint_hit_catch_vfork (struct breakpoint *b)
+{
+  return inferior_has_vforked (inferior_ptid, &b->forked_inferior_pid);
+}
+
+/* Implement the "print_it" breakpoint_ops method for vfork catchpoints.  */
+
+static enum print_stop_action
+print_it_catch_vfork (struct breakpoint *b)
+{
+  annotate_catchpoint (b->number);
+  printf_filtered (_("\nCatchpoint %d (vforked process %d), "),
+		   b->number, ptid_get_pid (b->forked_inferior_pid));
+  return PRINT_SRC_AND_LOC;
+}
+
+/* Implement the "print_one" breakpoint_ops method for vfork catchpoints.  */
+
+static void
+print_one_catch_vfork (struct breakpoint *b, CORE_ADDR *last_addr)
+{
+  /* 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 (addressprint)
+    ui_out_field_skip (uiout, "addr");
+  annotate_field (5);
+  ui_out_text (uiout, "vfork");
+  if (!ptid_equal (b->forked_inferior_pid, null_ptid))
+    {
+      ui_out_text (uiout, ", process ");
+      ui_out_field_int (uiout, "what",
+                        ptid_get_pid (b->forked_inferior_pid));
+      ui_out_spaces (uiout, 1);
+    }
+}
+
+/* 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);
+}
+
+/* The breakpoint_ops structure to be used in vfork catchpoints.  */
+
+static struct breakpoint_ops catch_vfork_breakpoint_ops =
+{
+  insert_catch_vfork,
+  remove_catch_vfork,
+  breakpoint_hit_catch_vfork,
+  print_it_catch_vfork,
+  print_one_catch_vfork,
+  print_mention_catch_vfork
+};
+
+/* Create a new breakpoint of the bp_catchpoint kind and return it.
+ 
+   If TEMPFLAG is non-zero, then make the breakpoint temporary.
+   If COND_STRING is not NULL, then store it in the breakpoint.
+   OPS, if not NULL, is the breakpoint_ops structure associated
+   to the catchpoint.  */
+
+static struct breakpoint *
+create_catchpoint (int tempflag, char *cond_string,
+                   struct breakpoint_ops *ops)
+{
+  struct symtab_and_line sal;
+  struct breakpoint *b;
 
   init_sal (&sal);
   sal.pc = 0;
   sal.symtab = NULL;
   sal.line = 0;
 
-  b = set_raw_breakpoint (sal, bp_kind);
+  b = set_raw_breakpoint (sal, bp_catchpoint);
   set_breakpoint_count (breakpoint_count + 1);
   b->number = breakpoint_count;
+
   b->cond_string = (cond_string == NULL) ? 
     NULL : savestring (cond_string, strlen (cond_string));
-  b->thread = thread;
+  b->thread = -1;
   b->addr_string = NULL;
   b->enable_state = bp_enabled;
   b->disposition = tempflag ? disp_del : disp_donttouch;
+  b->ops = ops;
+
+  mention (b);
+  update_global_location_list (1);
+
+  return b;
+}
+
+static void
+create_fork_vfork_event_catchpoint (int tempflag, char *cond_string,
+                                    struct breakpoint_ops *ops)
+{
+  struct breakpoint *b = create_catchpoint (tempflag, cond_string, ops);
+
+  /* FIXME: We should put this information in a breakpoint private data
+     area.  */
   b->forked_inferior_pid = null_ptid;
-  update_global_location_list (1);
-
-
-  mention (b);
-}
-
-static void
-create_fork_event_catchpoint (int tempflag, char *cond_string)
-{
-  create_fork_vfork_event_catchpoint (tempflag, cond_string, bp_catch_fork);
-}
-
-static void
-create_vfork_event_catchpoint (int tempflag, char *cond_string)
-{
-  create_fork_vfork_event_catchpoint (tempflag, cond_string, bp_catch_vfork);
 }
 
 static void
@@ -5023,12 +5145,6 @@ mention (struct breakpoint *b)
 			 (b->type == bp_catch_load) ? "load" : "unload",
 			 (b->dll_pathname != NULL) ? 
 			 b->dll_pathname : "<any library>");
-	break;
-      case bp_catch_fork:
-      case bp_catch_vfork:
-	printf_filtered (_("Catchpoint %d (%s)"),
-			 b->number,
-			 (b->type == bp_catch_fork) ? "fork" : "vfork");
 	break;
       case bp_catch_exec:
 	printf_filtered (_("Catchpoint %d (exec)"),
@@ -6450,11 +6566,13 @@ catch_fork_command_1 (char *arg, int fro
     {
     case catch_fork_temporary:
     case catch_fork_permanent:
-      create_fork_event_catchpoint (tempflag, cond_string);
+      create_fork_vfork_event_catchpoint (tempflag, cond_string,
+                                          &catch_fork_breakpoint_ops);
       break;
     case catch_vfork_temporary:
     case catch_vfork_permanent:
-      create_vfork_event_catchpoint (tempflag, cond_string);
+      create_fork_vfork_event_catchpoint (tempflag, cond_string,
+                                          &catch_vfork_breakpoint_ops);
       break;
     default:
       error (_("unsupported or unknown fork kind; cannot catch it"));
@@ -6651,6 +6769,9 @@ print_mention_exception_catchpoint (stru
 }
 
 static struct breakpoint_ops gnu_v3_exception_catchpoint_ops = {
+  NULL, /* insert */
+  NULL, /* remove */
+  NULL, /* breakpoint_hit */
   print_exception_catchpoint,
   print_one_exception_catchpoint,
   print_mention_exception_catchpoint
@@ -7617,8 +7738,7 @@ breakpoint_re_set_one (void *bint)
       /* We needn't really do anything to reset these, since the mask
          that requests them is unaffected by e.g., new libraries being
          loaded. */
-    case bp_catch_fork:
-    case bp_catch_vfork:
+    case bp_catchpoint:
     case bp_catch_exec:
       break;
 
@@ -7888,10 +8008,9 @@ disable_command (char *args, int from_tt
 		 bpt->number);
 	continue;
       case bp_breakpoint:
+      case bp_catchpoint:
       case bp_catch_load:
       case bp_catch_unload:
-      case bp_catch_fork:
-      case bp_catch_vfork:
       case bp_catch_exec:
       case bp_hardware_breakpoint:
       case bp_watchpoint:
@@ -8022,10 +8141,9 @@ enable_command (char *args, int from_tty
 		 bpt->number);
 	continue;
       case bp_breakpoint:
+      case bp_catchpoint:
       case bp_catch_load:
       case bp_catch_unload:
-      case bp_catch_fork:
-      case bp_catch_vfork:
       case bp_catch_exec:
       case bp_hardware_breakpoint:
       case bp_watchpoint:
diff -r c9da7894e8d5 gdb/breakpoint.h
--- a/gdb/breakpoint.h	Sun Oct 05 16:06:42 2008 -0700
+++ b/gdb/breakpoint.h	Mon Oct 06 13:05:03 2008 -0400
@@ -110,6 +110,8 @@ enum bptype
 
     bp_overlay_event, 
 
+    bp_catchpoint,
+
     /* These breakpoints are used to implement the "catch load" command
        on platforms whose dynamic linkers support such functionality.  */
     bp_catch_load,
@@ -124,8 +126,6 @@ enum bptype
        kernels which can raise an event when a fork or exec occurs, as
        opposed to the debugger setting breakpoints on functions named
        "fork" or "exec".) */
-    bp_catch_fork,
-    bp_catch_vfork,
     bp_catch_exec,
   };
 
@@ -315,6 +315,19 @@ struct bp_location
 
 struct breakpoint_ops 
 {
+  /* Insert the breakpoint or activate the catchpoint.  Should raise
+     an exception if the operation failed.  */
+  void (*insert) (struct breakpoint *);
+
+  /* Remove the breakpoint/catchpoint that was previously inserted
+     with the "insert" method above.  Return non-zero if the operation
+     succeeded.  */
+  int (*remove) (struct breakpoint *);
+
+  /* Return non-zero if the debugger should tell the user that this
+     breakpoint was hit.  */
+  int (*breakpoint_hit) (struct breakpoint *);
+
   /* The normal print routine for this breakpoint, called when we
      hit it.  */
   enum print_stop_action (*print_it) (struct breakpoint *);
diff -r c9da7894e8d5 gdb/testsuite/gdb.base/foll-fork.exp
--- a/gdb/testsuite/gdb.base/foll-fork.exp	Sun Oct 05 16:06:42 2008 -0700
+++ b/gdb/testsuite/gdb.base/foll-fork.exp	Mon Oct 06 13:05:03 2008 -0400
@@ -158,14 +158,11 @@ proc catch_fork_child_follow {} {
    # Verify that the catchpoint is mentioned in an "info breakpoints",
    # and further that the catchpoint mentions no process id.
    #
-   send_gdb "info breakpoints\n"
-   gdb_expect {
-     -re ".*catch fork.*keep y.*$gdb_prompt $"\
-                     {pass "info shows catchpoint without pid"}
-     -re ".*catch fork.*process .*$gdb_prompt $"\
-                     {fail "info shows catchpoint without pid"}
-     -re "$gdb_prompt $" {fail "info shows catchpoint without pid"}
-     timeout         {fail "(timeout) info shows catchpoint without pid"}
+   set test_name "info shows catchpoint without pid"
+   gdb_test_multiple "info breakpoints" "$test_name" {
+     -re ".*catchpoint.*keep y.*fork\[\r\n\]+$gdb_prompt $" {
+       pass "$test_name"
+     }
    }
 
    send_gdb "continue\n"
@@ -179,12 +176,11 @@ proc catch_fork_child_follow {} {
    # Verify that the catchpoint is mentioned in an "info breakpoints",
    # and further that the catchpoint managed to capture a process id.
    #
-   send_gdb "info breakpoints\n"
-   gdb_expect {
-     -re ".*catch fork .*process \[0-9\]+.*$gdb_prompt $"\
-                     {pass "info shows catchpoint pid"}
-     -re "$gdb_prompt $" {fail "info shows catchpoint pid"}
-     timeout         {fail "(timeout) info shows catchpoint pid"}
+   set test_name "info shows catchpoint without pid"
+   gdb_test_multiple "info breakpoints" "$test_name" {
+     -re ".*catchpoint.*keep y.*fork, process.*$gdb_prompt $" {
+       pass "$test_name"
+     }
    }
 
    send_gdb "set follow child\n"

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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-10-03 17:52       ` Daniel Jacobowitz
@ 2008-10-04 23:07         ` Sérgio Durigan Júnior
  0 siblings, 0 replies; 57+ messages in thread
From: Sérgio Durigan Júnior @ 2008-10-04 23:07 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Joel Brobecker, gdb-patches

Hi Daniel,

On Fri, 2008-10-03 at 13:51 -0400, Daniel Jacobowitz wrote:
> On Thu, Oct 02, 2008 at 11:06:29PM -0700, Joel Brobecker wrote:
> > That's correct. The short answer is that, if we make your catchpoint
> > use a more generic type and base the actual implementation of
> > breakpoint_ops methods, then, later on, when someone decides to
> > implement a new kind of catchpoint with similar functionality,
> > then all he should have to do is create a new breakpoint_ops vector
> > with appropriate methods, and voila.
> 
> Right.  This is what's supposed to be possible, at least.

I'm not sure if I understood correctly, but you're saying that the
approach that Joel proposed is already working, right? If that's the
case, then there should be no problem in "converting" my patch to this
new way.

Regards,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-10-03  6:07     ` Joel Brobecker
  2008-10-03 17:52       ` Daniel Jacobowitz
@ 2008-10-04 23:04       ` Sérgio Durigan Júnior
  2008-10-06 17:22         ` Joel Brobecker
  1 sibling, 1 reply; 57+ messages in thread
From: Sérgio Durigan Júnior @ 2008-10-04 23:04 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

Hello Joel,

On Thu, 2008-10-02 at 23:06 -0700, Joel Brobecker wrote:

> That's correct. The short answer is that, if we make your catchpoint
> use a more generic type and base the actual implementation of
> breakpoint_ops methods, then, later on, when someone decides to
> implement a new kind of catchpoint with similar functionality,
> then all he should have to do is create a new breakpoint_ops vector
> with appropriate methods, and voila.

Right, so I think I got your idea ;-).

> Now, going back to your patch, I think it's important not to let
> best be the enemy of good either.  I think that the feature that
> you are proposing is interesting and would like to make sure that
> we don't lose your work.  I am about to go away on a trip, so will
> have little time to review the details of your patch, but looks like
> Michael has started on that. I would love for you to investigate
> my (ahem) Vision (double-ahem), and I'm ready to guide you as much
> as I can.  But I am equally happy to incorporate your approach
> if another maintainer looks at your code and OKays it.

I really appreciate your review, and will certainly investigate your
vision about how things should work. Also, let me know when you have
more time (probably when you get back from your trip) so we can talk
more about this and work together to implement things better.

Meanwhile, I'll try to understand and improve things by my own :-).

Regards,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-10-03  6:07     ` Joel Brobecker
@ 2008-10-03 17:52       ` Daniel Jacobowitz
  2008-10-04 23:07         ` Sérgio Durigan Júnior
  2008-10-04 23:04       ` Sérgio Durigan Júnior
  1 sibling, 1 reply; 57+ messages in thread
From: Daniel Jacobowitz @ 2008-10-03 17:52 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Sérgio Durigan Júnior, gdb-patches

On Thu, Oct 02, 2008 at 11:06:29PM -0700, Joel Brobecker wrote:
> That's correct. The short answer is that, if we make your catchpoint
> use a more generic type and base the actual implementation of
> breakpoint_ops methods, then, later on, when someone decides to
> implement a new kind of catchpoint with similar functionality,
> then all he should have to do is create a new breakpoint_ops vector
> with appropriate methods, and voila.

Right.  This is what's supposed to be possible, at least.

The way I would suggest to do this is to create a new bp_catchpoint.
We can either leave existing OS catchpoints alone, or else convert
some of them (fork/vfork for example) to the new mechanism as proof
that it works.

-- 
Daniel Jacobowitz
CodeSourcery


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part
  2008-10-03  2:33   ` Sérgio Durigan Júnior
@ 2008-10-03  6:07     ` Joel Brobecker
  2008-10-03 17:52       ` Daniel Jacobowitz
  2008-10-04 23:04       ` Sérgio Durigan Júnior
  0 siblings, 2 replies; 57+ messages in thread
From: Joel Brobecker @ 2008-10-03  6:07 UTC (permalink / raw)
  To: Sérgio Durigan Júnior; +Cc: gdb-patches

Hi Sergio,

> If I understood correctly, you think it's better to create a "generic"
> bptype so that not only "catch syscall" can use it, right?

That's correct. The short answer is that, if we make your catchpoint
use a more generic type and base the actual implementation of
breakpoint_ops methods, then, later on, when someone decides to
implement a new kind of catchpoint with similar functionality,
then all he should have to do is create a new breakpoint_ops vector
with appropriate methods, and voila.

This is pretty much what happened when I implemented catchpoints
on Ada exception.  If you had a look at my patch, you'll notice
that I didn't have to sprinkle "case bp_my_new_kind" everywhere
to make it work.  To me, this makes me so much more confident
that I didn't miss an update somewhere. But my situation was simpler
than yours, however, because my catchpoints were barely more than just
an elaborated breakpoint.

The longer answer is that I have been wondering about making the
breakpoint structure a little more object-oriented. I have this
Vision (no mushrooms involved so far) where there are really two
types of catchpoints:
  (a): Those that are implemented using one or more breakpoints
  (b): Those that are implemented without the use of breakpoins
Either way, I would like to see these breakpoints as objects,
with methods that we would call:
  (1) insert: Insert the physical breakpoint, or activate the
      associated trace, or...
  (2) remove: The opposite of insert
  (3) is_mind: Return true if an event is recognized by the catchpoint
  (3) print_stop_action, print_one, print_mention.
      (the methods already provided by the breakpoint_ops)
  (4) any other method?
So, when the debugger needs to insert breakpoints, he doesn't have
to know how syscall catchpoints are implemented.  It just calls
the associated "insert" method. Same for removing.  When we receive
an event, we could rely on the "is_mine" method to determine that
we stopped due to this catchpoint, and thus call the associated
methods to tell the user about them. Etc.

The thing is, I haven't designed this part of the code, and I don't
know if the breakpoint_ops were meant to be used such that we could
make the core code completely abstract of which breakpoint was hit.
Nor am I certain that it is actually possible.  But it's definitely
something that I would like to try.

Now, going back to your patch, I think it's important not to let
best be the enemy of good either.  I think that the feature that
you are proposing is interesting and would like to make sure that
we don't lose your work.  I am about to go away on a trip, so will
have little time to review the details of your patch, but looks like
Michael has started on that. I would love for you to investigate
my (ahem) Vision (double-ahem), and I'm ready to guide you as much
as I can.  But I am equally happy to incorporate your approach
if another maintainer looks at your code and OKays it.

> The way I see, there's no problem to implement "catch syscall" using
> another way. However, and I think you already know that, the "catch
> fork", "catch vfork" and "catch exec" are implemented using an entry on
> "enum bptype" (and that's specific why I've chosen to do this). Do you
> think we should change this, too?

Not as part of your patch.  But in the long term, I would like to
migrate them to the breakpoint_ops approach if possible.  That's if
we have determined that it's doable, that is.

> I'm not sure if I understood this part correctly. Maybe my GDB-fu isn't
> good enought yet :-). I'll take a look at breakpoint_ops to see if I can
> understand better what you want.

Hopefully the explainations above might have clarified a little bit
the context. Let me know if there are parts that you'd like me to
expand more.

-- 
Joel


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

* Re: [PATCH 1/4] 'catch syscall' feature --  Architecture-independent part
  2008-10-02 21:13 ` Joel Brobecker
@ 2008-10-03  2:33   ` Sérgio Durigan Júnior
  2008-10-03  6:07     ` Joel Brobecker
  0 siblings, 1 reply; 57+ messages in thread
From: Sérgio Durigan Júnior @ 2008-10-03  2:33 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

Hi Joel,

On Thu, 2008-10-02 at 14:12 -0700, Joel Brobecker wrote:
> > 	* breakpoint.h (enum bptype): Add syscall catchpoint and entry
> > 	breakpoint types.
> > 	(struct breakpoint): Add field 'syscall_number' (used to know
> > 	which syscall triggered the catchpoint) and
> > 	'syscall_to_be_caught' (used to know which syscall we are trying
> > 	to catch).
> 
> I would really like to try to avoid creating a different enum for
> each catchpoint type, if we can.  See for instance how I implemented
> Ada exception catchpoints:
> 
>     http://www.sourceware.org/ml/gdb-patches/2007-01/msg00102.html
> 
> Do you think we could leverage on the "breakpoint_ops" structure
> to implement this feature? Daniel, you know this part of the code
> really well, what do you think?
> 
> Obviously, his feature is a different kind of beast than Ada exception
> catchpoints (which behind the scene is just an elaborated breakpoint).
> So he won't be able to use the bp_breakpoint kind I used. Actually,
> I don't see any bptype kind that could be used, so we'll have to create
> one, but if the name could be generic enough to be used in other
> circumstances.

If I understood correctly, you think it's better to create a "generic"
bptype so that not only "catch syscall" can use it, right? That, or
figure out some other way to use "breakpoint_ops" in order to identify
if the catchpoint is a syscall one. Is it?

The way I see, there's no problem to implement "catch syscall" using
another way. However, and I think you already know that, the "catch
fork", "catch vfork" and "catch exec" are implemented using an entry on
"enum bptype" (and that's specific why I've chosen to do this). Do you
think we should change this, too?

> Similarly, the breakpoint_ops structure might need to be extended 
> a bit, to provide methods that would insert/remove the catchpoint.

I'm not sure if I understood this part correctly. Maybe my GDB-fu isn't
good enought yet :-). I'll take a look at breakpoint_ops to see if I can
understand better what you want.

Thanks for the review, by the way.

Regards,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil


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

* Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent  part
  2008-09-30 18:12 Sérgio Durigan Júnior
@ 2008-10-02 21:13 ` Joel Brobecker
  2008-10-03  2:33   ` Sérgio Durigan Júnior
  0 siblings, 1 reply; 57+ messages in thread
From: Joel Brobecker @ 2008-10-02 21:13 UTC (permalink / raw)
  To: Sérgio Durigan Júnior; +Cc: gdb-patches

> 	* breakpoint.h (enum bptype): Add syscall catchpoint and entry
> 	breakpoint types.
> 	(struct breakpoint): Add field 'syscall_number' (used to know
> 	which syscall triggered the catchpoint) and
> 	'syscall_to_be_caught' (used to know which syscall we are trying
> 	to catch).

I would really like to try to avoid creating a different enum for
each catchpoint type, if we can.  See for instance how I implemented
Ada exception catchpoints:

    http://www.sourceware.org/ml/gdb-patches/2007-01/msg00102.html

Do you think we could leverage on the "breakpoint_ops" structure
to implement this feature? Daniel, you know this part of the code
really well, what do you think?

Obviously, his feature is a different kind of beast than Ada exception
catchpoints (which behind the scene is just an elaborated breakpoint).
So he won't be able to use the bp_breakpoint kind I used. Actually,
I don't see any bptype kind that could be used, so we'll have to create
one, but if the name could be generic enough to be used in other
circumstances.

Similarly, the breakpoint_ops structure might need to be extended 
a bit, to provide methods that would insert/remove the catchpoint.

-- 
Joel


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

* [PATCH 1/4] 'catch syscall' feature -- Architecture-independent  part
@ 2008-09-30 18:12 Sérgio Durigan Júnior
  2008-10-02 21:13 ` Joel Brobecker
  0 siblings, 1 reply; 57+ messages in thread
From: Sérgio Durigan Júnior @ 2008-09-30 18:12 UTC (permalink / raw)
  To: gdb-patches

This is the architecture-independent part of the patch.

Regards,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil


2008-09-29  Sergio Durigan Junior  <sergiodj@linux.vnet.ibm.com>

	* breakpoint.c (clear_syscall_catchpoints_info): New.
	(insert_catchpoint): Add syscall catchpoint case.
	(insert_bp_location): Add syscall catchpoint for checking.
	(remove_breakpoint): Removing syscall catchpoint.
	(ep_is_catchpoint): Adding syscall catchpoint for checking.
	(print_it_typical): Included syscall catchpoint code for 
	printing.  Also, handle the case of the breakpoint at the
	entrypoint.
	(bpstat_check_location): Add syscall catchpoint handler.
	(bpstat_what): Add the entry breakpoint case.
	(print_one_breakpoint): Add syscall catchpoint and entry
	breakpoint.
	(user_settable_breakpoint): Add syscall catchpoint.
	(breakpoint_address_is_meaningful): Likewise.
	(adjust_breakpoint_address): Likewise.
	(allocate_bp_location): Add syscall catchpoint and entry
	breakpoint.
	(set_raw_breakpoint_without_location): Zeroing fields related
	to syscall catchpoint.
	(create_entry_breakpoint): New.
	(create_syscall_event_catchpoint): New.
	(mention): Add syscall catchpoint and entry breakpoint.
	(catch_syscall_command_1): New.
	(delete_command): Add syscall catchpoint and entry breakpoint.
	(breakpoint_re_set_one): Likewise.
	(disable_command): Add syscall catchpoint.
	(enable_command): Likewise.
	(is_syscall_catchpoint_enabled): New.
	(catch_syscall_enabled): New.
	(catching_syscall_number): New.
	* breakpoint.h (enum bptype): Add syscall catchpoint and entry
	breakpoint types.
	(struct breakpoint): Add field 'syscall_number' (used to know
	which syscall triggered the catchpoint) and
	'syscall_to_be_caught' (used to know which syscall we are trying
	to catch).
	(enum bpstat_what_main_action): Add BPSTAT_WHAT_ENTRY_BREAKPOINT,
	used to identify wheter we hit an entry breakpoint.
	(clear_syscall_catchpoints_info): New.
	(catch_syscall_enabled): New.
	(catching_syscall_number): New.
	(create_entry_breakpoint): New.
	* gdbarch.c: Regenerate.
	* gdbarch.h: Regenerate.
	* gdbarch.sh: Add syscall catchpoint functions to gdbarch.
	(get_syscall_number): New.
	(syscall_name_from_number): New.
	(syscall_number_from_name): New.
	* gdbthread.h (struct thread_info): Add field 'syscall_state',
	used to know if we are calling or returning from a syscall.
	* inf-child.c (inf_child_insert_syscall_catchpoint): New.
	(inf_child_remove_syscall_catchpoint): New.
	(inf_child_target): Assign default values to target_ops.
	* inf-ptrace.c (inf_ptrace_resume): Select the proper request
	to be made for ptrace() considering if we are catching syscalls
	or not.
	* infcmd.c (run_command_1): Clean syscall catchpoint info on every
	run.
	* infrun.c (resume): Add syscall catchpoint and entry breakpoint.
	(deal_with_syscall_event): New.
	(handle_inferior_event): Add syscall entry/return events.  Also,
	add entry breakpoint event.
	(inferior_has_called_syscall): New.
	* target.c (update_current_target): Update/copy functions related to
	syscall catchpoint and entry breakpoint.
	(debug_to_wait): Add syscall catchpoint entry/return events.
	* target.h (struct target_waitstatus): Add syscall number.
	(struct target_ops): Add ops for syscall catchpoint and entry
	breakpoint.
	(inferior_has_called_syscall): New.
	(target_passed_by_entrypoint): New.
	(target_insert_syscall_catchpoint): New.
	(target_remove_syscall_catchpoint): New.
	(target_enable_tracesysgood): New.




diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 6e863d7..7f9cc49 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -404,6 +404,18 @@ set_breakpoint_count (int num)
 		   value_from_longest (builtin_type_int32, (LONGEST) num));
 }
 
+/* Used in run_command to reset syscall catchpoints fields. */
+
+void
+clear_syscall_catchpoints_info (void)
+{
+  struct breakpoint *b;
+
+  ALL_BREAKPOINTS (b)
+    if (b->type == bp_catch_syscall)
+      b->syscall_number = UNKNOWN_SYSCALL;
+}
+
 /* Used in run_command to zero the hit count when a new run starts. */
 
 void
@@ -810,6 +822,9 @@ insert_catchpoint (struct ui_out *uo, void *args)
     case bp_catch_exec:
       target_insert_exec_catchpoint (PIDGET (inferior_ptid));
       break;
+    case bp_catch_syscall:
+      target_insert_syscall_catchpoint (PIDGET (inferior_ptid));
+      break;
     default:
       internal_error (__FILE__, __LINE__, _("unknown breakpoint type"));
       break;
@@ -1250,7 +1265,8 @@ Note: automatically using hardware breakpoints for read-only addresses.\n"));
 
   else if (bpt->owner->type == bp_catch_fork
 	   || bpt->owner->type == bp_catch_vfork
-	   || bpt->owner->type == bp_catch_exec)
+	   || bpt->owner->type == bp_catch_exec
+	   || bpt->owner->type == bp_catch_syscall)
     {
       struct gdb_exception e = catch_exception (uiout, insert_catchpoint,
 						bpt->owner, RETURN_MASK_ERROR);
@@ -1708,6 +1724,9 @@ remove_breakpoint (struct bp_location *b, insertion_state_t is)
 	case bp_catch_exec:
 	  val = target_remove_exec_catchpoint (PIDGET (inferior_ptid));
 	  break;
+        case bp_catch_syscall:
+          val = target_remove_syscall_catchpoint (PIDGET (inferior_ptid));
+          break;
 	default:
 	  warning (_("Internal error, %s line %d."), __FILE__, __LINE__);
 	  break;
@@ -1957,7 +1976,8 @@ ep_is_catchpoint (struct breakpoint *ep)
     || (ep->type == bp_catch_unload)
     || (ep->type == bp_catch_fork)
     || (ep->type == bp_catch_vfork)
-    || (ep->type == bp_catch_exec);
+    || (ep->type == bp_catch_exec)
+    || (ep->type == bp_catch_syscall);
 
   /* ??rehrauer: Add more kinds here, as are implemented... */
 }
@@ -2278,6 +2298,13 @@ print_it_typical (bpstat bs)
   struct cleanup *old_chain, *ui_out_chain;
   struct breakpoint *b;
   const struct bp_location *bl;
+  /* Used for "catch syscall".
+     
+     This is needed because we want to know in which state a
+     syscall is. It can be in the TARGET_WAITKIND_SYSCALL_ENTRY
+     or TARGET_WAITKIND_SYSCALL_RETURN, and depending on it we
+     must print "called syscall" or "returned from syscall". */
+  struct thread_info *th_info = find_thread_pid (inferior_ptid);
   struct ui_stream *stb;
   int bp_temp = 0;  
   stb = ui_out_stream_new (uiout);
@@ -2329,6 +2356,14 @@ print_it_typical (bpstat bs)
       return PRINT_NOTHING;
       break;
 
+    case bp_entry_breakpoint:
+       /* Not sure how we will get here. 
+	 GDB should not stop for these breakpoints.  */
+      internal_error (__FILE__, __LINE__,
+                      _("Entry Breakpoint: gdb should not stop!\n"));
+      return PRINT_NOTHING;
+      break;    
+
     case bp_overlay_event:
       /* By analogy with the thread event, GDB should not stop for these. */
       printf_filtered (_("Overlay Event Breakpoint: gdb should not stop!\n"));
@@ -2375,6 +2410,17 @@ print_it_typical (bpstat bs)
       return PRINT_SRC_AND_LOC;
       break;
 
+    case bp_catch_syscall:
+      annotate_catchpoint (b->number);
+      printf_filtered (_("\nCatchpoint %d (%s syscall '%s ()'), "),
+		       b->number,
+                       (th_info->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY)
+                        ? "called" : "returned from",
+		       gdbarch_syscall_name_from_number (current_gdbarch,
+                                                         b->syscall_number));
+      return PRINT_SRC_AND_LOC;
+      break;
+
     case bp_watchpoint:
     case bp_hardware_watchpoint:
       annotate_watchpoint (b->number);
@@ -2795,7 +2841,8 @@ bpstat_check_location (const struct bp_location *bl, CORE_ADDR bp_addr)
       && b->type != bp_hardware_breakpoint
       && b->type != bp_catch_fork
       && b->type != bp_catch_vfork
-      && b->type != bp_catch_exec)	/* a non-watchpoint bp */
+      && b->type != bp_catch_exec
+      && b->type != bp_catch_syscall)	/* a non-watchpoint bp */
     {
       if (bl->address != bp_addr) 	/* address doesn't match */
 	return 0;
@@ -2869,6 +2916,25 @@ bpstat_check_location (const struct bp_location *bl, CORE_ADDR bp_addr)
       && !inferior_has_execd (inferior_ptid, &b->exec_pathname))
     return 0;
 
+  /* We must check if we are catching specific syscalls in this breakpoint.
+     If we are, then we must guarantee that the called syscall is the same
+     syscall we are catching. */
+  if (b->type == bp_catch_syscall)
+    {
+      int syscall_number;
+      if (!inferior_has_called_syscall (inferior_ptid, &syscall_number))
+        return 0;
+      /* Now, checking if the syscall is the same. */
+      if (b->syscall_to_be_caught != CATCHING_ANY_SYSCALL
+          && b->syscall_to_be_caught != syscall_number)
+        /* Not the same. */
+        return 0;
+
+      /* It's the same syscall. We can update the breakpoint struct
+         with the correct information. */
+      b->syscall_number = syscall_number;
+    }
+
   return 1;
 }
 
@@ -3212,6 +3278,9 @@ bpstat_what (bpstat bs)
       /* We caught a shared library event.  */
       catch_shlib_event,
 
+      /* We are in a entry breakpoint. */
+      entry_breakpoint,
+
       /* This is just used to count how many enums there are.  */
       class_last
     };
@@ -3228,6 +3297,7 @@ bpstat_what (bpstat bs)
 #define sr BPSTAT_WHAT_STEP_RESUME
 #define shl BPSTAT_WHAT_CHECK_SHLIBS
 #define shlr BPSTAT_WHAT_CHECK_SHLIBS_RESUME_FROM_HOOK
+#define entrybp BPSTAT_WHAT_ENTRY_BREAKPOINT
 
 /* "Can't happen."  Might want to print an error message.
    abort() is not out of the question, but chances are GDB is just
@@ -3272,30 +3342,33 @@ bpstat_what (bpstat bs)
     table[(int) class_last][(int) BPSTAT_WHAT_LAST] =
   {
   /*                              old action */
-  /*       kc    ss    sn    sgl    slr   clr   sr   shl   shlr
+  /*       kc    ss    sn    sgl    slr   clr   sr   shl   shlr   entrybp
    */
 /*no_effect */
-    {kc, ss, sn, sgl, slr, clr, sr, shl, shlr},
+    {kc, ss, sn, sgl, slr, clr, sr, shl, shlr, shlr},
 /*wp_silent */
-    {ss, ss, sn, ss, ss, ss, sr, shl, shlr},
+    {ss, ss, sn, ss, ss, ss, sr, shl, shlr, shlr},
 /*wp_noisy */
-    {sn, sn, sn, sn, sn, sn, sr, shl, shlr},
+    {sn, sn, sn, sn, sn, sn, sr, shl, shlr, shlr},
 /*bp_nostop */
-    {sgl, ss, sn, sgl, slr, slr, sr, shl, shlr},
+    {sgl, ss, sn, sgl, slr, slr, sr, shl, shlr, shlr},
 /*bp_silent */
-    {ss, ss, sn, ss, ss, ss, sr, shl, shlr},
+    {ss, ss, sn, ss, ss, ss, sr, shl, shlr, shlr},
 /*bp_noisy */
-    {sn, sn, sn, sn, sn, sn, sr, shl, shlr},
+    {sn, sn, sn, sn, sn, sn, sr, shl, shlr, shlr},
 /*long_jump */
-    {slr, ss, sn, slr, slr, err, sr, shl, shlr},
+    {slr, ss, sn, slr, slr, err, sr, shl, shlr, shlr},
 /*long_resume */
-    {clr, ss, sn, err, err, err, sr, shl, shlr},
+    {clr, ss, sn, err, err, err, sr, shl, shlr, shlr},
 /*step_resume */
-    {sr, sr, sr, sr, sr, sr, sr, sr, sr},
+    {sr, sr, sr, sr, sr, sr, sr, sr, sr, sr},
 /*shlib */
-    {shl, shl, shl, shl, shl, shl, sr, shl, shlr},
+    {shl, shl, shl, shl, shl, shl, sr, shl, shlr, shl},
 /*catch_shlib */
-    {shlr, shlr, shlr, shlr, shlr, shlr, sr, shlr, shlr}
+    {shlr, shlr, shlr, shlr, shlr, shlr, sr, shlr, shlr, shlr},
+/* entry_breakpoint */
+    {entrybp, entrybp, entrybp, entrybp, entrybp, entrybp, sr, entrybp,
+      entrybp, entrybp}
   };
 
 #undef kc
@@ -3309,6 +3382,7 @@ bpstat_what (bpstat bs)
 #undef ts
 #undef shl
 #undef shlr
+#undef entrybp
   enum bpstat_what_main_action current_action = BPSTAT_WHAT_KEEP_CHECKING;
   struct bpstat_what retval;
 
@@ -3396,6 +3470,7 @@ bpstat_what (bpstat bs)
 	case bp_catch_fork:
 	case bp_catch_vfork:
 	case bp_catch_exec:
+        case bp_catch_syscall:
 	  if (bs->stop)
 	    {
 	      if (bs->print)
@@ -3414,6 +3489,12 @@ bpstat_what (bpstat bs)
 	  bs_class = bp_silent;
 	  retval.call_dummy = 1;
 	  break;
+        case bp_entry_breakpoint:
+          if (bs->stop)
+            bs_class = entry_breakpoint;
+          else
+            bs_class = no_effect;
+          break;
 	}
       current_action = table[(int) bs_class][(int) current_action];
     }
@@ -3577,7 +3658,9 @@ print_one_breakpoint_location (struct breakpoint *b,
     {bp_catch_unload, "catch unload"},
     {bp_catch_fork, "catch fork"},
     {bp_catch_vfork, "catch vfork"},
-    {bp_catch_exec, "catch exec"}
+    {bp_catch_exec, "catch exec"},
+    {bp_catch_syscall, "catch syscall"},
+    {bp_entry_breakpoint, "entry breakpoint"}
   };
   
   static char bpenables[] = "nynny";
@@ -3743,6 +3826,23 @@ print_one_breakpoint_location (struct breakpoint *b,
 	  }
 	break;
 
+      case bp_catch_syscall:
+	/* 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 (addressprint)
+	  ui_out_field_skip (uiout, "addr");
+	annotate_field (5);
+        ui_out_text (uiout, "syscall \"");
+        if (b->syscall_number != UNKNOWN_SYSCALL)
+          ui_out_field_string (uiout, "what",
+                       gdbarch_syscall_name_from_number (current_gdbarch,
+                                                         b->syscall_number));
+        else
+            ui_out_field_string (uiout, "what", "<unknown syscall>");
+        ui_out_text (uiout, "\" ");
+	break;
+
       case bp_breakpoint:
       case bp_hardware_breakpoint:
       case bp_until:
@@ -3755,6 +3855,7 @@ print_one_breakpoint_location (struct breakpoint *b,
       case bp_shlib_event:
       case bp_thread_event:
       case bp_overlay_event:
+      case bp_entry_breakpoint:
 	if (addressprint)
 	  {
 	    annotate_field (4);
@@ -3944,6 +4045,7 @@ user_settable_breakpoint (const struct breakpoint *b)
 	  || b->type == bp_catch_fork
 	  || b->type == bp_catch_vfork
 	  || b->type == bp_catch_exec
+          || b->type == bp_catch_syscall
 	  || b->type == bp_hardware_breakpoint
 	  || b->type == bp_watchpoint
 	  || b->type == bp_read_watchpoint
@@ -4150,6 +4252,7 @@ set_default_breakpoint (int valid, CORE_ADDR addr, struct symtab *symtab,
       bp_hardware_watchpoint
       bp_read_watchpoint
       bp_access_watchpoint
+      bp_catch_syscall
       bp_catch_exec
       bp_catch_fork
       bp_catch_vork */
@@ -4163,6 +4266,7 @@ breakpoint_address_is_meaningful (struct breakpoint *bpt)
 	  && type != bp_hardware_watchpoint
 	  && type != bp_read_watchpoint
 	  && type != bp_access_watchpoint
+          && type != bp_catch_syscall
 	  && type != bp_catch_exec
 	  && type != bp_catch_fork
 	  && type != bp_catch_vfork);
@@ -4282,7 +4386,8 @@ adjust_breakpoint_address (CORE_ADDR bpaddr, enum bptype bptype)
            || bptype == bp_access_watchpoint
            || bptype == bp_catch_fork
            || bptype == bp_catch_vfork
-           || bptype == bp_catch_exec)
+           || bptype == bp_catch_exec
+	   || bptype == bp_catch_syscall)
     {
       /* Watchpoints and the various bp_catch_* eventpoints should not
          have their addresses modified.  */
@@ -4333,6 +4438,7 @@ allocate_bp_location (struct breakpoint *bpt, enum bptype bp_type)
     case bp_watchpoint_scope:
     case bp_call_dummy:
     case bp_shlib_event:
+    case bp_entry_breakpoint:
     case bp_thread_event:
     case bp_overlay_event:
     case bp_catch_load:
@@ -4351,6 +4457,7 @@ allocate_bp_location (struct breakpoint *bpt, enum bptype bp_type)
     case bp_catch_fork:
     case bp_catch_vfork:
     case bp_catch_exec:
+    case bp_catch_syscall:
       loc->loc_type = bp_loc_other;
       break;
     default:
@@ -4396,6 +4503,8 @@ set_raw_breakpoint_without_location (enum bptype bptype)
   b->triggered_dll_pathname = NULL;
   b->forked_inferior_pid = null_ptid;
   b->exec_pathname = NULL;
+  b->syscall_to_be_caught = CATCHING_ANY_SYSCALL;
+  b->syscall_number = UNKNOWN_SYSCALL;
   b->ops = NULL;
   b->condition_not_parsed = 0;
 
@@ -4618,6 +4727,31 @@ disable_overlay_breakpoints (void)
     }
 }
 
+int
+create_entry_breakpoint ()
+{
+  CORE_ADDR taddr, entry_addr;
+  struct breakpoint *b;
+
+  taddr = entry_point_address ();
+  /* Make certain that the address points at real code, and not a
+     function descriptor.  */
+  entry_addr = gdbarch_convert_from_func_ptr_addr (current_gdbarch, taddr,
+                                                   &current_target);
+
+  /* Setting the breakpoint */
+  b = create_internal_breakpoint (entry_addr, bp_entry_breakpoint);
+
+  b->enable_state = bp_enabled;
+  b->disposition = disp_del;
+  /* addr_string has to be used or breakpoint_re_set will delete me. */
+  b->addr_string = xstrprintf ("AT_ENTRY (0x%s)", paddr (entry_addr));
+
+  update_global_location_list (1);
+
+  return 1;
+}
+
 struct breakpoint *
 create_thread_event_breakpoint (CORE_ADDR address)
 {
@@ -4817,6 +4951,31 @@ create_exec_event_catchpoint (int tempflag, char *cond_string)
   mention (b);
 }
 
+static void
+create_syscall_event_catchpoint (int tempflag, int syscall_number)
+{
+  struct symtab_and_line sal;
+  struct breakpoint *b;
+  int thread = -1;		/* All threads. */
+
+  init_sal (&sal);
+
+  b = set_raw_breakpoint (sal, bp_catch_syscall);
+  set_breakpoint_count (breakpoint_count + 1);
+  b->number = breakpoint_count;
+  b->cond_string = NULL;
+  b->thread = thread;
+  b->syscall_to_be_caught = syscall_number;
+  /* We still don't know the syscall that will be caught :-). */
+  b->syscall_number = UNKNOWN_SYSCALL;
+  b->addr_string = NULL;
+  b->enable_state = bp_enabled;
+  b->disposition = tempflag ? disp_del : disp_donttouch;
+  update_global_location_list (1);
+
+  mention (b);
+}
+
 static int
 hw_breakpoint_used_count (void)
 {
@@ -5034,6 +5193,16 @@ mention (struct breakpoint *b)
 	printf_filtered (_("Catchpoint %d (exec)"),
 			 b->number);
 	break;
+      case bp_catch_syscall:
+        if (b->syscall_to_be_caught != CATCHING_ANY_SYSCALL)
+          printf_filtered (_("Catchpoint %d (syscall '%s ()')"),
+                           b->number,
+                           gdbarch_syscall_name_from_number (current_gdbarch,
+                                                    b->syscall_to_be_caught));
+        else
+          printf_filtered (_("Catchpoint %d (syscall)"),
+                           b->number);
+	break;
 
       case bp_until:
       case bp_finish:
@@ -5045,6 +5214,7 @@ mention (struct breakpoint *b)
       case bp_shlib_event:
       case bp_thread_event:
       case bp_overlay_event:
+      case bp_entry_breakpoint:
 	break;
       }
 
@@ -6795,6 +6965,36 @@ catch_ada_exception_command (char *arg, int from_tty,
                                    from_tty);
 }
 
+/* Implement the "catch syscall" command. */
+
+static void
+catch_syscall_command_1 (char *arg, int from_tty, struct cmd_list_element *command)
+{
+  int tempflag;
+  int syscall_number = CATCHING_ANY_SYSCALL;
+
+  tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
+
+  ep_skip_leading_whitespace (&arg);
+
+  /* The allowed syntax is:
+     catch syscall
+     catch syscall <name>
+
+     Let's check if there's a syscall name. */
+
+  if (arg != NULL)
+    {
+      syscall_number = gdbarch_syscall_number_from_name (current_gdbarch,
+                                                         (const char *) arg);
+      if (syscall_number == UNKNOWN_SYSCALL)
+        error (_("Invalid syscall name '%s'."), arg);
+    }
+
+  /* Now let's create the catchpoint */
+  create_syscall_event_catchpoint (tempflag, syscall_number);
+}
+
 /* Implement the "catch assert" command.  */
 
 static void
@@ -7312,6 +7512,7 @@ delete_command (char *arg, int from_tty)
 	    b->type != bp_shlib_event &&
 	    b->type != bp_thread_event &&
 	    b->type != bp_overlay_event &&
+            b->type != bp_entry_breakpoint &&
 	    b->number >= 0)
 	  {
 	    breaks_to_delete = 1;
@@ -7329,6 +7530,7 @@ delete_command (char *arg, int from_tty)
 		b->type != bp_shlib_event &&
 		b->type != bp_thread_event &&
 		b->type != bp_overlay_event &&
+                b->type != bp_entry_breakpoint &&
 		b->number >= 0)
 	      delete_breakpoint (b);
 	  }
@@ -7620,6 +7822,7 @@ breakpoint_re_set_one (void *bint)
     case bp_catch_fork:
     case bp_catch_vfork:
     case bp_catch_exec:
+    case bp_catch_syscall:
       break;
 
     default:
@@ -7639,6 +7842,9 @@ breakpoint_re_set_one (void *bint)
 	 Once it is set up, we do not want to touch it.  */
     case bp_thread_event:
 
+      /* Same for this one */
+    case bp_entry_breakpoint:
+
       /* Keep temporary breakpoints, which can be encountered when we step
          over a dlopen call and SOLIB_ADD is resetting the breakpoints.
          Otherwise these should have been blown away via the cleanup chain
@@ -7893,6 +8099,7 @@ disable_command (char *args, int from_tty)
       case bp_catch_fork:
       case bp_catch_vfork:
       case bp_catch_exec:
+      case bp_catch_syscall:
       case bp_hardware_breakpoint:
       case bp_watchpoint:
       case bp_hardware_watchpoint:
@@ -8027,6 +8234,7 @@ enable_command (char *args, int from_tty)
       case bp_catch_fork:
       case bp_catch_vfork:
       case bp_catch_exec:
+      case bp_catch_syscall:
       case bp_hardware_breakpoint:
       case bp_watchpoint:
       case bp_hardware_watchpoint:
@@ -8209,6 +8417,45 @@ single_step_breakpoint_inserted_here_p (CORE_ADDR pc)
   return 0;
 }
 
+/* Returns 0 if 'bp' is NOT a syscall catchpoint,
+   non-zero otherwise. */
+static int
+is_syscall_catchpoint_enabled (struct breakpoint *bp)
+{
+  if (bp->type == bp_catch_syscall
+      && bp->enable_state != bp_disabled
+      && bp->enable_state != bp_call_disabled)
+    return 1;
+  else
+    return 0;
+}
+
+int
+catch_syscall_enabled (void)
+{
+  struct breakpoint *bp;
+
+  ALL_BREAKPOINTS (bp)
+    if (is_syscall_catchpoint_enabled (bp))
+      return 1;
+
+  return 0;
+}
+
+int
+catching_syscall_number (int syscall_number)
+{
+  struct breakpoint *bp;
+
+  ALL_BREAKPOINTS (bp)
+    if (is_syscall_catchpoint_enabled (bp))
+      if (bp->syscall_to_be_caught == syscall_number
+          || bp->syscall_to_be_caught == CATCHING_ANY_SYSCALL)
+        return 1;
+
+  return 0;
+}
+
 \f
 /* This help string is used for the break, hbreak, tbreak and thbreak commands.
    It is defined as a macro to prevent duplication.
@@ -8549,6 +8796,12 @@ With an argument, catch only exceptions with the given name."),
 		     catch_exec_command_1,
 		     CATCH_PERMANENT,
 		     CATCH_TEMPORARY);
+  add_catch_command ("syscall", _("\
+Catch calls to syscalls.\n\
+With an argument, catch only calls of that syscall."),
+                     catch_syscall_command_1,
+                     CATCH_PERMANENT,
+                     CATCH_TEMPORARY);
   add_catch_command ("load", _("\
 Catch library loads.\n\
 With an argument, catch only loads of that library."),
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index db6e972..bc3de7f 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -33,6 +33,11 @@ struct block;
 
 #define	BREAKPOINT_MAX	16
 \f
+
+/* A number to represent wether we are catching any syscalls. */
+
+#define CATCHING_ANY_SYSCALL (-1)
+
 /* Type of breakpoint. */
 /* FIXME In the future, we should fold all other breakpoint-like things into
    here.  This includes:
@@ -127,6 +132,14 @@ enum bptype
     bp_catch_fork,
     bp_catch_vfork,
     bp_catch_exec,
+
+    /* This is not really a breakpoint, but the catchpoint which implements
+       the "catch syscall" functionality. */
+    bp_catch_syscall,
+
+    /* This type is used to signal an internal breakpoint located at
+       the AT_ENTRY address. */
+    bp_entry_breakpoint,
   };
 
 /* States of enablement of breakpoint. */
@@ -455,6 +468,21 @@ struct breakpoint
        triggered.  */
     char *exec_pathname;
 
+    /* Syscall number used for the 'catch syscall' feature.
+       If no syscall has been called, its value is UNKNOWN_SYSCALL.
+       Otherwise, it holds the system call number in the target.
+
+       This field is only valid immediately after this catchpoint has
+       triggered.  */
+    int syscall_number;
+
+    /* This field is used when we are "filtering" the syscalls
+       (i.e., when the user types "catch syscall <SYSCALL_NAME>".
+
+       It stores the syscall number in case we are in the "filter mode",
+       or CATCHING_ANY_SYSCALL otherwise. */
+    int syscall_to_be_caught;
+
     /* Methods associated with this breakpoint.  */
     struct breakpoint_ops *ops;
 
@@ -536,6 +564,10 @@ enum bpstat_what_main_action
        resume out of the dynamic linker's callback, stop and print.  */
     BPSTAT_WHAT_CHECK_SHLIBS_RESUME_FROM_HOOK,
 
+    /* This internal breakpoint is used syscall catchpoints only after the
+       shell and the dynamic linker have already ran. */
+    BPSTAT_WHAT_ENTRY_BREAKPOINT,
+
     /* This is just used to keep track of how many enums there are.  */
     BPSTAT_WHAT_LAST
   };
@@ -805,6 +837,8 @@ extern void enable_watchpoints_after_interactive_call_stop (void);
 extern enum command_control_type commands_from_control_command
   (char *arg, struct command_line *cmd);
 
+extern void clear_syscall_catchpoints_info (void);
+
 extern void clear_breakpoint_hit_counts (void);
 
 extern int get_number (char **);
@@ -884,4 +918,26 @@ extern int breakpoints_always_inserted_mode (void);
    in our opinion won't ever trigger.  */
 extern void breakpoint_retire_moribund (void);
 
+/* Checks if we are catching syscalls or not.
+   Returns 0 if not, greater than 0 if we are. */
+extern int catch_syscall_enabled (void);
+
+/* Checks if we are catching syscalls with the specific
+   syscall_number. Used for "filtering" the catchpoints.
+   Returns 0 if not, greater than 0 if we are. */
+extern int catching_syscall_number (int syscall_number);
+
+/* Function used to set an internal breakpoint at the AT_ENTRY
+   (a.k.a. the entry point of the inferior).
+
+   This is currently needed for us to know when to start setting
+   up catchpoints for syscalls in the inferior. If we don't do that,
+   then we would set a "catch syscall" too early, which would
+   catch syscalls from ld.so and/or libc (and we don't want that).
+
+   Returns zero if there was an error setting this breakpoint,
+   or 1 if everything went OK. */
+extern int create_entry_breakpoint (void);
+
+
 #endif /* !defined (BREAKPOINT_H) */
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index aa9a455..f4b3dec 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -240,6 +240,9 @@ struct gdbarch
   gdbarch_target_signal_from_host_ftype *target_signal_from_host;
   gdbarch_target_signal_to_host_ftype *target_signal_to_host;
   gdbarch_record_special_symbol_ftype *record_special_symbol;
+  gdbarch_get_syscall_number_ftype *get_syscall_number;
+  gdbarch_syscall_name_from_number_ftype *syscall_name_from_number;
+  gdbarch_syscall_number_from_name_ftype *syscall_number_from_name;
 };
 
 
@@ -371,6 +374,9 @@ struct gdbarch startup_gdbarch =
   default_target_signal_from_host,  /* target_signal_from_host */
   default_target_signal_to_host,  /* target_signal_to_host */
   0,  /* record_special_symbol */
+  0,  /* get_syscall_number */
+  0,  /* syscall_name_from_number */
+  0,  /* syscall_number_from_name */
   /* startup_gdbarch() */
 };
 
@@ -623,6 +629,9 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of target_signal_from_host, invalid_p == 0 */
   /* Skip verify of target_signal_to_host, invalid_p == 0 */
   /* Skip verify of record_special_symbol, has predicate */
+  /* Skip verify of get_syscall_number, has predicate */
+  /* Skip verify of syscall_name_from_number, has predicate */
+  /* Skip verify of syscall_number_from_name, has predicate */
   buf = ui_file_xstrdup (log, &dummy);
   make_cleanup (xfree, buf);
   if (strlen (buf) > 0)
@@ -832,6 +841,12 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: get_longjmp_target = <0x%lx>\n",
                       (long) gdbarch->get_longjmp_target);
   fprintf_unfiltered (file,
+                      "gdbarch_dump: gdbarch_get_syscall_number_p() = %d\n",
+                      gdbarch_get_syscall_number_p (gdbarch));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: get_syscall_number = <0x%lx>\n",
+                      (long) gdbarch->get_syscall_number);
+  fprintf_unfiltered (file,
                       "gdbarch_dump: have_nonsteppable_watchpoint = %s\n",
                       plongest (gdbarch->have_nonsteppable_watchpoint));
   fprintf_unfiltered (file,
@@ -1051,6 +1066,18 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: static_transform_name = <0x%lx>\n",
                       (long) gdbarch->static_transform_name);
   fprintf_unfiltered (file,
+                      "gdbarch_dump: gdbarch_syscall_name_from_number_p() = %d\n",
+                      gdbarch_syscall_name_from_number_p (gdbarch));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: syscall_name_from_number = <0x%lx>\n",
+                      (long) gdbarch->syscall_name_from_number);
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: gdbarch_syscall_number_from_name_p() = %d\n",
+                      gdbarch_syscall_number_from_name_p (gdbarch));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: syscall_number_from_name = <0x%lx>\n",
+                      (long) gdbarch->syscall_number_from_name);
+  fprintf_unfiltered (file,
                       "gdbarch_dump: target_desc = %s\n",
                       plongest ((long) gdbarch->target_desc));
   fprintf_unfiltered (file,
@@ -3237,6 +3264,78 @@ set_gdbarch_record_special_symbol (struct gdbarch *gdbarch,
   gdbarch->record_special_symbol = record_special_symbol;
 }
 
+int
+gdbarch_get_syscall_number_p (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  return gdbarch->get_syscall_number != NULL;
+}
+
+LONGEST
+gdbarch_get_syscall_number (struct gdbarch *gdbarch, ptid_t ptid)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->get_syscall_number != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_get_syscall_number called\n");
+  return gdbarch->get_syscall_number (gdbarch, ptid);
+}
+
+void
+set_gdbarch_get_syscall_number (struct gdbarch *gdbarch,
+                                gdbarch_get_syscall_number_ftype get_syscall_number)
+{
+  gdbarch->get_syscall_number = get_syscall_number;
+}
+
+int
+gdbarch_syscall_name_from_number_p (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  return gdbarch->syscall_name_from_number != NULL;
+}
+
+const char *
+gdbarch_syscall_name_from_number (struct gdbarch *gdbarch, int syscall_number)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->syscall_name_from_number != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_syscall_name_from_number called\n");
+  return gdbarch->syscall_name_from_number (gdbarch, syscall_number);
+}
+
+void
+set_gdbarch_syscall_name_from_number (struct gdbarch *gdbarch,
+                                      gdbarch_syscall_name_from_number_ftype syscall_name_from_number)
+{
+  gdbarch->syscall_name_from_number = syscall_name_from_number;
+}
+
+int
+gdbarch_syscall_number_from_name_p (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  return gdbarch->syscall_number_from_name != NULL;
+}
+
+int
+gdbarch_syscall_number_from_name (struct gdbarch *gdbarch, const char *syscall_name)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->syscall_number_from_name != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_syscall_number_from_name called\n");
+  return gdbarch->syscall_number_from_name (gdbarch, syscall_name);
+}
+
+void
+set_gdbarch_syscall_number_from_name (struct gdbarch *gdbarch,
+                                      gdbarch_syscall_number_from_name_ftype syscall_number_from_name)
+{
+  gdbarch->syscall_number_from_name = syscall_number_from_name;
+}
+
 
 /* Keep a registry of per-architecture data-pointers required by GDB
    modules. */
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index bc8298d..a2b4468 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -811,6 +811,37 @@ typedef void (gdbarch_record_special_symbol_ftype) (struct gdbarch *gdbarch, str
 extern void gdbarch_record_special_symbol (struct gdbarch *gdbarch, struct objfile *objfile, asymbol *sym);
 extern void set_gdbarch_record_special_symbol (struct gdbarch *gdbarch, gdbarch_record_special_symbol_ftype *record_special_symbol);
 
+/* Functions for the 'catch syscall' feature.
+   Get architecture-specific system calls information from registers. */
+
+extern int gdbarch_get_syscall_number_p (struct gdbarch *gdbarch);
+
+typedef LONGEST (gdbarch_get_syscall_number_ftype) (struct gdbarch *gdbarch, ptid_t ptid);
+extern LONGEST gdbarch_get_syscall_number (struct gdbarch *gdbarch, ptid_t ptid);
+extern void set_gdbarch_get_syscall_number (struct gdbarch *gdbarch, gdbarch_get_syscall_number_ftype *get_syscall_number);
+
+/* Translate a syscall number to its corresponding name. */
+
+extern int gdbarch_syscall_name_from_number_p (struct gdbarch *gdbarch);
+
+typedef const char * (gdbarch_syscall_name_from_number_ftype) (struct gdbarch *gdbarch, int syscall_number);
+extern const char * gdbarch_syscall_name_from_number (struct gdbarch *gdbarch, int syscall_number);
+extern void set_gdbarch_syscall_name_from_number (struct gdbarch *gdbarch, gdbarch_syscall_name_from_number_ftype *syscall_name_from_number);
+
+/* Translate a syscall name to its corresponding number.
+  
+   This function must return the syscall number if found, or
+   UNKNOWN_SYSCALL if not found. */
+
+extern int gdbarch_syscall_number_from_name_p (struct gdbarch *gdbarch);
+
+typedef int (gdbarch_syscall_number_from_name_ftype) (struct gdbarch *gdbarch, const char *syscall_name);
+extern int gdbarch_syscall_number_from_name (struct gdbarch *gdbarch, const char *syscall_name);
+extern void set_gdbarch_syscall_number_from_name (struct gdbarch *gdbarch, gdbarch_syscall_number_from_name_ftype *syscall_number_from_name);
+
+/* Definition for an unknown syscall, used basically in error-cases. */
+#define UNKNOWN_SYSCALL (-1)
+
 extern struct gdbarch_tdep *gdbarch_tdep (struct gdbarch *gdbarch);
 
 
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 0c513a5..684bb8c 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -707,6 +707,20 @@ m:int:target_signal_to_host:enum target_signal ts:ts::default_target_signal_to_h
 
 # Record architecture-specific information from the symbol table.
 M:void:record_special_symbol:struct objfile *objfile, asymbol *sym:objfile, sym
+
+# Functions for the 'catch syscall' feature.
+
+# Get architecture-specific system calls information from registers.
+M:LONGEST:get_syscall_number:ptid_t ptid:ptid
+
+# Translate a syscall number to its corresponding name.
+M:const char *:syscall_name_from_number:int syscall_number:syscall_number
+
+# Translate a syscall name to its corresponding number.
+#
+# This function must return the syscall number if found, or
+# UNKNOWN_SYSCALL if not found.
+M:int:syscall_number_from_name:const char *syscall_name:syscall_name
 EOF
 }
 
@@ -888,6 +902,9 @@ done
 # close it off
 cat <<EOF
 
+/* Definition for an unknown syscall, used basically in error-cases. */
+#define UNKNOWN_SYSCALL (-1)
+
 extern struct gdbarch_tdep *gdbarch_tdep (struct gdbarch *gdbarch);
 
 
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 0fb53fb..85245ca 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -170,6 +170,13 @@ struct thread_info
 
   /* Private data used by the target vector implementation.  */
   struct private_thread_info *private;
+
+  /* Signal wether we are in a SYSCALL_ENTRY or
+     in a SYSCALL_RETURN event.
+     Values:
+     - TARGET_WAITKIND_SYSCALL_ENTRY
+     - TARGET_WAITKIND_SYSCALL_RETURN */
+  int syscall_state;
 };
 
 /* Create an empty thread list, or empty the existing one.  */
diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 8da25f4..058cb34 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -144,6 +144,21 @@ inf_child_remove_exec_catchpoint (int pid)
   return 0;
 }
 
+static void
+inf_child_insert_syscall_catchpoint (int pid)
+{
+  /* This version of Unix doesn't support notification of syscall
+     events.  */
+}
+
+static int
+inf_child_remove_syscall_catchpoint (int pid)
+{
+  /* This version of Unix doesn't support notification of syscall
+     events.  */
+  return 0;
+}
+
 static int
 inf_child_can_run (void)
 {
@@ -187,6 +202,8 @@ inf_child_target (void)
   t->to_follow_fork = inf_child_follow_fork;
   t->to_insert_exec_catchpoint = inf_child_insert_exec_catchpoint;
   t->to_remove_exec_catchpoint = inf_child_remove_exec_catchpoint;
+  t->to_insert_syscall_catchpoint = inf_child_insert_syscall_catchpoint;
+  t->to_remove_syscall_catchpoint = inf_child_remove_syscall_catchpoint;
   t->to_can_run = inf_child_can_run;
   t->to_pid_to_exec_file = inf_child_pid_to_exec_file;
   t->to_stratum = process_stratum;
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 57af79a..65e7acd 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -355,13 +355,19 @@ static void
 inf_ptrace_resume (ptid_t ptid, int step, enum target_signal signal)
 {
   pid_t pid = ptid_get_pid (ptid);
-  int request = PT_CONTINUE;
+  int request;
 
   if (pid == -1)
     /* Resume all threads.  Traditionally ptrace() only supports
        single-threaded processes, so simply resume the inferior.  */
     pid = ptid_get_pid (inferior_ptid);
 
+  if (target_passed_by_entrypoint () > 0
+      && catch_syscall_enabled () > 0)
+    request = PT_SYSCALL;
+  else
+    request = PT_CONTINUE;
+
   if (step)
     {
       /* If this system does not support PT_STEP, a higher level
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 6ed6341..09d4d9b 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -450,6 +450,11 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
   init_wait_for_inferior ();
   clear_breakpoint_hit_counts ();
 
+  /* If we already caught a syscall catchpoint, then reset its
+     syscall_number information because we are starting all over
+     again. */
+  clear_syscall_catchpoints_info ();
+
   /* Clean up any leftovers from other runs.  Some other things from
      this function should probably be moved into target_pre_inferior.  */
   target_pre_inferior (from_tty);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 4b4df8f..9c5efef 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -964,7 +964,7 @@ a command like `return' or `jump' to continue execution."));
         }
     }
 
-  /* If there were any forks/vforks/execs that were caught and are
+  /* If there were any forks/vforks/execs/syscalls that were caught and are
      now to be followed, then do so.  */
   switch (pending_follow.kind)
     {
@@ -980,6 +980,11 @@ a command like `return' or `jump' to continue execution."));
       pending_follow.kind = TARGET_WAITKIND_SPURIOUS;
       break;
 
+    case TARGET_WAITKIND_SYSCALL_ENTRY:
+    case TARGET_WAITKIND_SYSCALL_RETURN:
+      pending_follow.kind = TARGET_WAITKIND_SPURIOUS;
+      break;
+
     default:
       break;
     }
@@ -1386,7 +1391,7 @@ init_wait_for_inferior (void)
 
   breakpoint_init_inferior (inf_starting);
 
-  /* The first resume is not following a fork/vfork/exec. */
+  /* The first resume is not following a fork/vfork/exec/syscall. */
   pending_follow.kind = TARGET_WAITKIND_SPURIOUS;	/* I.e., none. */
 
   clear_proceed_status ();
@@ -1831,6 +1836,50 @@ ensure_not_running (void)
     error_is_running ();
 }
 
+/* Auxiliary function that handles syscall entry/return events.
+   It returns 1 if the inferior should keep going (and GDB
+   should ignore the event), or 0 if the event deserves to be
+   processed. */
+static int
+deal_with_syscall_event (struct execution_control_state *ecs)
+{
+  int syscall_number = gdbarch_get_syscall_number (current_gdbarch,
+                                                   ecs->ptid);
+  if (catch_syscall_enabled () > 0
+      && catching_syscall_number (syscall_number) > 0)
+    {
+      ecs->event_thread->stop_signal = TARGET_SIGNAL_TRAP;
+      pending_follow.kind = ecs->ws.kind;
+
+      if (!ptid_equal (ecs->ptid, inferior_ptid))
+        {
+          context_switch (ecs->ptid);
+          reinit_frame_cache ();
+        }
+
+      stop_pc = read_pc ();
+
+      ecs->event_thread->stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid);
+
+      ecs->random_signal = !bpstat_explains_signal (ecs->event_thread->stop_bpstat);
+
+      /* If no catchpoint triggered for this, then keep going.  */
+      if (ecs->random_signal)
+        {
+          ecs->event_thread->stop_signal = TARGET_SIGNAL_0;
+          keep_going (ecs);
+          return 1;
+        }
+      return 0;
+    }
+  else
+    {
+      resume (0, TARGET_SIGNAL_0);
+      prepare_to_wait (ecs);
+      return 1;
+    }
+}
+
 /* Given an execution control state that has been freshly filled in
    by an event from the inferior, figure out what it means and take
    appropriate action.  */
@@ -2119,9 +2168,11 @@ handle_inferior_event (struct execution_control_state *ecs)
     case TARGET_WAITKIND_SYSCALL_ENTRY:
       if (debug_infrun)
         fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_SYSCALL_ENTRY\n");
-      resume (0, TARGET_SIGNAL_0);
-      prepare_to_wait (ecs);
-      return;
+      /* Getting the current syscall number */
+      if (deal_with_syscall_event (ecs) != 0)
+        return;
+      goto process_event_stop_test;
+      break;
 
       /* Before examining the threads further, step this thread to
          get it entirely out of the syscall.  (We get notice of the
@@ -2131,9 +2182,10 @@ handle_inferior_event (struct execution_control_state *ecs)
     case TARGET_WAITKIND_SYSCALL_RETURN:
       if (debug_infrun)
         fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_SYSCALL_RETURN\n");
-      target_resume (ecs->ptid, 1, TARGET_SIGNAL_0);
-      prepare_to_wait (ecs);
-      return;
+      if (deal_with_syscall_event (ecs) != 0)
+        return;
+      goto process_event_stop_test;
+      break;
 
     case TARGET_WAITKIND_STOPPED:
       if (debug_infrun)
@@ -2951,6 +3003,16 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n");
 	}
 	break;
 
+      case BPSTAT_WHAT_ENTRY_BREAKPOINT:
+        /* We hit the AT_ENTRY breakpoint, and now we have to enable
+           the PTRACE_O_TRACESYSGOOD option in the inferior *if* we
+           are catching syscalls. */
+        if (debug_infrun)
+          fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_ENTRY_BREAKPOINT\n");
+        target_enable_tracesysgood (ecs->ptid);
+        ecs->event_thread->stepping_over_breakpoint = 1;
+        break;
+
       case BPSTAT_WHAT_LAST:
 	/* Not a real code, but listed here to shut up gcc -Wall.  */
 
@@ -4563,6 +4625,25 @@ inferior_has_execd (ptid_t pid, char **execd_pathname)
   return 1;
 }
 
+int
+inferior_has_called_syscall (ptid_t pid, int *syscall_number)
+{
+  struct target_waitstatus last;
+  ptid_t last_ptid;
+
+  get_last_target_status (&last_ptid, &last);
+
+  if (last.kind != TARGET_WAITKIND_SYSCALL_ENTRY &&
+      last.kind != TARGET_WAITKIND_SYSCALL_RETURN)
+    return 0;
+
+  if (!ptid_equal (last_ptid, pid))
+    return 0;
+
+  *syscall_number = last.value.syscall_number;
+  return 1;
+}
+
 /* Oft used ptids */
 ptid_t null_ptid;
 ptid_t minus_one_ptid;
diff --git a/gdb/target.c b/gdb/target.c
index a509c17..39a92b5 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -426,6 +426,10 @@ update_current_target (void)
       /* Do not inherit to_follow_fork.  */
       INHERIT (to_insert_exec_catchpoint, t);
       INHERIT (to_remove_exec_catchpoint, t);
+      INHERIT (to_passed_by_entrypoint, t);
+      INHERIT (to_insert_syscall_catchpoint, t);
+      INHERIT (to_remove_syscall_catchpoint, t);
+      INHERIT (to_enable_tracesysgood, t);
       INHERIT (to_has_exited, t);
       INHERIT (to_mourn_inferior, t);
       INHERIT (to_can_run, t);
@@ -581,9 +585,21 @@ update_current_target (void)
   de_fault (to_insert_exec_catchpoint,
 	    (void (*) (int))
 	    tcomplain);
+  de_fault (to_passed_by_entrypoint,
+            (int (*) (void))
+            tcomplain);
   de_fault (to_remove_exec_catchpoint,
 	    (int (*) (int))
 	    tcomplain);
+  de_fault (to_insert_syscall_catchpoint,
+	    (void (*) (int))
+	    tcomplain);
+  de_fault (to_remove_syscall_catchpoint,
+	    (int (*) (int))
+	    tcomplain);
+  de_fault (to_enable_tracesysgood,
+            (void (*) (ptid_t))
+            tcomplain);
   de_fault (to_has_exited,
 	    (int (*) (int, int, int *))
 	    return_zero);
@@ -2532,6 +2548,12 @@ debug_to_wait (ptid_t ptid, struct target_waitstatus *status)
     case TARGET_WAITKIND_EXECD:
       fprintf_unfiltered (gdb_stdlog, "execd\n");
       break;
+    case TARGET_WAITKIND_SYSCALL_ENTRY:
+      fprintf_unfiltered (gdb_stdlog, "entered syscall\n");
+      break;
+    case TARGET_WAITKIND_SYSCALL_RETURN:
+      fprintf_unfiltered (gdb_stdlog, "exited syscall\n");
+      break;
     case TARGET_WAITKIND_SPURIOUS:
       fprintf_unfiltered (gdb_stdlog, "spurious\n");
       break;
diff --git a/gdb/target.h b/gdb/target.h
index 067c031..6e379d3 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -135,14 +135,15 @@ struct target_waitstatus
   {
     enum target_waitkind kind;
 
-    /* Forked child pid, execd pathname, exit status or signal number.  */
+    /* Forked child pid, execd pathname, exit status, signal number or
+       syscall name. */
     union
       {
 	int integer;
 	enum target_signal sig;
 	ptid_t related_pid;
 	char *execd_pathname;
-	int syscall_id;
+	int syscall_number;
       }
     value;
   };
@@ -393,6 +394,10 @@ struct target_ops
     int (*to_follow_fork) (struct target_ops *, int);
     void (*to_insert_exec_catchpoint) (int);
     int (*to_remove_exec_catchpoint) (int);
+    int (*to_passed_by_entrypoint) (void);
+    void (*to_insert_syscall_catchpoint) (int);
+    int (*to_remove_syscall_catchpoint) (int);
+    void (*to_enable_tracesysgood) (ptid_t);
     int (*to_has_exited) (int, int, int *);
     void (*to_mourn_inferior) (void);
     int (*to_can_run) (void);
@@ -708,6 +713,8 @@ extern int inferior_has_vforked (ptid_t pid, ptid_t *child_pid);
 
 extern int inferior_has_execd (ptid_t pid, char **execd_pathname);
 
+extern int inferior_has_called_syscall (ptid_t pid, int *syscall_number);
+
 /* From exec.c */
 
 extern void print_section_info (struct target_ops *, bfd *);
@@ -867,6 +874,24 @@ int target_follow_fork (int follow_child);
 #define target_remove_exec_catchpoint(pid) \
      (*current_target.to_remove_exec_catchpoint) (pid)
 
+/* Has the inferior already passed through its entrypoint? */
+#define target_passed_by_entrypoint() \
+     (*current_target.to_passed_by_entrypoint) ()
+
+/* Syscall catch functions */
+
+#define target_insert_syscall_catchpoint(pid) \
+     (*current_target.to_insert_syscall_catchpoint) (pid)
+
+#define target_remove_syscall_catchpoint(pid) \
+     (*current_target.to_remove_syscall_catchpoint) (pid)
+
+/* Enable PTRACE_O_TRACESYSGOOD in the inferior.
+   This is mainly used for the "catch syscall" feature. */
+
+#define target_enable_tracesysgood(ptid) \
+     (*current_target.to_enable_tracesysgood) (ptid)
+
 /* Returns TRUE if PID has exited.  And, also sets EXIT_STATUS to the
    exit code of PID, if any.  */
 



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

end of thread, other threads:[~2008-11-08 19:31 UTC | newest]

Thread overview: 57+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-11-04  4:32 [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part Sérgio Durigan Júnior
2008-11-04 16:17 ` Pedro Alves
2008-11-07  3:30   ` Sérgio Durigan Júnior
2008-11-07 12:12     ` Pedro Alves
2008-11-07 13:30       ` Daniel Jacobowitz
2008-11-08 15:35       ` Sérgio Durigan Júnior
2008-11-04 17:57 ` Tom Tromey
2008-11-04 21:55   ` Thiago Jung Bauermann
2008-11-04 22:33     ` Tom Tromey
2008-11-05 19:05       ` Tom Tromey
2008-11-05 19:13         ` Sérgio Durigan Júnior
2008-11-07  3:41         ` Sérgio Durigan Júnior
2008-11-07  3:39   ` Sérgio Durigan Júnior
2008-11-07 18:21     ` Tom Tromey
2008-11-04 21:13 ` Eli Zaretskii
2008-11-04 22:12   ` Thiago Jung Bauermann
2008-11-04 22:22     ` Eli Zaretskii
2008-11-04 22:35       ` Daniel Jacobowitz
2008-11-05  4:19         ` Eli Zaretskii
2008-11-05 13:34           ` Sérgio Durigan Júnior
2008-11-05 18:42             ` Eli Zaretskii
2008-11-08 19:31             ` Mark Kettenis
2008-11-05 14:55           ` Daniel Jacobowitz
2008-11-05 18:43             ` Eli Zaretskii
2008-11-05 18:59               ` Daniel Jacobowitz
2008-11-05 19:11                 ` Eli Zaretskii
2008-11-06 23:03               ` Mark Kettenis
2008-11-04 22:31     ` Pedro Alves
2008-11-05  4:10       ` Eli Zaretskii
2008-11-05 12:29         ` Pedro Alves
2008-11-05 18:38           ` Eli Zaretskii
2008-11-05 18:57             ` Pedro Alves
2008-11-05 19:10               ` Eli Zaretskii
2008-11-05 19:34                 ` Pedro Alves
2008-11-05 20:36                   ` Eli Zaretskii
2008-11-05 21:10                     ` Pedro Alves
2008-11-06  4:27                       ` Eli Zaretskii
2008-11-06 14:32                         ` Pedro Alves
2008-11-07  9:59                           ` Eli Zaretskii
2008-11-07 10:10                             ` Pedro Alves
2008-11-05 13:32         ` Mark Kettenis
  -- strict thread matches above, loose matches on Subject: below --
2008-09-30 18:12 Sérgio Durigan Júnior
2008-10-02 21:13 ` Joel Brobecker
2008-10-03  2:33   ` Sérgio Durigan Júnior
2008-10-03  6:07     ` Joel Brobecker
2008-10-03 17:52       ` Daniel Jacobowitz
2008-10-04 23:07         ` Sérgio Durigan Júnior
2008-10-04 23:04       ` Sérgio Durigan Júnior
2008-10-06 17:22         ` Joel Brobecker
2008-10-10 13:12           ` Daniel Jacobowitz
2008-10-10 15:28           ` Sérgio Durigan Júnior
2008-10-12  2:26           ` Sérgio Durigan Júnior
2008-10-15  5:40             ` Joel Brobecker
2008-10-16  3:35               ` Sérgio Durigan Júnior
2008-10-16 12:37                 ` Daniel Jacobowitz
2008-10-16 15:17                   ` Daniel Jacobowitz
2008-10-16 16:28                     ` Joel Brobecker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox