Re: [commit] Reimplement "catch exec" using bp_catchpoint

[Joel Brobecker <brobecker@adacore.com>]

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

> gdb/
> 2008-10-27  Joel Brobecker  <brobecker@adacore.com>
> 
>         * breakpoint.h (enum bptype): Delete bp_catch_exec.
>         * breakpoint.c (insert_catchpoint): Remove handling for
>         bp_catch_exec breakpoint kinds.
>         (insert_bp_location, update_breakpoints_after_exec, remove_breakpoint)
>         (ep_is_catchpoint, print_it_typical, bpstat_check_location),
>         (bpstat_check_location, bpstat_what, print_one_breakpoint_location)
>         (print_one_breakpoint_location, user_settable_breakpoint)
>         (breakpoint_address_is_meaningful, adjust_breakpoint_address)
>         (allocate_bp_location, mention, breakpoint_re_set_one)
>         (disable_command, enable_command): Likewise.
>         (create_exec_event_catchpoint): Delete.
>         (insert_catch_exec, remove_catch_exec, breakpoint_hit_catch_exec)
>         (print_it_catch_exec, print_one_catch_exec, print_mention_catch_exec):
>         New functions.
>         (catch_exec_breakpoint_ops): New static global.
>         (catch_exec_command_1): Use create_catchpoint instead of
>         create_exec_event_catchpoint to create the exec catchpoint.
> 
> gdb/testsuite/
> 2008-10-27  Joel Brobecker  <brobecker@adacore.com>
> 
>         gdb.base/foll-exec.exp: Update the expected output of a couple
>         of "info breakpoints" tests.
> 
> Tested on x86-linux. No regression.
> I'll commit in a couple of days unless there are some corrections
> to be made.

Now checked in as attached (there was one adjustment that I had to make
because of a collision with a patch from Tom). Rebuilt and retested
on x86-linux, no regression.

-- 
Joel

[-- Attachment #2: catch-exec.diff --]
[-- Type: text/plain, Size: 13283 bytes --]

Index: breakpoint.h
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.h,v
retrieving revision 1.83
diff -u -p -r1.83 breakpoint.h
--- breakpoint.h	17 Oct 2008 19:43:46 -0000	1.83
+++ breakpoint.h	30 Oct 2008 05:14:20 -0000
@@ -119,14 +119,6 @@ enum bptype
     /* These breakpoints are used to implement the "catch unload" command
        on platforms whose dynamic linkers support such functionality.  */
     bp_catch_unload,
-
-    /* These are not really breakpoints, but are catchpoints that
-       implement the "catch fork", "catch vfork" and "catch exec" commands
-       on platforms whose kernel support such functionality.  (I.e.,
-       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_exec,
   };
 
 /* States of enablement of breakpoint. */
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.357
diff -u -p -r1.357 breakpoint.c
--- breakpoint.c	28 Oct 2008 17:19:56 -0000	1.357
+++ breakpoint.c	30 Oct 2008 05:14:24 -0000
@@ -790,19 +790,10 @@ insert_catchpoint (struct ui_out *uo, vo
   struct breakpoint *b = (struct breakpoint *) args;
   int val = -1;
 
-  switch (b->type)
-    {
-    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));
-      break;
-    default:
-      internal_error (__FILE__, __LINE__, _("unknown breakpoint type"));
-      break;
-    }
+  gdb_assert (b->type == bp_catchpoint);
+  gdb_assert (b->ops != NULL && b->ops->insert != NULL);
+
+  b->ops->insert (b);
 }
 
 static int
@@ -1237,8 +1228,7 @@ Note: automatically using hardware break
       bpt->inserted = (val != -1);
     }
 
-  else if (bpt->owner->type == bp_catchpoint
-	   || bpt->owner->type == bp_catch_exec)
+  else if (bpt->owner->type == bp_catchpoint)
     {
       struct gdb_exception e = catch_exception (uiout, insert_catchpoint,
 						bpt->owner, RETURN_MASK_ERROR);
@@ -1502,13 +1492,6 @@ 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_catch_exec)
-      {
-	continue;
-      }
-
     /* bp_finish is a special case.  The only way we ought to be able
        to see one of these when an exec() has happened, is if the user
        caught a vfork, and then said "finish".  Ordinarily a finish just
@@ -1690,24 +1673,6 @@ remove_breakpoint (struct bp_location *b
 	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_exec:
-	  val = target_remove_exec_catchpoint (PIDGET (inferior_ptid));
-	  break;
-	default:
-	  warning (_("Internal error, %s line %d."), __FILE__, __LINE__);
-	  break;
-	}
-      if (val)
-	return val;
-      b->inserted = (is == mark_inserted);
-    }
 
   return 0;
 }
@@ -1966,8 +1931,7 @@ ep_is_catchpoint (struct breakpoint *ep)
 {
   return (ep->type == bp_catchpoint)
     || (ep->type == bp_catch_load)
-    || (ep->type == bp_catch_unload)
-    || (ep->type == bp_catch_exec);
+    || (ep->type == bp_catch_unload);
 
   /* ??rehrauer: Add more kinds here, as are implemented... */
 }
@@ -2365,14 +2329,6 @@ print_it_typical (bpstat bs)
       return PRINT_SRC_AND_LOC;
       break;
 
-    case bp_catch_exec:
-      annotate_catchpoint (b->number);
-      printf_filtered (_("\nCatchpoint %d (exec'd %s), "),
-		       b->number,
-		       b->exec_pathname);
-      return PRINT_SRC_AND_LOC;
-      break;
-
     case bp_watchpoint:
     case bp_hardware_watchpoint:
       annotate_watchpoint (b->number);
@@ -2791,8 +2747,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_catchpoint
-      && b->type != bp_catch_exec)	/* a non-watchpoint bp */
+      && b->type != bp_catchpoint)	/* a non-watchpoint bp */
     {
       if (bl->address != bp_addr) 	/* address doesn't match */
 	return 0;
@@ -2859,10 +2814,6 @@ bpstat_check_location (const struct bp_l
         return 0;
     }
      
-  if ((b->type == bp_catch_exec)
-      && !inferior_has_execd (inferior_ptid, &b->exec_pathname))
-    return 0;
-
   return 1;
 }
 
@@ -3388,7 +3339,6 @@ bpstat_what (bpstat bs)
 	    bs_class = no_effect;
 	  break;
 	case bp_catchpoint:
-	case bp_catch_exec:
 	  if (bs->stop)
 	    {
 	      if (bs->print)
@@ -3568,8 +3518,7 @@ print_one_breakpoint_location (struct br
     {bp_overlay_event, "overlay events"},
     {bp_catchpoint, "catchpoint"},
     {bp_catch_load, "catch load"},
-    {bp_catch_unload, "catch unload"},
-    {bp_catch_exec, "catch exec"}
+    {bp_catch_unload, "catch unload"}
   };
   
   static char bpenables[] = "nynny";
@@ -3706,21 +3655,6 @@ print_one_breakpoint_location (struct br
 	  }
 	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
-	   is relatively readable).  */
-	if (opts.addressprint)
-	  ui_out_field_skip (uiout, "addr");
-	annotate_field (5);
-	if (b->exec_pathname != NULL)
-	  {
-	    ui_out_text (uiout, "program \"");
-	    ui_out_field_string (uiout, "what", b->exec_pathname);
-	    ui_out_text (uiout, "\" ");
-	  }
-	break;
-
       case bp_breakpoint:
       case bp_hardware_breakpoint:
       case bp_until:
@@ -3920,7 +3854,6 @@ user_settable_breakpoint (const struct b
 	  || b->type == bp_catchpoint
 	  || b->type == bp_catch_load
 	  || b->type == bp_catch_unload
-	  || b->type == bp_catch_exec
 	  || b->type == bp_hardware_breakpoint
 	  || b->type == bp_watchpoint
 	  || b->type == bp_read_watchpoint
@@ -4130,8 +4063,7 @@ set_default_breakpoint (int valid, CORE_
       bp_hardware_watchpoint
       bp_read_watchpoint
       bp_access_watchpoint
-      bp_catchpoint
-      bp_catch_exec */
+      bp_catchpoint */
 
 static int
 breakpoint_address_is_meaningful (struct breakpoint *bpt)
@@ -4142,8 +4074,7 @@ breakpoint_address_is_meaningful (struct
 	  && type != bp_hardware_watchpoint
 	  && type != bp_read_watchpoint
 	  && type != bp_access_watchpoint
-	  && type != bp_catchpoint
-	  && type != bp_catch_exec);
+	  && type != bp_catchpoint);
 }
 
 /* Rescan breakpoints at the same address and section as BPT,
@@ -4258,8 +4189,7 @@ adjust_breakpoint_address (CORE_ADDR bpa
            || bptype == bp_hardware_watchpoint
            || bptype == bp_read_watchpoint
            || bptype == bp_access_watchpoint
-           || bptype == bp_catchpoint
-           || bptype == bp_catch_exec)
+           || bptype == bp_catchpoint)
     {
       /* Watchpoints and the various bp_catch_* eventpoints should not
          have their addresses modified.  */
@@ -4326,7 +4256,6 @@ allocate_bp_location (struct breakpoint 
       break;
     case bp_watchpoint:
     case bp_catchpoint:
-    case bp_catch_exec:
       loc->loc_type = bp_loc_other;
       break;
     default:
@@ -4939,32 +4868,73 @@ create_fork_vfork_event_catchpoint (int 
   b->forked_inferior_pid = null_ptid;
 }
 
+/* Exec catchpoints.  */
+
 static void
-create_exec_event_catchpoint (int tempflag, char *cond_string)
+insert_catch_exec (struct breakpoint *b)
 {
-  struct symtab_and_line sal;
-  struct breakpoint *b;
-  int thread = -1;		/* All threads. */
+  target_insert_exec_catchpoint (PIDGET (inferior_ptid));
+}
 
-  init_sal (&sal);
-  sal.pc = 0;
-  sal.symtab = NULL;
-  sal.line = 0;
+static int
+remove_catch_exec (struct breakpoint *b)
+{
+  return target_remove_exec_catchpoint (PIDGET (inferior_ptid));
+}
 
-  b = set_raw_breakpoint (sal, bp_catch_exec);
-  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->addr_string = NULL;
-  b->enable_state = bp_enabled;
-  b->disposition = tempflag ? disp_del : disp_donttouch;
-  update_global_location_list (1);
+static int
+breakpoint_hit_catch_exec (struct breakpoint *b)
+{
+  return inferior_has_execd (inferior_ptid, &b->exec_pathname);
+}
 
-  mention (b);
+static enum print_stop_action
+print_it_catch_exec (struct breakpoint *b)
+{
+  annotate_catchpoint (b->number);
+  printf_filtered (_("\nCatchpoint %d (exec'd %s), "), b->number,
+		   b->exec_pathname);
+  return PRINT_SRC_AND_LOC;
 }
 
+static void
+print_one_catch_exec (struct breakpoint *b, CORE_ADDR *last_addr)
+{
+  struct value_print_options opts;
+
+  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, "exec");
+  if (b->exec_pathname != NULL)
+    {
+      ui_out_text (uiout, ", program \"");
+      ui_out_field_string (uiout, "what", b->exec_pathname);
+      ui_out_text (uiout, "\" ");
+    }
+}
+
+static void
+print_mention_catch_exec (struct breakpoint *b)
+{
+  printf_filtered (_("Catchpoint %d (exec)"), b->number);
+}
+
+static struct breakpoint_ops catch_exec_breakpoint_ops =
+{
+  insert_catch_exec,
+  remove_catch_exec,
+  breakpoint_hit_catch_exec,
+  print_it_catch_exec,
+  print_one_catch_exec,
+  print_mention_catch_exec
+};
+
 static int
 hw_breakpoint_used_count (void)
 {
@@ -5175,10 +5145,6 @@ mention (struct breakpoint *b)
 			 (b->dll_pathname != NULL) ? 
 			 b->dll_pathname : "<any library>");
 	break;
-      case bp_catch_exec:
-	printf_filtered (_("Catchpoint %d (exec)"),
-			 b->number);
-	break;
 
       case bp_until:
       case bp_finish:
@@ -6633,7 +6599,7 @@ catch_exec_command_1 (char *arg, int fro
 
   /* If this target supports it, create an exec catchpoint
      and enable reporting of such events. */
-  create_exec_event_catchpoint (tempflag, cond_string);
+  create_catchpoint (tempflag, cond_string, &catch_exec_breakpoint_ops);
 }
 
 static void
@@ -7772,7 +7738,6 @@ breakpoint_re_set_one (void *bint)
          that requests them is unaffected by e.g., new libraries being
          loaded. */
     case bp_catchpoint:
-    case bp_catch_exec:
       break;
 
     default:
@@ -8041,7 +8006,6 @@ disable_command (char *args, int from_tt
       case bp_catchpoint:
       case bp_catch_load:
       case bp_catch_unload:
-      case bp_catch_exec:
       case bp_hardware_breakpoint:
       case bp_watchpoint:
       case bp_hardware_watchpoint:
@@ -8174,7 +8138,6 @@ enable_command (char *args, int from_tty
       case bp_catchpoint:
       case bp_catch_load:
       case bp_catch_unload:
-      case bp_catch_exec:
       case bp_hardware_breakpoint:
       case bp_watchpoint:
       case bp_hardware_watchpoint:
Index: testsuite/gdb.base/foll-exec.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/foll-exec.exp,v
retrieving revision 1.8
diff -u -p -r1.8 foll-exec.exp
--- testsuite/gdb.base/foll-exec.exp	15 Apr 2008 14:33:54 -0000	1.8
+++ testsuite/gdb.base/foll-exec.exp	30 Oct 2008 05:14:24 -0000
@@ -217,14 +217,11 @@ proc do_exec_tests {} {
    # Verify that the catchpoint is mentioned in an "info breakpoints",
    # and further that the catchpoint mentions no program name.
    #
-   send_gdb "info breakpoints\n"
-   gdb_expect {
-     -re ".*catch exec.*keep y.*$gdb_prompt $"\
-                     {pass "info shows catchpoint without exec pathname"}
-     -re ".*catch exec.*program \"\".*$gdb_prompt $"\
-                     {fail "info shows catchpoint without exec pathname"}
-     -re "$gdb_prompt $" {fail "info shows catchpoint without exec pathname"}
-     timeout         {fail "(timeout) info shows catchpoint without exec pathname"}
+   set msg "info shows catchpoint without exec pathname"
+   gdb_test_multiple "info breakpoints" $msg {
+       -re ".*catchpoint.*keep y.*exec\[\n\r\]+$gdb_prompt $" {
+           pass $msg
+       }
    }
 
    # DTS CLLbs16760
@@ -248,12 +245,11 @@ proc do_exec_tests {} {
    # and further that the catchpoint managed to capture the exec'd
    # program's name.
    #
-   send_gdb "info breakpoints\n"
-   gdb_expect {
-     -re ".*catch exec .*program \".*${testfile2}\".*$gdb_prompt $"\
-                     {pass "info shows catchpoint exec pathname"}
-     -re "$gdb_prompt $" {fail "info shows catchpoint exec pathname"}
-     timeout         {fail "(timeout) info shows catchpoint exec pathname"}
+   set msg "info shows catchpoint exec pathname"
+   gdb_test_multiple "info breakpoints" $msg {
+       -re ".*catchpoint.*keep y.*exec, program \".*${testfile2}\".*$gdb_prompt $" {
+           pass $msg
+       }
    }
 
    # Verify that we can continue from the catchpoint, and land in the