[WIP] pending breakpoint support

["J. Johnston" <jjohnstn at redhat dot com>]

I have been hacking around with supporting "future-break" functionality (note, I 
said "hacking").  I have seen past discussion regarding the issue and the 
requirement to add functionality to the current breakpoint command as opposed to 
creating a separate command.

For my change, I wrappered the call to parse_breakpoint_sals() to catch any 
error.  If the call fails, then I simply give the user the option as marking it 
as pending.  I then make a fake breakpoint with a special enable_state of 
bp_shlib_pending.  I also save the original breakpoint command as the 
addr_string plus some needed state to recreate the command at a later time.  I 
attempted to have any original condition parsed but it isn't used in my current 
design as I end up reparsing the original command from scratch anyway.

When we are loading shared libraries, the function 
re_enable_breakpoints_in_shlibs() gets called.  I have added code in there to 
attempt to reparse any pending break command.  If successful, a new breakpoint 
is created by basically reissing the command with saved state accounted for. 
After creating the new breakpoint(s), I delete the pending place holder breakpoint.

Ideally, I would liked to have reused the same breakpoint structure that was 
initially allocated but in my code there still was the possibility of one 
command generating multiple breakpoints so I took the aforementioned strategy. 
I have been informed that the newer breakpoint model will eliminate that problem 
and I should be able to reuse the breakpoint number, etc...

A problem I didn't solve yet has to do with the issuing of error messages for 
pending and disabled shared-library breakpoints when the reenablement is 
attempted and it fails.  This can be very annoying when you have a large number 
of shared libraries loaded each time (e.g. Eclipse).

I have included a gdb session below along with the patch I used so folks can see 
how it currently works.

What I would like to do is get some discussion going:

  1. Is the user interface on the right track?
  2. What gotchas do I need to think about?
  3. Any design recommendations for implementing this better?

-- Jeff J.

/../gdb -nw a.out
GNU gdb 2003-11-07-cvs
Copyright 2003 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "ia64-unknown-linux-gnu"...Using host libthread_db 
library "/lib/tls/libthread_db.so.1".

(gdb) b printf
Function "printf" not defined.
Make breakpoint pending? (y or n) y
Breakpoint 1 (printf) pending.
(gdb) info break
Num Type           Disp Enb Address            What
1   breakpoint     keep n   <PENDING>          printf
(gdb) b main
Breakpoint 2 at 0x4000000000000702: file hello.c, line 9.
(gdb) info break
Num Type           Disp Enb Address            What
1   breakpoint     keep n   <PENDING>          printf
2   breakpoint     keep y   0x4000000000000702 in main at hello.c:9
(gdb) run
Starting program: 
/to/scratch/jjohnstn/gdb-patches/future-break/build/ia64/gdb/testsuite/gdb.base/a.out 

Pending breakpoint <printf> resolved
Breakpoint 3 at 0x20000000000ec5e0

Breakpoint 2, main () at hello.c:9
9	   x = printf ("hello world\n");
(gdb) info break
Num Type           Disp Enb Address            What
2   breakpoint     keep y   0x4000000000000702 in main at hello.c:9
	breakpoint already hit 1 time
3   breakpoint     keep y   0x20000000000ec5e0 <printf+48>
(gdb) c
Continuing.

Breakpoint 3, 0x20000000000ec5e0 in printf () from /lib/tls/libc.so.6.1
(gdb)

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.142
diff -u -p -r1.142 breakpoint.c
--- breakpoint.c	6 Nov 2003 18:35:05 -0000	1.142
+++ breakpoint.c	19 Nov 2003 00:51:08 -0000
@@ -119,6 +119,8 @@ static void condition_command (char *, i
 
 static int get_number_trailer (char **, int);
 
+static int do_captured_parse_breakpoint (void *);
+
 void set_breakpoint_count (int);
 
 typedef enum
@@ -2572,7 +2574,8 @@ bpstat_stop_status (CORE_ADDR *pc, int n
   {
     if (b->enable_state == bp_disabled
 	|| b->enable_state == bp_shlib_disabled
-	|| b->enable_state == bp_call_disabled)
+	|| b->enable_state == bp_call_disabled
+	|| b->enable_state == bp_shlib_pending)
       continue;
 
     if (b->type != bp_watchpoint
@@ -3300,7 +3303,7 @@ print_one_breakpoint (struct breakpoint 
   
   static char *bpdisps[] =
   {"del", "dstp", "dis", "keep"};
-  static char bpenables[] = "nynny";
+  static char bpenables[] = "nynnyn";
   char wrap_indent[80];
   struct ui_stream *stb = ui_out_stream_new (uiout);
   struct cleanup *old_chain = make_cleanup_ui_out_stream_delete (stb);
@@ -3454,7 +3457,15 @@ print_one_breakpoint (struct breakpoint 
 	if (addressprint)
 	  {
 	    annotate_field (4);
-	    ui_out_field_core_addr (uiout, "addr", b->loc->address);
+	    if (b->enable_state == bp_shlib_pending)
+	      {
+		if (TARGET_ADDR_BIT <= 32)
+		  ui_out_field_string (uiout, "addr", "<PENDING>  ");
+		else
+		  ui_out_field_string (uiout, "addr", "<PENDING>        ");
+	      }
+	    else
+	      ui_out_field_core_addr (uiout, "addr", b->loc->address);
 	  }
 	annotate_field (5);
 	*last_addr = b->loc->address;
@@ -3473,6 +3484,10 @@ print_one_breakpoint (struct breakpoint 
 	    ui_out_text (uiout, ":");
 	    ui_out_field_int (uiout, "line", b->line_number);
 	  }
+	else if (b->enable_state == bp_shlib_pending)
+	  {
+	    ui_out_field_string (uiout, "pending", b->addr_string);
+	  }
 	else
 	  {
 	    print_address_symbolic (b->loc->address, stb->stream, demangle, "");
@@ -3740,14 +3755,14 @@ describe_other_breakpoints (CORE_ADDR pc
 
   ALL_BREAKPOINTS (b)
     if (b->loc->address == pc)	/* address match / overlay match */
-      if (!overlay_debugging || b->loc->section == section)
+      if (b->enable_state != bp_shlib_pending && (!overlay_debugging || b->loc->section == section))
 	others++;
   if (others > 0)
     {
       printf_filtered ("Note: breakpoint%s ", (others > 1) ? "s" : "");
       ALL_BREAKPOINTS (b)
 	if (b->loc->address == pc)	/* address match / overlay match */
-	  if (!overlay_debugging || b->loc->section == section)
+	  if (b->enable_state != bp_shlib_pending && (!overlay_debugging || b->loc->section == section))
 	    {
 	      others--;
 	      printf_filtered ("%d%s%s ",
@@ -3836,6 +3851,7 @@ check_duplicates (struct breakpoint *bpt
   ALL_BP_LOCATIONS (b)
     if (b->owner->enable_state != bp_disabled
 	&& b->owner->enable_state != bp_shlib_disabled
+	&& b->owner->enable_state != bp_shlib_pending
 	&& b->owner->enable_state != bp_call_disabled
 	&& b->address == address	/* address / overlay match */
 	&& (!overlay_debugging || b->section == section)
@@ -3870,6 +3886,7 @@ check_duplicates (struct breakpoint *bpt
 	  {
 	    if (b->owner->enable_state != bp_disabled
 		&& b->owner->enable_state != bp_shlib_disabled
+		&& b->owner->enable_state != bp_shlib_pending
 		&& b->owner->enable_state != bp_call_disabled
 		&& b->address == address	/* address / overlay match */
 		&& (!overlay_debugging || b->section == section)
@@ -4284,22 +4301,84 @@ disable_breakpoints_in_shlibs (int silen
   }
 }
 
+struct captured_parse_breakpoint_args
+  {
+    char **arg_p;
+    struct symtabs_and_lines *sals_p;
+    char ***addr_string_p;
+  };
+
 /* Try to reenable any breakpoints in shared libraries.  */
 void
 re_enable_breakpoints_in_shlibs (void)
 {
   struct breakpoint *b;
+  struct breakpoint *del_b = NULL;
 
   ALL_BREAKPOINTS (b)
+  {
+    if (del_b)
+      {
+	delete_breakpoint (del_b);
+	del_b = NULL;
+	if (b == NULL)
+	  break;
+      }
     if (b->enable_state == bp_shlib_disabled)
-    {
-      char buf[1];
+      {
+	char buf[1];
+	
+	/* Do not reenable the breakpoint if the shared library
+	   is still not mapped in.  */
+	if (target_read_memory (b->loc->address, buf, 1) == 0)
+	  b->enable_state = bp_enabled;
+      }
+    else if (b->enable_state == bp_shlib_pending)
+      {
+	/* Try and reparse the breakpoint in case the shared library
+	   is now loaded.  */
+	struct symtabs_and_lines sals;
+	struct symtab_and_line pending_sal;
+	/* Pointers in arg to the start, and one past the end, of the
+	   condition.  */
+	char **cond_string = (char **) NULL;
+	char *copy_arg = b->addr_string;
+	char **addr_string;
+	struct captured_parse_breakpoint_args parse_args;
+	int rc;
+	
+	sals.sals = NULL;
+	sals.nelts = 0;
+	addr_string = NULL;
+	
+	parse_args.arg_p = &copy_arg;
+	parse_args.sals_p = &sals;
+	parse_args.addr_string_p = &addr_string;
+	
+	rc = catch_errors (do_captured_parse_breakpoint, &parse_args,
+			   NULL, RETURN_MASK_ALL);
 
-      /* Do not reenable the breakpoint if the shared library
-         is still not mapped in.  */
-      if (target_read_memory (b->loc->address, buf, 1) == 0)
-	b->enable_state = bp_enabled;
-    }
+	if (rc == GDB_RC_OK)
+	  {
+	    enum language old_language = current_language->la_language;
+	    int old_input_radix = input_radix;
+
+	    printf_filtered ("Pending breakpoint <%s> resolved\n", b->addr_string);
+
+	    /* Set language, input-radix, then reissue breakpoint command.  Following the
+	       command, restore the language and input-radix.  */
+	    set_language (b->language);
+	    input_radix = b->input_radix;
+	    break_command_1 (b->addr_string, b->flag, b->from_tty);
+	    set_language (old_language);
+	    input_radix = old_input_radix;
+	    del_b = b; /* Delete on next pass.  */
+	  }
+      }
+  }
+
+  if (del_b)
+    delete_breakpoint (del_b);
 }
 
 #endif
@@ -4712,14 +4791,21 @@ mention (struct breakpoint *b)
 
   if (say_where)
     {
-      if (addressprint || b->source_file == NULL)
+      if (b->enable_state == bp_shlib_pending)
 	{
-	  printf_filtered (" at ");
-	  print_address_numeric (b->loc->address, 1, gdb_stdout);
+	  printf_filtered (" (%s) pending.", b->addr_string);
+	}
+      else
+	{
+	  if (addressprint || b->source_file == NULL)
+	    {
+	      printf_filtered (" at ");
+	      print_address_numeric (b->loc->address, 1, gdb_stdout);
+	    }
+	  if (b->source_file)
+	    printf_filtered (": file %s, line %d.",
+			     b->source_file, b->line_number);
 	}
-      if (b->source_file)
-	printf_filtered (": file %s, line %d.",
-			 b->source_file, b->line_number);
     }
   do_cleanups (old_chain);
   if (ui_out_is_mi_like_p (uiout))
@@ -4892,6 +4978,16 @@ breakpoint_sals_to_pc (struct symtabs_an
     }
 }
 
+static int
+do_captured_parse_breakpoint (void *data)
+{
+  struct captured_parse_breakpoint_args *args = data;
+  
+  parse_breakpoint_sals (args->arg_p, args->sals_p, args->addr_string_p);
+
+  return GDB_RC_OK;
+}
+
 /* Set a breakpoint according to ARG (function, linenum or *address)
    flag: first bit  : 0 non-temporary, 1 temporary.
    second bit : 0 normal breakpoint, 1 hardware breakpoint. */
@@ -4902,14 +4998,18 @@ break_command_1 (char *arg, int flag, in
   int tempflag, hardwareflag;
   struct symtabs_and_lines sals;
   struct expression **cond = 0;
+  struct symtab_and_line pending_sal;
   /* Pointers in arg to the start, and one past the end, of the
      condition.  */
   char **cond_string = (char **) NULL;
+  char *copy_arg;
   char *addr_start = arg;
   char **addr_string;
   struct cleanup *old_chain;
   struct cleanup *breakpoint_chain = NULL;
-  int i;
+  struct captured_parse_breakpoint_args parse_args;
+  int i, rc;
+  int pending = 0;
   int thread = -1;
   int ignore_count = 0;
 
@@ -4919,19 +5019,40 @@ break_command_1 (char *arg, int flag, in
   sals.sals = NULL;
   sals.nelts = 0;
   addr_string = NULL;
-  parse_breakpoint_sals (&arg, &sals, &addr_string);
 
-  if (!sals.nelts)
+  parse_args.arg_p = &arg;
+  parse_args.sals_p = &sals;
+  parse_args.addr_string_p = &addr_string;
+
+  rc = catch_errors (do_captured_parse_breakpoint, &parse_args,
+		     NULL, RETURN_MASK_ALL);
+
+  if (rc != GDB_RC_OK)
+    {
+      if (!query ("Make breakpoint pending? "))
+	return;
+      copy_arg = (char *)xmalloc (strlen (addr_start));
+      strcpy (copy_arg, addr_start);
+      addr_string = &copy_arg;
+      sals.nelts = 1;
+      sals.sals = &pending_sal;
+      pending_sal.pc = 0;
+      pending = 1;
+    }
+  else if (!sals.nelts)
     return;
 
   /* Create a chain of things that always need to be cleaned up. */
   old_chain = make_cleanup (null_cleanup, 0);
 
-  /* Make sure that all storage allocated to SALS gets freed.  */
-  make_cleanup (xfree, sals.sals);
-
-  /* Cleanup the addr_string array but not its contents. */
-  make_cleanup (xfree, addr_string);
+  if (!pending)
+    {
+      /* Make sure that all storage allocated to SALS gets freed.  */
+      make_cleanup (xfree, sals.sals);
+      
+      /* Cleanup the addr_string array but not its contents. */
+      make_cleanup (xfree, addr_string);
+    }
 
   /* Allocate space for all the cond expressions. */
   cond = xcalloc (sals.nelts, sizeof (struct expression *));
@@ -4958,7 +5079,8 @@ break_command_1 (char *arg, int flag, in
 
   /* Resolve all line numbers to PC's and verify that the addresses
      are ok for the target.  */
-  breakpoint_sals_to_pc (&sals, addr_start);
+  if (!pending)
+    breakpoint_sals_to_pc (&sals, addr_start);
 
   /* Verify that condition can be parsed, before setting any
      breakpoints.  Allocate a separate condition expression for each
@@ -5009,11 +5131,34 @@ break_command_1 (char *arg, int flag, in
 	}
     }
 
-  create_breakpoints (sals, addr_string, cond, cond_string,
-		      hardwareflag ? bp_hardware_breakpoint : bp_breakpoint,
-		      tempflag ? disp_del : disp_donttouch,
-		      thread, ignore_count, from_tty);
+  if (pending)
+    {
+      struct symtab_and_line sal;
+      struct breakpoint *b;
+
+      sal.symtab = NULL;
+      sal.pc = 0;
 
+      b = set_raw_breakpoint (sal, hardwareflag ? bp_hardware_breakpoint : bp_breakpoint);
+      set_breakpoint_count (breakpoint_count + 1);
+      b->number = breakpoint_count;
+      b->cond = *cond;
+      b->thread = thread;
+      b->addr_string = *addr_string;
+      b->cond_string = *cond_string;
+      b->ignore_count = ignore_count;
+      b->enable_state = bp_shlib_pending;
+      b->disposition = tempflag ? disp_del : disp_donttouch;
+      b->from_tty = from_tty;
+      b->flag = flag;
+      mention (b);
+    }
+  else
+    create_breakpoints (sals, addr_string, cond, cond_string,
+			hardwareflag ? bp_hardware_breakpoint : bp_breakpoint,
+			tempflag ? disp_del : disp_donttouch,
+			thread, ignore_count, from_tty);
+  
   if (sals.nelts > 1)
     {
       warning ("Multiple breakpoints were set.");
@@ -6759,6 +6904,7 @@ delete_breakpoint (struct breakpoint *bp
 	    && !b->loc->duplicate
 	    && b->enable_state != bp_disabled
 	    && b->enable_state != bp_shlib_disabled
+	    && b->enable_state != bp_shlib_pending
 	    && b->enable_state != bp_call_disabled)
 	{
 	  int val;
@@ -6964,8 +7110,12 @@ breakpoint_re_set_one (void *bint)
          shlib_disabled breakpoint though.  There's a fair chance we
          can't re-set it if the shared library it's in hasn't been
          loaded yet.  */
+
+      if (b->enable_state == bp_shlib_pending)
+	break;
+
       save_enable = b->enable_state;
-      if (b->enable_state != bp_shlib_disabled)
+      if (b->enable_state != bp_shlib_disabled) 
         b->enable_state = bp_disabled;
 
       set_language (b->language);
Index: breakpoint.h
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.h,v
retrieving revision 1.26
diff -u -p -r1.26 breakpoint.h
--- breakpoint.h	6 Nov 2003 18:24:55 -0000	1.26
+++ breakpoint.h	19 Nov 2003 00:51:08 -0000
@@ -158,11 +158,14 @@ enum enable_state
 			   automatically enabled and reset when the call 
 			   "lands" (either completes, or stops at another 
 			   eventpoint). */
-    bp_permanent	/* There is a breakpoint instruction hard-wired into
+    bp_permanent,	/* There is a breakpoint instruction hard-wired into
 			   the target's code.  Don't try to write another
 			   breakpoint instruction on top of it, or restore
 			   its value.  Step over it using the architecture's
 			   SKIP_INSN macro.  */
+    bp_shlib_pending,   /* The eventpoint could not be set as an shlib has
+			   not yet been loaded the first time.  When the
+			   shlib is loaded, it will be reissued.  */
   };
 
 
@@ -385,6 +388,12 @@ struct breakpoint
 
     /* Methods associated with this breakpoint.  */
     struct breakpoint_ops *ops;
+
+    /* Initial from_tty value.  */
+    int from_tty;
+
+    /* Initial flag value.  */
+    int flag;
   };
 
 /* The following stuff is an abstract data type "bpstat" ("breakpoint