Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA]: pending breakpoint support  [1/3]
@ 2004-01-21 20:52 Jeff Johnston
  2004-01-22 22:27 ` Daniel Jacobowitz
  0 siblings, 1 reply; 17+ messages in thread
From: Jeff Johnston @ 2004-01-21 20:52 UTC (permalink / raw)
  To: gdb-patches

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

This is the first of the last 3 patches for pending breakpoint support.
This patch adds the actual support.  The other two patches are the
documentation (previously approved, but before code was accepted) and
changes to the testsuite.

This patch accounts for previous comments on the breakpoint code.

Ok to commit?

-- Jeff J.

2004-01-21  Jeff Johnston  <jjohnstn@redhat.com>

        * breakpoint.h (struct breakpoint): Add new flag, from_tty,
        and pending fields for pending breakpoint support.
        * breakpoint.c (breakpoint_enabled): Add check for not pending.
        (condition_command): Only parse condition if not a pending
        breakpoint.
        (print_one_breakpoint): Add support for pending breakpoints.
        (describe_other_breakpoints): Add checks to verify we are not
        dealing with pending breakpoints.
        (check_duplicates): Don't check pending breakpoints.
        (set_raw_breakpoint): Initialize pending flag.
        (do_restore_lang_radix_cleanup): New cleanup routine.
        (resolve_pending_breakpoint): New function.
        (re_enable_breakpoints_in_shlibs): Try and resolve any
        pending breakpoints via resolve_pending_breakpoint.
        (mention): Add pending breakpoint support.
        (parse_breakpoint_sals): Add new parameter to pass to
        decode_line_1 to indicate silent errors when files or functions
        are not found.  Change all callers.
        (do_captured_parse_breakpoint): New function.
        (break_command_1): Catch errors when parsing the breakpoint.
        If a "not found" error occurs, offer to make the breakpoint
        pending on a future shared library load.  Do not parse any
        conditional if breakpoint is pending.
        (delete_breakpoint): Add appropriate check for pending.
        (breakpoint_re_set_one): Ditto.
        (do_enable_breakpoint): Ditto.


[-- Attachment #2: pbreak.patch1 --]
[-- Type: text/plain, Size: 23745 bytes --]

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	21 Jan 2004 02:34:10 -0000
@@ -385,6 +385,15 @@ struct breakpoint
 
     /* Methods associated with this breakpoint.  */
     struct breakpoint_ops *ops;
+
+    /* Initial from_tty value.  */
+    int from_tty;
+
+    /* Initial flag value.  */
+    int flag;
+
+    /* Is breakpoint pending on shlib loads?  */
+    int pending;
   };
 \f
 /* The following stuff is an abstract data type "bpstat" ("breakpoint
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.149
diff -u -p -r1.149 breakpoint.c
--- breakpoint.c	17 Jan 2004 21:56:12 -0000	1.149
+++ breakpoint.c	21 Jan 2004 02:34:10 -0000
@@ -119,6 +119,8 @@ static void condition_command (char *, i
 
 static int get_number_trailer (char **, int);
 
+static int do_captured_parse_breakpoint (struct ui_out *, void *);
+
 void set_breakpoint_count (int);
 
 typedef enum
@@ -326,7 +328,7 @@ int exception_support_initialized = 0;
 static int
 breakpoint_enabled (struct breakpoint *b)
 {
-  return b->enable_state == bp_enabled;
+  return (b->enable_state == bp_enabled && !b->pending);
 }
 
 /* Set breakpoint count to NUM.  */
@@ -560,9 +562,12 @@ condition_command (char *arg, int from_t
 	  /* I don't know if it matters whether this is the string the user
 	     typed in or the decompiled expression.  */
 	  b->cond_string = savestring (arg, strlen (arg));
-	  b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
-	  if (*arg)
-	    error ("Junk at end of expression");
+	  if (!b->pending)
+	    {
+	      b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
+	      if (*arg)
+		error ("Junk at end of expression");
+	    }
 	}
       breakpoints_changed ();
       breakpoint_modify_event (b->number);
@@ -3458,7 +3463,15 @@ print_one_breakpoint (struct breakpoint 
 	if (addressprint)
 	  {
 	    annotate_field (4);
-	    ui_out_field_core_addr (uiout, "addr", b->loc->address);
+	    if (b->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;
@@ -3477,6 +3490,10 @@ print_one_breakpoint (struct breakpoint 
 	    ui_out_text (uiout, ":");
 	    ui_out_field_int (uiout, "line", b->line_number);
 	  }
+	else if (b->pending)
+	  {
+	    ui_out_field_string (uiout, "pending", b->addr_string);
+	  }
 	else
 	  {
 	    print_address_symbolic (b->loc->address, stb->stream, demangle, "");
@@ -3513,7 +3530,15 @@ print_one_breakpoint (struct breakpoint 
       ui_out_field_stream (uiout, "cond", stb);
       ui_out_text (uiout, "\n");
     }
-  
+
+  if (b->pending && b->cond_string)
+    {
+      annotate_field (7);
+      ui_out_text (uiout, "\tstop only if ");
+      ui_out_field_string (uiout, "cond", b->cond_string);
+      ui_out_text (uiout, "\n");
+    }
+
   if (b->thread != -1)
     {
       /* FIXME should make an annotation for this */
@@ -3744,14 +3769,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->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->pending && (!overlay_debugging || b->loc->section == section))
 	    {
 	      others--;
 	      printf_filtered ("%d%s%s ",
@@ -3840,6 +3865,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->pending
 	&& b->owner->enable_state != bp_call_disabled
 	&& b->address == address	/* address / overlay match */
 	&& (!overlay_debugging || b->section == section)
@@ -3874,6 +3900,7 @@ check_duplicates (struct breakpoint *bpt
 	  {
 	    if (b->owner->enable_state != bp_disabled
 		&& b->owner->enable_state != bp_shlib_disabled
+		&& !b->owner->pending
 		&& b->owner->enable_state != bp_call_disabled
 		&& b->address == address	/* address / overlay match */
 		&& (!overlay_debugging || b->section == section)
@@ -4050,6 +4077,7 @@ set_raw_breakpoint (struct symtab_and_li
   b->forked_inferior_pid = 0;
   b->exec_pathname = NULL;
   b->ops = NULL;
+  b->pending = 0;
 
   /* Add this breakpoint to the end of the chain
      so that a list of breakpoints will come out in order
@@ -4288,23 +4316,139 @@ disable_breakpoints_in_shlibs (int silen
   }
 }
 
+struct captured_parse_breakpoint_args
+  {
+    char **arg_p;
+    struct symtabs_and_lines *sals_p;
+    char ***addr_string_p;
+    int *not_found_ptr;
+  };
+
+struct lang_and_radix
+  {
+    enum language lang;
+    int radix;
+  };
+
+/* Cleanup helper routine to restore the current language and
+   input radix.  */
+static void
+do_restore_lang_radix_cleanup (void *old)
+{
+  struct lang_and_radix *p = old;
+  set_language (p->lang);
+  input_radix = p->radix;
+}
+
+/* Try and resolve a pending breakpoint.  */
+static struct breakpoint *
+resolve_pending_breakpoint (struct breakpoint *b)
+{
+  /* 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;
+  char *errmsg;
+  struct captured_parse_breakpoint_args parse_args;
+  int rc;
+  int not_found = 0;
+  struct ui_file *old_gdb_stderr;
+  
+  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;
+  parse_args.not_found_ptr = &not_found;
+  
+  rc = catch_exceptions (uiout, do_captured_parse_breakpoint, 
+		         &parse_args, NULL, RETURN_MASK_ALL);
+  
+  if (rc == GDB_RC_OK)
+    {
+      struct lang_and_radix old_lr;
+      struct cleanup *old_chain;
+      char *arg;
+      struct breakpoint *b1;
+      
+      printf_filtered ("Pending breakpoint \"%s\" resolved\n", b->addr_string);
+      
+      /* Set language, input-radix, then reissue breakpoint command. 
+	 Ensure the language and input-radix are restored afterwards.  */
+      old_lr.lang = current_language->la_language;
+      old_lr.radix = input_radix;
+      old_chain = make_cleanup (do_restore_lang_radix_cleanup, &old_lr);
+      
+      set_language (b->language);
+      input_radix = b->input_radix;
+      break_command_1 (b->addr_string, b->flag, b->from_tty);
+      b1 = breakpoint_chain;
+      while (b1->next)
+	b1 = b1->next;
+      /* If there is condition specified, it should be copied over.  */
+      if (b->cond_string)
+	{
+	  arg = b->cond_string;
+	  b1->cond_string = savestring (arg, strlen (arg));
+	  b1->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
+	  if (*arg)
+	    error ("Junk at end of expression");
+	}
+      /* If there are commands associated with the breakpoint, they should 
+         be copied too.  */
+      if (b->commands)
+	{
+	  b1->commands = copy_command_lines (b->commands);
+	}
+
+      do_cleanups (old_chain);
+      return b1; /* Pending breakpoint resolved.  */
+    }
+
+  /* Otherwise, we didn't successfully resolve pending breakpoint.  */
+  return NULL;
+}
+
 /* 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->enable_state == bp_shlib_disabled)
-    {
-      char buf[1], *lib;
+      {
+	char buf[1], *lib;
+	
+	/* Do not reenable the breakpoint if the shared library
+	   is still not mapped in.  */
+	lib = PC_SOLIB (b->loc->address);
+	if (lib != NULL && target_read_memory (b->loc->address, buf, 1) == 0)
+	  b->enable_state = bp_enabled;
+      }
+    else if (b->pending && (b->enable_state == bp_enabled))
+      {
+	if (resolve_pending_breakpoint (b) != NULL)
+	  del_b = b; /* Mark pending breakpoint for deletion.  */
+      }
+  }
 
-      /* Do not reenable the breakpoint if the shared library
-         is still not mapped in.  */
-      lib = PC_SOLIB (b->loc->address);
-      if (lib != NULL && target_read_memory (b->loc->address, buf, 1) == 0)
-	b->enable_state = bp_enabled;
-    }
+  if (del_b)
+    delete_breakpoint (del_b);
 }
 
 #endif
@@ -4716,14 +4860,21 @@ mention (struct breakpoint *b)
 
   if (say_where)
     {
-      if (addressprint || b->source_file == NULL)
+      if (b->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))
@@ -4799,7 +4950,8 @@ create_breakpoints (struct symtabs_and_l
 static void
 parse_breakpoint_sals (char **address,
 		       struct symtabs_and_lines *sals,
-		       char ***addr_string)
+		       char ***addr_string,
+		       int *not_found_ptr)
 {
   char *addr_start = *address;
   *addr_string = NULL;
@@ -4840,9 +4992,11 @@ parse_breakpoint_sals (char **address,
  	      || ((strchr ("+-", (*address)[0]) != NULL)
  		  && ((*address)[1] != '['))))
 	*sals = decode_line_1 (address, 1, default_breakpoint_symtab,
-			       default_breakpoint_line, addr_string, NULL);
+			       default_breakpoint_line, addr_string, 
+			       not_found_ptr);
       else
-	*sals = decode_line_1 (address, 1, (struct symtab *) NULL, 0, addr_string, NULL);
+	*sals = decode_line_1 (address, 1, (struct symtab *) NULL, 0,
+		               addr_string, not_found_ptr);
     }
   /* For any SAL that didn't have a canonical string, fill one in. */
   if (sals->nelts > 0 && *addr_string == NULL)
@@ -4896,6 +5050,17 @@ breakpoint_sals_to_pc (struct symtabs_an
     }
 }
 
+static int
+do_captured_parse_breakpoint (struct ui_out *ui, void *data)
+{
+  struct captured_parse_breakpoint_args *args = data;
+  
+  parse_breakpoint_sals (args->arg_p, args->sals_p, args->addr_string_p, 
+		         args->not_found_ptr);
+
+  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. */
@@ -4906,16 +5071,22 @@ 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 *err_msg;
   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;
+  int not_found = 0;
 
   hardwareflag = flag & BP_HARDWAREFLAG;
   tempflag = flag & BP_TEMPFLAG;
@@ -4923,19 +5094,51 @@ 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;
+  parse_args.not_found_ptr = &not_found;
+
+  rc = catch_exceptions_with_msg (uiout, do_captured_parse_breakpoint, 
+		  		  &parse_args, NULL, &err_msg, 
+				  RETURN_MASK_ALL);
+
+  if (rc != GDB_RC_OK)
+    {
+      /* Check for file or function not found.  */
+      if (not_found)
+	{
+	  error_output_message (NULL, err_msg);
+	  xfree (err_msg);
+	  if (!query ("Make breakpoint pending on future shared library load? "))
+	    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
+	return;
+    }
+  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 *));
@@ -4962,62 +5165,91 @@ 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
      breakpoint. */
   thread = -1;			/* No specific thread yet */
-  for (i = 0; i < sals.nelts; i++)
+  if (!pending)
     {
-      char *tok = arg;
-      while (tok && *tok)
+      for (i = 0; i < sals.nelts; i++)
 	{
-	  char *end_tok;
-	  int toklen;
-	  char *cond_start = NULL;
-	  char *cond_end = NULL;
-	  while (*tok == ' ' || *tok == '\t')
-	    tok++;
-
-	  end_tok = tok;
-
-	  while (*end_tok != ' ' && *end_tok != '\t' && *end_tok != '\000')
-	    end_tok++;
-
-	  toklen = end_tok - tok;
-
-	  if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
-	    {
-	      tok = cond_start = end_tok + 1;
-	      cond[i] = parse_exp_1 (&tok, block_for_pc (sals.sals[i].pc), 0);
-	      make_cleanup (xfree, cond[i]);
-	      cond_end = tok;
-	      cond_string[i] = savestring (cond_start, cond_end - cond_start);
-	      make_cleanup (xfree, cond_string[i]);
-	    }
-	  else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
+	  char *tok = arg;
+	  while (tok && *tok)
 	    {
-	      char *tmptok;
-
-	      tok = end_tok + 1;
-	      tmptok = tok;
-	      thread = strtol (tok, &tok, 0);
-	      if (tok == tmptok)
-		error ("Junk after thread keyword.");
-	      if (!valid_thread_id (thread))
-		error ("Unknown thread %d\n", thread);
+	      char *end_tok;
+	      int toklen;
+	      char *cond_start = NULL;
+	      char *cond_end = NULL;
+	      while (*tok == ' ' || *tok == '\t')
+		tok++;
+	      
+	      end_tok = tok;
+	      
+	      while (*end_tok != ' ' && *end_tok != '\t' && *end_tok != '\000')
+		end_tok++;
+	      
+	      toklen = end_tok - tok;
+	      
+	      if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
+		{
+		  tok = cond_start = end_tok + 1;
+		  cond[i] = parse_exp_1 (&tok, block_for_pc (sals.sals[i].pc), 
+				         0);
+		  make_cleanup (xfree, cond[i]);
+		  cond_end = tok;
+		  cond_string[i] = savestring (cond_start, 
+				               cond_end - cond_start);
+		  make_cleanup (xfree, cond_string[i]);
+		}
+	      else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
+		{
+		  char *tmptok;
+		  
+		  tok = end_tok + 1;
+		  tmptok = tok;
+		  thread = strtol (tok, &tok, 0);
+		  if (tok == tmptok)
+		    error ("Junk after thread keyword.");
+		  if (!valid_thread_id (thread))
+		    error ("Unknown thread %d\n", thread);
+		}
+	      else
+		error ("Junk at end of arguments.");
 	    }
-	  else
-	    error ("Junk at end of arguments.");
 	}
+      create_breakpoints (sals, addr_string, cond, cond_string,
+			  hardwareflag ? bp_hardware_breakpoint 
+			  : bp_breakpoint,
+			  tempflag ? disp_del : disp_donttouch,
+			  thread, ignore_count, from_tty);
     }
+  else
+    {
+      struct symtab_and_line sal;
+      struct breakpoint *b;
 
-  create_breakpoints (sals, addr_string, cond, cond_string,
-		      hardwareflag ? bp_hardware_breakpoint : bp_breakpoint,
-		      tempflag ? disp_del : disp_donttouch,
-		      thread, ignore_count, from_tty);
+      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->pending = 1;
+      b->disposition = tempflag ? disp_del : disp_donttouch;
+      b->from_tty = from_tty;
+      b->flag = flag;
+      mention (b);
+    }
+  
   if (sals.nelts > 1)
     {
       warning ("Multiple breakpoints were set.");
@@ -5064,7 +5296,7 @@ do_captured_breakpoint (void *data)
   sals.nelts = 0;
   address_end = args->address;
   addr_string = NULL;
-  parse_breakpoint_sals (&address_end, &sals, &addr_string);
+  parse_breakpoint_sals (&address_end, &sals, &addr_string, 0);
 
   if (!sals.nelts)
     return GDB_RC_NONE;
@@ -6763,6 +6995,7 @@ delete_breakpoint (struct breakpoint *bp
 	    && !b->loc->duplicate
 	    && b->enable_state != bp_disabled
 	    && b->enable_state != bp_shlib_disabled
+	    && !b->pending
 	    && b->enable_state != bp_call_disabled)
 	{
 	  int val;
@@ -6968,6 +7201,10 @@ 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->pending)
+	break;
+
       save_enable = b->enable_state;
       if (b->enable_state != bp_shlib_disabled)
         b->enable_state = bp_disabled;
@@ -7363,70 +7600,92 @@ do_enable_breakpoint (struct breakpoint 
 	error ("Hardware breakpoints used exceeds limit.");
     }
 
-  if (bpt->enable_state != bp_permanent)
-    bpt->enable_state = bp_enabled;
-  bpt->disposition = disposition;
-  check_duplicates (bpt);
-  breakpoints_changed ();
-
-  if (bpt->type == bp_watchpoint || 
-      bpt->type == bp_hardware_watchpoint ||
-      bpt->type == bp_read_watchpoint || 
-      bpt->type == bp_access_watchpoint)
-    {
-      if (bpt->exp_valid_block != NULL)
-	{
-	  struct frame_info *fr =
-	  fr = frame_find_by_id (bpt->watchpoint_frame);
-	  if (fr == NULL)
+  if (bpt->pending)
+    {
+      if (bpt->enable_state != bp_enabled)
+	{
+	  /* When enabling a pending breakpoint, we need to check if the breakpoint
+	     is resolvable since shared libraries could have been loaded
+	     after the breakpoint was disabled.  */
+	  struct breakpoint *new_bp;
+	  breakpoints_changed ();
+ 	  if ((new_bp = resolve_pending_breakpoint (bpt)) != NULL)
 	    {
-	      printf_filtered ("\
-Cannot enable watchpoint %d because the block in which its expression\n\
-is valid is not currently in scope.\n", bpt->number);
-	      bpt->enable_state = bp_disabled;
+	      delete_breakpoint (bpt);
 	      return;
 	    }
-
-	  save_selected_frame = deprecated_selected_frame;
-	  save_selected_frame_level = frame_relative_level (deprecated_selected_frame);
-	  select_frame (fr);
+	  bpt->enable_state = bp_enabled;
+	  bpt->disposition = disposition;
 	}
-
-      value_free (bpt->val);
-      mark = value_mark ();
-      bpt->val = evaluate_expression (bpt->exp);
-      release_value (bpt->val);
-      if (VALUE_LAZY (bpt->val))
-	value_fetch_lazy (bpt->val);
-
-      if (bpt->type == bp_hardware_watchpoint ||
-	  bpt->type == bp_read_watchpoint ||
+    }
+  else  /* Not a pending breakpoint.  */
+    {
+      if (bpt->enable_state != bp_permanent)
+	bpt->enable_state = bp_enabled;
+      bpt->disposition = disposition;
+      check_duplicates (bpt);
+      breakpoints_changed ();
+      
+      if (bpt->type == bp_watchpoint || 
+	  bpt->type == bp_hardware_watchpoint ||
+	  bpt->type == bp_read_watchpoint || 
 	  bpt->type == bp_access_watchpoint)
 	{
-	  int i = hw_watchpoint_used_count (bpt->type, &other_type_used);
-	  int mem_cnt = can_use_hardware_watchpoint (bpt->val);
-
-	  /* Hack around 'unused var' error for some targets here */
-	  (void) mem_cnt, i;
-	  target_resources_ok = TARGET_CAN_USE_HARDWARE_WATCHPOINT (
-				   bpt->type, i + mem_cnt, other_type_used);
-	  /* we can consider of type is bp_hardware_watchpoint, convert to 
-	     bp_watchpoint in the following condition */
-	  if (target_resources_ok < 0)
+	  if (bpt->exp_valid_block != NULL)
 	    {
-	      printf_filtered ("\
+	      struct frame_info *fr =
+		fr = frame_find_by_id (bpt->watchpoint_frame);
+	      if (fr == NULL)
+		{
+		  printf_filtered ("\
+Cannot enable watchpoint %d because the block in which its expression\n\
+is valid is not currently in scope.\n", bpt->number);
+		  bpt->enable_state = bp_disabled;
+		  return;
+		}
+	      
+	      save_selected_frame = deprecated_selected_frame;
+	      save_selected_frame_level = frame_relative_level (deprecated_selected_frame);
+	      select_frame (fr);
+	    }
+	  
+	  value_free (bpt->val);
+	  mark = value_mark ();
+	  bpt->val = evaluate_expression (bpt->exp);
+	  release_value (bpt->val);
+	  if (VALUE_LAZY (bpt->val))
+	    value_fetch_lazy (bpt->val);
+	  
+	  if (bpt->type == bp_hardware_watchpoint ||
+	      bpt->type == bp_read_watchpoint ||
+	      bpt->type == bp_access_watchpoint)
+	    {
+	      int i = hw_watchpoint_used_count (bpt->type, &other_type_used);
+	      int mem_cnt = can_use_hardware_watchpoint (bpt->val);
+	      
+	      /* Hack around 'unused var' error for some targets here */
+	      (void) mem_cnt, i;
+	      target_resources_ok = TARGET_CAN_USE_HARDWARE_WATCHPOINT (
+									bpt->type, i + mem_cnt, other_type_used);
+	      /* we can consider of type is bp_hardware_watchpoint, convert to 
+		 bp_watchpoint in the following condition */
+	      if (target_resources_ok < 0)
+		{
+		  printf_filtered ("\
 Cannot enable watchpoint %d because target watch resources\n\
 have been allocated for other watchpoints.\n", bpt->number);
-	      bpt->enable_state = bp_disabled;
-	      value_free_to_mark (mark);
-	      return;
+		  bpt->enable_state = bp_disabled;
+		  value_free_to_mark (mark);
+		  return;
+		}
 	    }
+	  
+	  if (save_selected_frame_level >= 0)
+	    select_frame (save_selected_frame);
+	  value_free_to_mark (mark);
 	}
-
-      if (save_selected_frame_level >= 0)
-	select_frame (save_selected_frame);
-      value_free_to_mark (mark);
     }
+
   if (modify_breakpoint_hook)
     modify_breakpoint_hook (bpt);
   breakpoint_modify_event (bpt->number);

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

* Re: [RFA]: pending breakpoint support  [1/3]
  2004-01-21 20:52 [RFA]: pending breakpoint support [1/3] Jeff Johnston
@ 2004-01-22 22:27 ` Daniel Jacobowitz
  2004-01-27 20:41   ` J. Johnston
  0 siblings, 1 reply; 17+ messages in thread
From: Daniel Jacobowitz @ 2004-01-22 22:27 UTC (permalink / raw)
  To: Jeff Johnston; +Cc: gdb-patches

On Wed, Jan 21, 2004 at 03:52:42PM -0500, Jeff Johnston wrote:
> This is the first of the last 3 patches for pending breakpoint support.
> This patch adds the actual support.  The other two patches are the
> documentation (previously approved, but before code was accepted) and
> changes to the testsuite.
> 
> This patch accounts for previous comments on the breakpoint code.
> 
> Ok to commit?

Sorry, but a number of my comments from last time still apply, and I
have some others.  The concept is looking great now, though!

> 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	21 Jan 2004 02:34:10 -0000
> @@ -385,6 +385,15 @@ struct breakpoint
>  
>      /* Methods associated with this breakpoint.  */
>      struct breakpoint_ops *ops;
> +
> +    /* Initial from_tty value.  */
> +    int from_tty;
> +
> +    /* Initial flag value.  */
> +    int flag;
> +
> +    /* Is breakpoint pending on shlib loads?  */
> +    int pending;

Can you read this comment and figure out from it what flag is, or even
where to look for more information?  I can't.  Please indicate where
they come from, and that they are used for pending breakpoints. 
Something more descriptive than "flag" would be good.

> @@ -3458,7 +3463,15 @@ print_one_breakpoint (struct breakpoint 
>  	if (addressprint)
>  	  {
>  	    annotate_field (4);
> -	    ui_out_field_core_addr (uiout, "addr", b->loc->address);
> +	    if (b->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;

Last time I said:

  I'm curious what effect this will have on MI consumers.  Also, the
  resulting MI output will be somewhat gruesome with the embedded spaces.
  Maybe:
        ui_out_field_string (uiout, "addr", "<PENDING>");
        ui_out_text (uiout, "addr", "  ");
  but I'm not sure that works without testing it.

  The other possible alternative is not outputting addr at all for
  MI-like uiouts.  May be better to do this (more flexible).

I still don't know what to do about the latter, but please try the
former.

> @@ -4288,23 +4316,139 @@ disable_breakpoints_in_shlibs (int silen
>    }
>  }
>  
> +struct captured_parse_breakpoint_args
> +  {
> +    char **arg_p;
> +    struct symtabs_and_lines *sals_p;
> +    char ***addr_string_p;
> +    int *not_found_ptr;
> +  };
> +
> +struct lang_and_radix
> +  {
> +    enum language lang;
> +    int radix;
> +  };
> +
> +/* Cleanup helper routine to restore the current language and
> +   input radix.  */
> +static void
> +do_restore_lang_radix_cleanup (void *old)
> +{
> +  struct lang_and_radix *p = old;
> +  set_language (p->lang);
> +  input_radix = p->radix;
> +}
> +
> +/* Try and resolve a pending breakpoint.  */
> +static struct breakpoint *
> +resolve_pending_breakpoint (struct breakpoint *b)
> +{
> +  /* 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.  */

Last time I read this patch I read this comment and said "Huh?"  That
still applies :) What pointers in what arg to the end of what
condition?

> +  char **cond_string = (char **) NULL;
> +  char *copy_arg = b->addr_string;
> +  char **addr_string;
> +  char *errmsg;
> +  struct captured_parse_breakpoint_args parse_args;
> +  int rc;
> +  int not_found = 0;
> +  struct ui_file *old_gdb_stderr;
> +  
> +  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;
> +  parse_args.not_found_ptr = &not_found;
> +  
> +  rc = catch_exceptions (uiout, do_captured_parse_breakpoint, 
> +		         &parse_args, NULL, RETURN_MASK_ALL);
> +  
> +  if (rc == GDB_RC_OK)
> +    {
> +      struct lang_and_radix old_lr;
> +      struct cleanup *old_chain;
> +      char *arg;
> +      struct breakpoint *b1;
> +      
> +      printf_filtered ("Pending breakpoint \"%s\" resolved\n", b->addr_string);
> +      
> +      /* Set language, input-radix, then reissue breakpoint command. 
> +	 Ensure the language and input-radix are restored afterwards.  */
> +      old_lr.lang = current_language->la_language;
> +      old_lr.radix = input_radix;
> +      old_chain = make_cleanup (do_restore_lang_radix_cleanup, &old_lr);

Thanks for adding the cleanup.

> +      
> +      set_language (b->language);
> +      input_radix = b->input_radix;
> +      break_command_1 (b->addr_string, b->flag, b->from_tty);
> +      b1 = breakpoint_chain;
> +      while (b1->next)
> +	b1 = b1->next;

I would really prefer that you not walk the breakpoint chain looking
for the last breakpoint.

> +      /* If there is condition specified, it should be copied over.  */
> +      if (b->cond_string)
> +	{
> +	  arg = b->cond_string;
> +	  b1->cond_string = savestring (arg, strlen (arg));
> +	  b1->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
> +	  if (*arg)
> +	    error ("Junk at end of expression");
> +	}
> +      /* If there are commands associated with the breakpoint, they should 
> +         be copied too.  */
> +      if (b->commands)
> +	{
> +	  b1->commands = copy_command_lines (b->commands);
> +	}

No, I've already asked you to do entire section differently:
  This, on the other hand, is not OK.  First of all you're wasting work -
  you parse the breakpoint location twice.  Then you go grubbing around
  in the breakpoint chain looking for it, which assumes it will be added
  at the end of the chain - it will, but that sort of detail shouldn't be
  exposed.  And break_command_1 can currently create multiple
  breakpoints, so you'll get that wrong too.
  
  decode_line_1 can end up prompting the user; you should have a testcase
  for that and make sure it does something sane.  Easiest way is probably
  to put it in gdb.cp using an overloaded function.  All set breakpoints
  should get the condition and commands.
  
  Can you arrange for most of this function to happen inside
  break_command_1, possibly by giving it a pending breakpoint as an
  optional argument?  Other ideas?

Also, I'm 99% sure you mean b1->loc->address above.  b->loc->address is
the pending breakpoint's address, which is unset.

> +
> +      do_cleanups (old_chain);
> +      return b1; /* Pending breakpoint resolved.  */
> +    }
> +
> +  /* Otherwise, we didn't successfully resolve pending breakpoint.  */
> +  return NULL;
> +}
> +
>  /* 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;
> +      }

I said:
  No, use ALL_BREAKPOINTS_SAFE and you can get rid of del_b.

> @@ -4923,19 +5094,51 @@ 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;
> +  parse_args.not_found_ptr = &not_found;
> +
> +  rc = catch_exceptions_with_msg (uiout, do_captured_parse_breakpoint, 
> +		  		  &parse_args, NULL, &err_msg, 
> +				  RETURN_MASK_ALL);

Awesome, the need to play with the text of the error message is gone.

> +
> +  if (rc != GDB_RC_OK)
> +    {
> +      /* Check for file or function not found.  */
> +      if (not_found)
> +	{
> +	  error_output_message (NULL, err_msg);
> +	  xfree (err_msg);
> +	  if (!query ("Make breakpoint pending on future shared library load? "))
> +	    return;

I said:
  I'm still not really happy with this wording.  What does that mean?
  Plus, it'll look really out of place if we don't have shared library
  support.

When reading the testsuite diffs I asked:
  Might deferred be a better name for this concept anyway?

Comments?

> +	  copy_arg = (char *)xmalloc (strlen (addr_start));
> +	  strcpy (copy_arg, addr_start);

I said:
  The cast is not necessary nowadays.  There are also a number of
  functions for doing this, which don't suffer from the off-by-one error
  above :)  Try xstrdup instead.

> +	  addr_string = &copy_arg;

I said:
  Is addr_string guaranteed to still be NULL at this point, or are we
  leaking?

The answer to this question is somewhere in the depths of
decode_line_1, so I'm not sure what the answer is.

> +	  sals.nelts = 1;
> +	  sals.sals = &pending_sal;
> +	  pending_sal.pc = 0;
> +	  pending = 1;
> +	}
> +      else
> +	return;
> +    }
> +  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 *));

Yes, you don't want to free addr_string, but a few lines further down
you probably want to add a cleanup to free copy_arg on the
breakpoint_chain.

> @@ -7363,70 +7600,92 @@ do_enable_breakpoint (struct breakpoint 
>  	error ("Hardware breakpoints used exceeds limit.");
>      }
>  
> -  if (bpt->enable_state != bp_permanent)
> -    bpt->enable_state = bp_enabled;
> -  bpt->disposition = disposition;
> -  check_duplicates (bpt);
> -  breakpoints_changed ();
> -
> -  if (bpt->type == bp_watchpoint || 
> -      bpt->type == bp_hardware_watchpoint ||
> -      bpt->type == bp_read_watchpoint || 
> -      bpt->type == bp_access_watchpoint)
> -    {
> -      if (bpt->exp_valid_block != NULL)
> -	{
> -	  struct frame_info *fr =
> -	  fr = frame_find_by_id (bpt->watchpoint_frame);
> -	  if (fr == NULL)
> +  if (bpt->pending)
> +    {
> +      if (bpt->enable_state != bp_enabled)
> +	{
> +	  /* When enabling a pending breakpoint, we need to check if the breakpoint
> +	     is resolvable since shared libraries could have been loaded
> +	     after the breakpoint was disabled.  */
> +	  struct breakpoint *new_bp;
> +	  breakpoints_changed ();

I said:
  Sure this is necessary?  If we resolve the breakpoint successfully,
  this will get called from set_raw_breakpoint.

> + 	  if ((new_bp = resolve_pending_breakpoint (bpt)) != NULL)

I said:
  Might as well not use assignment-in-if-statement in new code.

I think this is in the ARI.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

* Re: [RFA]: pending breakpoint support  [1/3]
  2004-01-22 22:27 ` Daniel Jacobowitz
@ 2004-01-27 20:41   ` J. Johnston
  2004-01-30  4:13     ` Daniel Jacobowitz
  0 siblings, 1 reply; 17+ messages in thread
From: J. Johnston @ 2004-01-27 20:41 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

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

Daniel Jacobowitz wrote:
> On Wed, Jan 21, 2004 at 03:52:42PM -0500, Jeff Johnston wrote:
> 
>>This is the first of the last 3 patches for pending breakpoint support.
>>This patch adds the actual support.  The other two patches are the
>>documentation (previously approved, but before code was accepted) and
>>changes to the testsuite.
>>
>>This patch accounts for previous comments on the breakpoint code.
>>
>>Ok to commit?
> 
> 
> Sorry, but a number of my comments from last time still apply, and I
> have some others.  The concept is looking great now, though!
> 

Actually, I'm the one who's sorry.  I worked on a number of these issues but 
they were strewn about multiple copies.  I never finished the multiple parsing 
issue.  Anyway, I have a new and improved patch attached that addresses the 
issues below.  I have included answers to questions below as well.

2004-01-26  Jeff Johnston  <jjohnstn@redhat.com>

         * breakpoint.h (struct breakpoint): Add new flag, from_tty,
         and pending fields for pending breakpoint support.
         * breakpoint.c (breakpoint_enabled): Add check for not pending.
         (condition_command): Only parse condition if not a pending
         breakpoint.
         (print_one_breakpoint): Add support for pending breakpoints.
         (describe_other_breakpoints): Add checks to verify we are not
         dealing with pending breakpoints.
         (check_duplicates): Don't check pending breakpoints.
         (set_raw_breakpoint): Initialize pending flag.
         (do_restore_lang_radix_cleanup): New cleanup routine.
         (resolve_pending_breakpoint): New function.
         (re_enable_breakpoints_in_shlibs): Try and resolve any
         pending breakpoints via resolve_pending_breakpoint.
         (mention): Add pending breakpoint support.
         (parse_breakpoint_sals): Add new parameter to pass to
         decode_line_1 to indicate silent errors when files or functions
         are not found.  Change all callers.
         (do_captured_parse_breakpoint): New function.
         (break_command_1): Change prototype to return an rc value and to
         take an optional pending breakpoint pointer.  Support creating
         a pending breakpoint if a "not found" form of error occurs when
         parsing the breakpoint.  Also support resolving an existing pending
         breakpoint and be silent if the resolution fails.
         (create_breakpoints): Change prototype to take pending breakpoint
         pointer.  When resolving a pending breakpoint, use the new pointer
         to provide a conditional or commands added by the end-user.
         (delete_breakpoint): Add appropriate check for pending.
         (breakpoint_re_set_one): Ditto.
         (do_enable_breakpoint): Ditto.


> 
>>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	21 Jan 2004 02:34:10 -0000
>>@@ -385,6 +385,15 @@ struct breakpoint
>> 
>>     /* Methods associated with this breakpoint.  */
>>     struct breakpoint_ops *ops;
>>+
>>+    /* Initial from_tty value.  */
>>+    int from_tty;
>>+
>>+    /* Initial flag value.  */
>>+    int flag;
>>+
>>+    /* Is breakpoint pending on shlib loads?  */
>>+    int pending;
> 
> 
> Can you read this comment and figure out from it what flag is, or even
> where to look for more information?  I can't.  Please indicate where
> they come from, and that they are used for pending breakpoints. 
> Something more descriptive than "flag" would be good.
>

Ok, done.

> 
>>@@ -3458,7 +3463,15 @@ print_one_breakpoint (struct breakpoint 
>> 	if (addressprint)
>> 	  {
>> 	    annotate_field (4);
>>-	    ui_out_field_core_addr (uiout, "addr", b->loc->address);
>>+	    if (b->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;
> 
> 
> Last time I said:
> 
>   I'm curious what effect this will have on MI consumers.  Also, the
>   resulting MI output will be somewhat gruesome with the embedded spaces.
>   Maybe:
>         ui_out_field_string (uiout, "addr", "<PENDING>");
>         ui_out_text (uiout, "addr", "  ");
>   but I'm not sure that works without testing it.
> 
>   The other possible alternative is not outputting addr at all for
>   MI-like uiouts.  May be better to do this (more flexible).
> 
> I still don't know what to do about the latter, but please try the
> former.
> 

The answer appears to be ui_out_spaces() which under mi does nothing and under 
cli does exactly what I want it to do.

> 
>>@@ -4288,23 +4316,139 @@ disable_breakpoints_in_shlibs (int silen
>>   }
>> }
>> 
>>+struct captured_parse_breakpoint_args
>>+  {
>>+    char **arg_p;
>>+    struct symtabs_and_lines *sals_p;
>>+    char ***addr_string_p;
>>+    int *not_found_ptr;
>>+  };
>>+
>>+struct lang_and_radix
>>+  {
>>+    enum language lang;
>>+    int radix;
>>+  };
>>+
>>+/* Cleanup helper routine to restore the current language and
>>+   input radix.  */
>>+static void
>>+do_restore_lang_radix_cleanup (void *old)
>>+{
>>+  struct lang_and_radix *p = old;
>>+  set_language (p->lang);
>>+  input_radix = p->radix;
>>+}
>>+
>>+/* Try and resolve a pending breakpoint.  */
>>+static struct breakpoint *
>>+resolve_pending_breakpoint (struct breakpoint *b)
>>+{
>>+  /* 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.  */
> 
> 
> Last time I read this patch I read this comment and said "Huh?"  That
> still applies :) What pointers in what arg to the end of what
> condition?
>

I agree. Comment removed.

> 
>>+  char **cond_string = (char **) NULL;
>>+  char *copy_arg = b->addr_string;
>>+  char **addr_string;
>>+  char *errmsg;
>>+  struct captured_parse_breakpoint_args parse_args;
>>+  int rc;
>>+  int not_found = 0;
>>+  struct ui_file *old_gdb_stderr;
>>+  
>>+  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;
>>+  parse_args.not_found_ptr = &not_found;
>>+  
>>+  rc = catch_exceptions (uiout, do_captured_parse_breakpoint, 
>>+		         &parse_args, NULL, RETURN_MASK_ALL);
>>+  
>>+  if (rc == GDB_RC_OK)
>>+    {
>>+      struct lang_and_radix old_lr;
>>+      struct cleanup *old_chain;
>>+      char *arg;
>>+      struct breakpoint *b1;
>>+      
>>+      printf_filtered ("Pending breakpoint \"%s\" resolved\n", b->addr_string);
>>+      
>>+      /* Set language, input-radix, then reissue breakpoint command. 
>>+	 Ensure the language and input-radix are restored afterwards.  */
>>+      old_lr.lang = current_language->la_language;
>>+      old_lr.radix = input_radix;
>>+      old_chain = make_cleanup (do_restore_lang_radix_cleanup, &old_lr);
> 
> 
> Thanks for adding the cleanup.
> 
> 
>>+      
>>+      set_language (b->language);
>>+      input_radix = b->input_radix;
>>+      break_command_1 (b->addr_string, b->flag, b->from_tty);
>>+      b1 = breakpoint_chain;
>>+      while (b1->next)
>>+	b1 = b1->next;
> 
> 
> I would really prefer that you not walk the breakpoint chain looking
> for the last breakpoint.
> 

Fixed.  I know pass the pending breakpoint along so I don't have to walk the 
chain anymore.

> 
>>+      /* If there is condition specified, it should be copied over.  */
>>+      if (b->cond_string)
>>+	{
>>+	  arg = b->cond_string;
>>+	  b1->cond_string = savestring (arg, strlen (arg));
>>+	  b1->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
>>+	  if (*arg)
>>+	    error ("Junk at end of expression");
>>+	}
>>+      /* If there are commands associated with the breakpoint, they should 
>>+         be copied too.  */
>>+      if (b->commands)
>>+	{
>>+	  b1->commands = copy_command_lines (b->commands);
>>+	}
> 
> 
> No, I've already asked you to do entire section differently:
>   This, on the other hand, is not OK.  First of all you're wasting work -
>   you parse the breakpoint location twice.  Then you go grubbing around
>   in the breakpoint chain looking for it, which assumes it will be added
>   at the end of the chain - it will, but that sort of detail shouldn't be
>   exposed.  And break_command_1 can currently create multiple
>   breakpoints, so you'll get that wrong too.
>   
>   decode_line_1 can end up prompting the user; you should have a testcase
>   for that and make sure it does something sane.  Easiest way is probably
>   to put it in gdb.cp using an overloaded function.  All set breakpoints
>   should get the condition and commands.
>   
>   Can you arrange for most of this function to happen inside
>   break_command_1, possibly by giving it a pending breakpoint as an
>   optional argument?  Other ideas?
> 
> Also, I'm 99% sure you mean b1->loc->address above.  b->loc->address is
> the pending breakpoint's address, which is unset.
>

You are correct about the loc->address.  I have implemented your suggestion. 
The break_command_1 routine now has a return code and takes an optional argument 
for a pending breakpoint to resolve.  It pass the pending breakpoint pointer to 
the create_breakpoints code so that if multiple breakpoints are created, 
operations can be done when the breakpoint is created rather than finding them 
on the list.  Anyway, the result is that only one parsing is performed and the 
breakpoint list does not need to be traversed.  The caller can also find out if 
the breakpoint was successful without having to use a catch.

One minor change is needed to the pending.exp test because of the redesign. 
Rather than stating the breakpoint is resolved and then showing the new 
breakpoints, the issuing of the message has been moved to after confirmation of 
the resolved breakpoints created.  It has been slightly reworded as well.
> 
>>+
>>+      do_cleanups (old_chain);
>>+      return b1; /* Pending breakpoint resolved.  */
>>+    }
>>+
>>+  /* Otherwise, we didn't successfully resolve pending breakpoint.  */
>>+  return NULL;
>>+}
>>+
>> /* 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;
>>+      }
> 
> 
> I said:
>   No, use ALL_BREAKPOINTS_SAFE and you can get rid of del_b.
> 

Agreed, done.

> 
>>@@ -4923,19 +5094,51 @@ 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;
>>+  parse_args.not_found_ptr = &not_found;
>>+
>>+  rc = catch_exceptions_with_msg (uiout, do_captured_parse_breakpoint, 
>>+		  		  &parse_args, NULL, &err_msg, 
>>+				  RETURN_MASK_ALL);
> 
> 
> Awesome, the need to play with the text of the error message is gone.
> 
> 
>>+
>>+  if (rc != GDB_RC_OK)
>>+    {
>>+      /* Check for file or function not found.  */
>>+      if (not_found)
>>+	{
>>+	  error_output_message (NULL, err_msg);
>>+	  xfree (err_msg);
>>+	  if (!query ("Make breakpoint pending on future shared library load? "))
>>+	    return;
> 
> 
> I said:
>   I'm still not really happy with this wording.  What does that mean?
>   Plus, it'll look really out of place if we don't have shared library
>   support.
> 
> When reading the testsuite diffs I asked:
>   Might deferred be a better name for this concept anyway?
> 
> Comments?
>

This is a case of "you can't please everybody all the time".  I am certainly 
happy with it.  I wanted to distinguish it from "future-break".  The two people 
who are using it are happy with the terminology.  One of them doesn't like the 
word "deferred" - like I said, you can't please everybody.  Anyway, we are 
dealing with a whopping sampling of 4 people here.  The breakpoint is "pending" 
on a future event - that is certainly true.  Yes, it could also be considered 
"deferred" until a future event.  There are multiple choices, I have made my 
choice "pending".  Like all unfamiliar terminology, people get used to it once 
they know what it is.  I am sure there are lots of such instances in gdb - for 
example, "inferior".

Regarding shared libraries: I chose to spell it out at present because that is 
the only way such a breakpoint gets resolved.  I would imagine that on a 
platform without shared libraries, a user wouldn't say "y" and if they did, it 
certainly wouldn't affect them much as it would never occur.  You have to 
remember that this would only occur when the user enters a breakpoint location 
that doesn't exist.  I also mentioned perviously that a future option would be 
added to make it so the query wouldn't be necessary.  The people who use this 
all the time always want it defaulted and I imagine those that don't have shared 
libraries never want it on.  The first step is to get the base functionality in.

> 
>>+	  copy_arg = (char *)xmalloc (strlen (addr_start));
>>+	  strcpy (copy_arg, addr_start);
> 
> 
> I said:
>   The cast is not necessary nowadays.  There are also a number of
>   functions for doing this, which don't suffer from the off-by-one error
>   above :)  Try xstrdup instead.
> 

Ok.

> 
>>+	  addr_string = &copy_arg;
>
> 
> I said:
>   Is addr_string guaranteed to still be NULL at this point, or are we
>   leaking?
> 
> The answer to this question is somewhere in the depths of
> decode_line_1, so I'm not sure what the answer is.
> 

Well, it only gets there on a "not-found" error from decode_line_1 so the onus 
is on decode_line_1 and it's children to do proper cleanups which is outside of 
this patch.

> 
>>+	  sals.nelts = 1;
>>+	  sals.sals = &pending_sal;
>>+	  pending_sal.pc = 0;
>>+	  pending = 1;
>>+	}
>>+      else
>>+	return;
>>+    }
>>+  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 *));
> 
> 
> Yes, you don't want to free addr_string, but a few lines further down
> you probably want to add a cleanup to free copy_arg on the
> breakpoint_chain.
> 

Added, thanks.

> 
>>@@ -7363,70 +7600,92 @@ do_enable_breakpoint (struct breakpoint 
>> 	error ("Hardware breakpoints used exceeds limit.");
>>     }
>> 
>>-  if (bpt->enable_state != bp_permanent)
>>-    bpt->enable_state = bp_enabled;
>>-  bpt->disposition = disposition;
>>-  check_duplicates (bpt);
>>-  breakpoints_changed ();
>>-
>>-  if (bpt->type == bp_watchpoint || 
>>-      bpt->type == bp_hardware_watchpoint ||
>>-      bpt->type == bp_read_watchpoint || 
>>-      bpt->type == bp_access_watchpoint)
>>-    {
>>-      if (bpt->exp_valid_block != NULL)
>>-	{
>>-	  struct frame_info *fr =
>>-	  fr = frame_find_by_id (bpt->watchpoint_frame);
>>-	  if (fr == NULL)
>>+  if (bpt->pending)
>>+    {
>>+      if (bpt->enable_state != bp_enabled)
>>+	{
>>+	  /* When enabling a pending breakpoint, we need to check if the breakpoint
>>+	     is resolvable since shared libraries could have been loaded
>>+	     after the breakpoint was disabled.  */
>>+	  struct breakpoint *new_bp;
>>+	  breakpoints_changed ();
> 
> 
> I said:
>   Sure this is necessary?  If we resolve the breakpoint successfully,
>   this will get called from set_raw_breakpoint.
>

It is necessary because when we aren't successful, the breakpoint status will 
still change from disabled to enabled but we won't have created a new breakpoint.

> 
>>+ 	  if ((new_bp = resolve_pending_breakpoint (bpt)) != NULL)
> 
> 
> I said:
>   Might as well not use assignment-in-if-statement in new code.
> 
> I think this is in the ARI.
>

Fixed.



[-- Attachment #2: pbreak.patch1b --]
[-- Type: text/plain, Size: 28201 bytes --]

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	27 Jan 2004 19:50:31 -0000
@@ -385,6 +385,17 @@ struct breakpoint
 
     /* Methods associated with this breakpoint.  */
     struct breakpoint_ops *ops;
+
+    /* Was breakpoint issued from a tty?  Saved for the use of pending breakpoints.  */
+    int from_tty;
+
+    /* Flag value for pending breakpoint.
+       first bit  : 0 non-temporary, 1 temporary.
+       second bit : 0 normal breakpoint, 1 hardware breakpoint. */
+    int flag;
+
+    /* Is breakpoint pending on shlib loads?  */
+    int pending;
   };
 \f
 /* The following stuff is an abstract data type "bpstat" ("breakpoint
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.151
diff -u -p -r1.151 breakpoint.c
--- breakpoint.c	27 Jan 2004 03:13:34 -0000	1.151
+++ breakpoint.c	27 Jan 2004 19:50:31 -0000
@@ -89,7 +89,7 @@ extern void break_at_finish_at_depth_com
 
 extern void tbreak_at_finish_command (char *, int);
 
-static void break_command_1 (char *, int, int);
+static int break_command_1 (char *, int, int, struct breakpoint *);
 
 static void mention (struct breakpoint *);
 
@@ -119,6 +119,8 @@ static void condition_command (char *, i
 
 static int get_number_trailer (char **, int);
 
+static int do_captured_parse_breakpoint (struct ui_out *, void *);
+
 void set_breakpoint_count (int);
 
 typedef enum
@@ -322,7 +324,7 @@ int exception_support_initialized = 0;
 static int
 breakpoint_enabled (struct breakpoint *b)
 {
-  return b->enable_state == bp_enabled;
+  return (b->enable_state == bp_enabled && !b->pending);
 }
 
 /* Set breakpoint count to NUM.  */
@@ -556,9 +558,12 @@ condition_command (char *arg, int from_t
 	  /* I don't know if it matters whether this is the string the user
 	     typed in or the decompiled expression.  */
 	  b->cond_string = savestring (arg, strlen (arg));
-	  b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
-	  if (*arg)
-	    error ("Junk at end of expression");
+	  if (!b->pending)
+	    {
+	      b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
+	      if (*arg)
+		error ("Junk at end of expression");
+	    }
 	}
       breakpoints_changed ();
       breakpoint_modify_event (b->number);
@@ -3451,7 +3456,16 @@ print_one_breakpoint (struct breakpoint 
 	if (addressprint)
 	  {
 	    annotate_field (4);
-	    ui_out_field_core_addr (uiout, "addr", b->loc->address);
+	    if (b->pending)
+	      {
+		ui_out_field_string (uiout, "addr", "<PENDING>");
+		if (TARGET_ADDR_BIT <= 32)
+		  ui_out_spaces (uiout, 2);
+		else
+		  ui_out_spaces (uiout, 8);
+	      }
+	    else
+	      ui_out_field_core_addr (uiout, "addr", b->loc->address);
 	  }
 	annotate_field (5);
 	*last_addr = b->loc->address;
@@ -3470,6 +3484,10 @@ print_one_breakpoint (struct breakpoint 
 	    ui_out_text (uiout, ":");
 	    ui_out_field_int (uiout, "line", b->line_number);
 	  }
+	else if (b->pending)
+	  {
+	    ui_out_field_string (uiout, "pending", b->addr_string);
+	  }
 	else
 	  {
 	    print_address_symbolic (b->loc->address, stb->stream, demangle, "");
@@ -3506,7 +3524,15 @@ print_one_breakpoint (struct breakpoint 
       ui_out_field_stream (uiout, "cond", stb);
       ui_out_text (uiout, "\n");
     }
-  
+
+  if (b->pending && b->cond_string)
+    {
+      annotate_field (7);
+      ui_out_text (uiout, "\tstop only if ");
+      ui_out_field_string (uiout, "cond", b->cond_string);
+      ui_out_text (uiout, "\n");
+    }
+
   if (b->thread != -1)
     {
       /* FIXME should make an annotation for this */
@@ -3737,14 +3763,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->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->pending && (!overlay_debugging || b->loc->section == section))
 	    {
 	      others--;
 	      printf_filtered ("%d%s%s ",
@@ -3833,6 +3859,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->pending
 	&& b->owner->enable_state != bp_call_disabled
 	&& b->address == address	/* address / overlay match */
 	&& (!overlay_debugging || b->section == section)
@@ -3867,6 +3894,7 @@ check_duplicates (struct breakpoint *bpt
 	  {
 	    if (b->owner->enable_state != bp_disabled
 		&& b->owner->enable_state != bp_shlib_disabled
+		&& !b->owner->pending
 		&& b->owner->enable_state != bp_call_disabled
 		&& b->address == address	/* address / overlay match */
 		&& (!overlay_debugging || b->section == section)
@@ -4043,6 +4071,7 @@ set_raw_breakpoint (struct symtab_and_li
   b->forked_inferior_pid = 0;
   b->exec_pathname = NULL;
   b->ops = NULL;
+  b->pending = 0;
 
   /* Add this breakpoint to the end of the chain
      so that a list of breakpoints will come out in order
@@ -4281,23 +4310,90 @@ disable_breakpoints_in_shlibs (int silen
   }
 }
 
+struct captured_parse_breakpoint_args
+  {
+    char **arg_p;
+    struct symtabs_and_lines *sals_p;
+    char ***addr_string_p;
+    int *not_found_ptr;
+  };
+
+struct lang_and_radix
+  {
+    enum language lang;
+    int radix;
+  };
+
+/* Cleanup helper routine to restore the current language and
+   input radix.  */
+static void
+do_restore_lang_radix_cleanup (void *old)
+{
+  struct lang_and_radix *p = old;
+  set_language (p->lang);
+  input_radix = p->radix;
+}
+
+/* Try and resolve a pending breakpoint.  */
+static int
+resolve_pending_breakpoint (struct breakpoint *b)
+{
+  /* Try and reparse the breakpoint in case the shared library
+     is now loaded.  */
+  struct symtabs_and_lines sals;
+  struct symtab_and_line pending_sal;
+  char **cond_string = (char **) NULL;
+  char *copy_arg = b->addr_string;
+  char **addr_string;
+  char *errmsg;
+  int rc;
+  int not_found = 0;
+  struct ui_file *old_gdb_stderr;
+  struct lang_and_radix old_lr;
+  struct cleanup *old_chain;
+  
+  /* Set language, input-radix, then reissue breakpoint command. 
+     Ensure the language and input-radix are restored afterwards.  */
+  old_lr.lang = current_language->la_language;
+  old_lr.radix = input_radix;
+  old_chain = make_cleanup (do_restore_lang_radix_cleanup, &old_lr);
+  
+  set_language (b->language);
+  input_radix = b->input_radix;
+  rc = break_command_1 (b->addr_string, b->flag, b->from_tty, b);
+  
+  if (rc == GDB_RC_OK)
+    /* Pending breakpoint has been resolved.  */
+    printf_filtered ("Resolves pending breakpoint \"%s\"\n", b->addr_string);
+
+  do_cleanups (old_chain);
+  return rc;
+}
+
 /* Try to reenable any breakpoints in shared libraries.  */
 void
 re_enable_breakpoints_in_shlibs (void)
 {
-  struct breakpoint *b;
+  struct breakpoint *b, *tmp;
 
-  ALL_BREAKPOINTS (b)
+  ALL_BREAKPOINTS_SAFE (b, tmp)
+  {
     if (b->enable_state == bp_shlib_disabled)
-    {
-      char buf[1], *lib;
-
-      /* Do not reenable the breakpoint if the shared library
-         is still not mapped in.  */
-      lib = PC_SOLIB (b->loc->address);
-      if (lib != NULL && target_read_memory (b->loc->address, buf, 1) == 0)
-	b->enable_state = bp_enabled;
-    }
+      {
+	char buf[1], *lib;
+	
+	/* Do not reenable the breakpoint if the shared library
+	   is still not mapped in.  */
+	lib = PC_SOLIB (b->loc->address);
+	if (lib != NULL && target_read_memory (b->loc->address, buf, 1) == 0)
+	  b->enable_state = bp_enabled;
+      }
+    else if (b->pending && (b->enable_state == bp_enabled))
+      {
+	if (resolve_pending_breakpoint (b) == GDB_RC_OK)
+	  delete_breakpoint (b);
+      }
+  }
 }
 
 #endif
@@ -4709,14 +4805,21 @@ mention (struct breakpoint *b)
 
   if (say_where)
     {
-      if (addressprint || b->source_file == NULL)
+      if (b->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))
@@ -4729,6 +4832,11 @@ mention (struct breakpoint *b)
    SALS.sal[i] breakpoint, include the corresponding ADDR_STRING[i],
    COND[i] and COND_STRING[i] values.
 
+   The parameter PENDING_BP points to a pending breakpoint that is
+   the basis of the breakpoints currently being created.  The pending
+   breakpoint may contain a separate condition string or commands
+   that were added after the initial pending breakpoint was created.
+
    NOTE: If the function succeeds, the caller is expected to cleanup
    the arrays ADDR_STRING, COND_STRING, COND and SALS (but not the
    array contents).  If the function fails (error() is called), the
@@ -4739,7 +4847,8 @@ static void
 create_breakpoints (struct symtabs_and_lines sals, char **addr_string,
 		    struct expression **cond, char **cond_string,
 		    enum bptype type, enum bpdisp disposition,
-		    int thread, int ignore_count, int from_tty)
+		    int thread, int ignore_count, int from_tty,
+		    struct breakpoint *pending_bp)
 {
   if (type == bp_hardware_breakpoint)
     {
@@ -4779,6 +4888,26 @@ create_breakpoints (struct symtabs_and_l
 	b->ignore_count = ignore_count;
 	b->enable_state = bp_enabled;
 	b->disposition = disposition;
+	/* If resolving a pending breakpoint, a check must be made to see if
+	   the user has specified a new condition or commands for the 
+	   breakpoint.  A new condition will override any condition that was 
+	   initially specified with the initial breakpoint command.  */
+	if (pending_bp)
+	  {
+	    char *arg;
+	    if (pending_bp->cond_string)
+	      {
+		arg = pending_bp->cond_string;
+		b->cond_string = savestring (arg, strlen (arg));
+		b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
+		if (*arg)
+		  error ("Junk at end of pending breakpoint condition expression");
+	      }
+	    /* If there are commands associated with the breakpoint, they should 
+	       be copied too.  */
+	    if (pending_bp->commands)
+	      b->commands = copy_command_lines (pending_bp->commands);
+	  }
 	mention (b);
       }
   }    
@@ -4792,7 +4921,8 @@ create_breakpoints (struct symtabs_and_l
 static void
 parse_breakpoint_sals (char **address,
 		       struct symtabs_and_lines *sals,
-		       char ***addr_string)
+		       char ***addr_string,
+		       int *not_found_ptr)
 {
   char *addr_start = *address;
   *addr_string = NULL;
@@ -4833,9 +4963,11 @@ parse_breakpoint_sals (char **address,
  	      || ((strchr ("+-", (*address)[0]) != NULL)
  		  && ((*address)[1] != '['))))
 	*sals = decode_line_1 (address, 1, default_breakpoint_symtab,
-			       default_breakpoint_line, addr_string, NULL);
+			       default_breakpoint_line, addr_string, 
+			       not_found_ptr);
       else
-	*sals = decode_line_1 (address, 1, (struct symtab *) NULL, 0, addr_string, NULL);
+	*sals = decode_line_1 (address, 1, (struct symtab *) NULL, 0,
+		               addr_string, not_found_ptr);
     }
   /* For any SAL that didn't have a canonical string, fill one in. */
   if (sals->nelts > 0 && *addr_string == NULL)
@@ -4889,26 +5021,46 @@ breakpoint_sals_to_pc (struct symtabs_an
     }
 }
 
+static int
+do_captured_parse_breakpoint (struct ui_out *ui, void *data)
+{
+  struct captured_parse_breakpoint_args *args = data;
+  
+  parse_breakpoint_sals (args->arg_p, args->sals_p, args->addr_string_p, 
+		         args->not_found_ptr);
+
+  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. */
+   second bit : 0 normal breakpoint, 1 hardware breakpoint. 
 
-static void
-break_command_1 (char *arg, int flag, int from_tty)
+   PENDING_BP is non-NULL when this function is being called to resolve
+   a pending breakpoint.  */
+
+static int
+break_command_1 (char *arg, int flag, int from_tty, struct breakpoint *pending_bp)
 {
   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 *err_msg;
   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;
+  int not_found = 0;
 
   hardwareflag = flag & BP_HARDWAREFLAG;
   tempflag = flag & BP_TEMPFLAG;
@@ -4916,19 +5068,55 @@ 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)
-    return;
+  parse_args.arg_p = &arg;
+  parse_args.sals_p = &sals;
+  parse_args.addr_string_p = &addr_string;
+  parse_args.not_found_ptr = &not_found;
+
+  rc = catch_exceptions_with_msg (uiout, do_captured_parse_breakpoint, 
+		  		  &parse_args, NULL, &err_msg, 
+				  RETURN_MASK_ALL);
+
+  /* If caller is interested in rc value from parse, set value.  */
+
+  if (rc != GDB_RC_OK)
+    {
+      /* Check for file or function not found.  */
+      if (not_found)
+	{
+	  /* If called to resolve pending breakpoint, just return error code.  */
+	  if (pending_bp)
+	    return rc;
+
+	  error_output_message (NULL, err_msg);
+	  xfree (err_msg);
+	  if (!query ("Make breakpoint pending on future shared library load? "))
+	    return rc;
+	  copy_arg = xstrdup (addr_start);
+	  addr_string = &copy_arg;
+	  sals.nelts = 1;
+	  sals.sals = &pending_sal;
+	  pending_sal.pc = 0;
+	  pending = 1;
+	}
+      else
+	return rc;
+    }
+  else if (!sals.nelts)
+    return GDB_RC_FAIL;
 
   /* 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 *));
@@ -4955,62 +5143,94 @@ 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
      breakpoint. */
   thread = -1;			/* No specific thread yet */
-  for (i = 0; i < sals.nelts; i++)
+  if (!pending)
     {
-      char *tok = arg;
-      while (tok && *tok)
+      for (i = 0; i < sals.nelts; i++)
 	{
-	  char *end_tok;
-	  int toklen;
-	  char *cond_start = NULL;
-	  char *cond_end = NULL;
-	  while (*tok == ' ' || *tok == '\t')
-	    tok++;
-
-	  end_tok = tok;
-
-	  while (*end_tok != ' ' && *end_tok != '\t' && *end_tok != '\000')
-	    end_tok++;
-
-	  toklen = end_tok - tok;
-
-	  if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
-	    {
-	      tok = cond_start = end_tok + 1;
-	      cond[i] = parse_exp_1 (&tok, block_for_pc (sals.sals[i].pc), 0);
-	      make_cleanup (xfree, cond[i]);
-	      cond_end = tok;
-	      cond_string[i] = savestring (cond_start, cond_end - cond_start);
-	      make_cleanup (xfree, cond_string[i]);
-	    }
-	  else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
+	  char *tok = arg;
+	  while (tok && *tok)
 	    {
-	      char *tmptok;
-
-	      tok = end_tok + 1;
-	      tmptok = tok;
-	      thread = strtol (tok, &tok, 0);
-	      if (tok == tmptok)
-		error ("Junk after thread keyword.");
-	      if (!valid_thread_id (thread))
-		error ("Unknown thread %d\n", thread);
+	      char *end_tok;
+	      int toklen;
+	      char *cond_start = NULL;
+	      char *cond_end = NULL;
+	      while (*tok == ' ' || *tok == '\t')
+		tok++;
+	      
+	      end_tok = tok;
+	      
+	      while (*end_tok != ' ' && *end_tok != '\t' && *end_tok != '\000')
+		end_tok++;
+	      
+	      toklen = end_tok - tok;
+	      
+	      if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
+		{
+		  tok = cond_start = end_tok + 1;
+		  cond[i] = parse_exp_1 (&tok, block_for_pc (sals.sals[i].pc), 
+				         0);
+		  make_cleanup (xfree, cond[i]);
+		  cond_end = tok;
+		  cond_string[i] = savestring (cond_start, 
+				               cond_end - cond_start);
+		  make_cleanup (xfree, cond_string[i]);
+		}
+	      else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
+		{
+		  char *tmptok;
+		  
+		  tok = end_tok + 1;
+		  tmptok = tok;
+		  thread = strtol (tok, &tok, 0);
+		  if (tok == tmptok)
+		    error ("Junk after thread keyword.");
+		  if (!valid_thread_id (thread))
+		    error ("Unknown thread %d\n", thread);
+		}
+	      else
+		error ("Junk at end of arguments.");
 	    }
-	  else
-	    error ("Junk at end of arguments.");
 	}
+      create_breakpoints (sals, addr_string, cond, cond_string,
+			  hardwareflag ? bp_hardware_breakpoint 
+			  : bp_breakpoint,
+			  tempflag ? disp_del : disp_donttouch,
+			  thread, ignore_count, from_tty,
+			  pending_bp);
     }
+  else
+    {
+      struct symtab_and_line sal;
+      struct breakpoint *b;
 
-  create_breakpoints (sals, addr_string, cond, cond_string,
-		      hardwareflag ? bp_hardware_breakpoint : bp_breakpoint,
-		      tempflag ? disp_del : disp_donttouch,
-		      thread, ignore_count, from_tty);
+      sal.symtab = NULL;
+      sal.pc = 0;
 
+      make_cleanup (xfree, copy_arg);
+
+      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->pending = 1;
+      b->disposition = tempflag ? disp_del : disp_donttouch;
+      b->from_tty = from_tty;
+      b->flag = flag;
+      mention (b);
+    }
+  
   if (sals.nelts > 1)
     {
       warning ("Multiple breakpoints were set.");
@@ -5021,6 +5241,8 @@ break_command_1 (char *arg, int flag, in
   discard_cleanups (breakpoint_chain);
   /* But cleanup everything else. */
   do_cleanups (old_chain);
+
+  return GDB_RC_OK;
 }
 
 /* Set a breakpoint of TYPE/DISPOSITION according to ARG (function,
@@ -5057,7 +5279,7 @@ do_captured_breakpoint (void *data)
   sals.nelts = 0;
   address_end = args->address;
   addr_string = NULL;
-  parse_breakpoint_sals (&address_end, &sals, &addr_string);
+  parse_breakpoint_sals (&address_end, &sals, &addr_string, 0);
 
   if (!sals.nelts)
     return GDB_RC_NONE;
@@ -5121,7 +5343,8 @@ do_captured_breakpoint (void *data)
   create_breakpoints (sals, addr_string, cond, cond_string,
 		      args->hardwareflag ? bp_hardware_breakpoint : bp_breakpoint,
 		      args->tempflag ? disp_del : disp_donttouch,
-		      args->thread, args->ignore_count, 0/*from-tty*/);
+		      args->thread, args->ignore_count, 0/*from-tty*/, 
+		      NULL/*pending_bp*/);
 
   /* That's it. Discard the cleanups for data inserted into the
      breakpoint. */
@@ -5213,7 +5436,7 @@ break_at_finish_at_depth_command_1 (char
 	    addr_string = xstrprintf ("*0x%s %s", paddr_nz (high), extra_args);
 	  else
 	    addr_string = xstrprintf ("*0x%s", paddr_nz (high));
-	  break_command_1 (addr_string, flag, from_tty);
+	  break_command_1 (addr_string, flag, from_tty, NULL);
 	  xfree (addr_string);
 	}
       else
@@ -5296,7 +5519,7 @@ break_at_finish_command_1 (char *arg, in
 				       extra_args);
 	  else
 	    break_string = xstrprintf ("*0x%s", paddr_nz (high));
-	  break_command_1 (break_string, flag, from_tty);
+	  break_command_1 (break_string, flag, from_tty, NULL);
 	  xfree (break_string);
 	}
       else
@@ -5363,7 +5586,7 @@ resolve_sal_pc (struct symtab_and_line *
 void
 break_command (char *arg, int from_tty)
 {
-  break_command_1 (arg, 0, from_tty);
+  break_command_1 (arg, 0, from_tty, NULL);
 }
 
 void
@@ -5381,7 +5604,7 @@ break_at_finish_at_depth_command (char *
 void
 tbreak_command (char *arg, int from_tty)
 {
-  break_command_1 (arg, BP_TEMPFLAG, from_tty);
+  break_command_1 (arg, BP_TEMPFLAG, from_tty, NULL);
 }
 
 void
@@ -5393,13 +5616,13 @@ tbreak_at_finish_command (char *arg, int
 static void
 hbreak_command (char *arg, int from_tty)
 {
-  break_command_1 (arg, BP_HARDWAREFLAG, from_tty);
+  break_command_1 (arg, BP_HARDWAREFLAG, from_tty, NULL);
 }
 
 static void
 thbreak_command (char *arg, int from_tty)
 {
-  break_command_1 (arg, (BP_TEMPFLAG | BP_HARDWAREFLAG), from_tty);
+  break_command_1 (arg, (BP_TEMPFLAG | BP_HARDWAREFLAG), from_tty, NULL);
 }
 
 static void
@@ -5440,7 +5663,7 @@ stopin_command (char *arg, int from_tty)
   if (badInput)
     printf_filtered ("Usage: stop in <function | address>\n");
   else
-    break_command_1 (arg, 0, from_tty);
+    break_command_1 (arg, 0, from_tty, NULL);
 }
 
 static void
@@ -5472,7 +5695,7 @@ stopat_command (char *arg, int from_tty)
   if (badInput)
     printf_filtered ("Usage: stop at <line>\n");
   else
-    break_command_1 (arg, 0, from_tty);
+    break_command_1 (arg, 0, from_tty, NULL);
 }
 
 /* accessflag:  hw_write:  watch write, 
@@ -6679,6 +6902,7 @@ delete_breakpoint (struct breakpoint *bp
 	    && !b->loc->duplicate
 	    && b->enable_state != bp_disabled
 	    && b->enable_state != bp_shlib_disabled
+	    && !b->pending
 	    && b->enable_state != bp_call_disabled)
 	{
 	  int val;
@@ -6884,6 +7108,10 @@ 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->pending)
+	break;
+
       save_enable = b->enable_state;
       if (b->enable_state != bp_shlib_disabled)
         b->enable_state = bp_disabled;
@@ -7279,70 +7507,92 @@ do_enable_breakpoint (struct breakpoint 
 	error ("Hardware breakpoints used exceeds limit.");
     }
 
-  if (bpt->enable_state != bp_permanent)
-    bpt->enable_state = bp_enabled;
-  bpt->disposition = disposition;
-  check_duplicates (bpt);
-  breakpoints_changed ();
-
-  if (bpt->type == bp_watchpoint || 
-      bpt->type == bp_hardware_watchpoint ||
-      bpt->type == bp_read_watchpoint || 
-      bpt->type == bp_access_watchpoint)
-    {
-      if (bpt->exp_valid_block != NULL)
-	{
-	  struct frame_info *fr =
-	  fr = frame_find_by_id (bpt->watchpoint_frame);
-	  if (fr == NULL)
+  if (bpt->pending)
+    {
+      if (bpt->enable_state != bp_enabled)
+	{
+	  /* When enabling a pending breakpoint, we need to check if the breakpoint
+	     is resolvable since shared libraries could have been loaded
+	     after the breakpoint was disabled.  */
+	  struct breakpoint *new_bp;
+	  breakpoints_changed ();
+ 	  if (resolve_pending_breakpoint (bpt) == GDB_RC_OK)
 	    {
-	      printf_filtered ("\
-Cannot enable watchpoint %d because the block in which its expression\n\
-is valid is not currently in scope.\n", bpt->number);
-	      bpt->enable_state = bp_disabled;
+	      delete_breakpoint (bpt);
 	      return;
 	    }
-
-	  save_selected_frame = deprecated_selected_frame;
-	  save_selected_frame_level = frame_relative_level (deprecated_selected_frame);
-	  select_frame (fr);
+	  bpt->enable_state = bp_enabled;
+	  bpt->disposition = disposition;
 	}
-
-      value_free (bpt->val);
-      mark = value_mark ();
-      bpt->val = evaluate_expression (bpt->exp);
-      release_value (bpt->val);
-      if (VALUE_LAZY (bpt->val))
-	value_fetch_lazy (bpt->val);
-
-      if (bpt->type == bp_hardware_watchpoint ||
-	  bpt->type == bp_read_watchpoint ||
+    }
+  else  /* Not a pending breakpoint.  */
+    {
+      if (bpt->enable_state != bp_permanent)
+	bpt->enable_state = bp_enabled;
+      bpt->disposition = disposition;
+      check_duplicates (bpt);
+      breakpoints_changed ();
+      
+      if (bpt->type == bp_watchpoint || 
+	  bpt->type == bp_hardware_watchpoint ||
+	  bpt->type == bp_read_watchpoint || 
 	  bpt->type == bp_access_watchpoint)
 	{
-	  int i = hw_watchpoint_used_count (bpt->type, &other_type_used);
-	  int mem_cnt = can_use_hardware_watchpoint (bpt->val);
-
-	  /* Hack around 'unused var' error for some targets here */
-	  (void) mem_cnt, i;
-	  target_resources_ok = TARGET_CAN_USE_HARDWARE_WATCHPOINT (
-				   bpt->type, i + mem_cnt, other_type_used);
-	  /* we can consider of type is bp_hardware_watchpoint, convert to 
-	     bp_watchpoint in the following condition */
-	  if (target_resources_ok < 0)
+	  if (bpt->exp_valid_block != NULL)
 	    {
-	      printf_filtered ("\
+	      struct frame_info *fr =
+		fr = frame_find_by_id (bpt->watchpoint_frame);
+	      if (fr == NULL)
+		{
+		  printf_filtered ("\
+Cannot enable watchpoint %d because the block in which its expression\n\
+is valid is not currently in scope.\n", bpt->number);
+		  bpt->enable_state = bp_disabled;
+		  return;
+		}
+	      
+	      save_selected_frame = deprecated_selected_frame;
+	      save_selected_frame_level = frame_relative_level (deprecated_selected_frame);
+	      select_frame (fr);
+	    }
+	  
+	  value_free (bpt->val);
+	  mark = value_mark ();
+	  bpt->val = evaluate_expression (bpt->exp);
+	  release_value (bpt->val);
+	  if (VALUE_LAZY (bpt->val))
+	    value_fetch_lazy (bpt->val);
+	  
+	  if (bpt->type == bp_hardware_watchpoint ||
+	      bpt->type == bp_read_watchpoint ||
+	      bpt->type == bp_access_watchpoint)
+	    {
+	      int i = hw_watchpoint_used_count (bpt->type, &other_type_used);
+	      int mem_cnt = can_use_hardware_watchpoint (bpt->val);
+	      
+	      /* Hack around 'unused var' error for some targets here */
+	      (void) mem_cnt, i;
+	      target_resources_ok = TARGET_CAN_USE_HARDWARE_WATCHPOINT (
+									bpt->type, i + mem_cnt, other_type_used);
+	      /* we can consider of type is bp_hardware_watchpoint, convert to 
+		 bp_watchpoint in the following condition */
+	      if (target_resources_ok < 0)
+		{
+		  printf_filtered ("\
 Cannot enable watchpoint %d because target watch resources\n\
 have been allocated for other watchpoints.\n", bpt->number);
-	      bpt->enable_state = bp_disabled;
-	      value_free_to_mark (mark);
-	      return;
+		  bpt->enable_state = bp_disabled;
+		  value_free_to_mark (mark);
+		  return;
+		}
 	    }
+	  
+	  if (save_selected_frame_level >= 0)
+	    select_frame (save_selected_frame);
+	  value_free_to_mark (mark);
 	}
-
-      if (save_selected_frame_level >= 0)
-	select_frame (save_selected_frame);
-      value_free_to_mark (mark);
     }
+
   if (modify_breakpoint_hook)
     modify_breakpoint_hook (bpt);
   breakpoint_modify_event (bpt->number);

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

* Re: [RFA]: pending breakpoint support  [1/3]
  2004-01-27 20:41   ` J. Johnston
@ 2004-01-30  4:13     ` Daniel Jacobowitz
  2004-01-30 18:51       ` Jeff Johnston
  0 siblings, 1 reply; 17+ messages in thread
From: Daniel Jacobowitz @ 2004-01-30  4:13 UTC (permalink / raw)
  To: gdb-patches

On Tue, Jan 27, 2004 at 03:41:04PM -0500, J. Johnston wrote:
> Anyway, I have a new and improved patch attached that 
> addresses the issues below.  I have included answers to questions below as 
> well.

Thanks.  I have just one question and a couple of cosmetic issues left.

I'm also still curious about pending breakpoints which resolve to
multiple functions, et cetera.  We can revisit at a later date.

> >When reading the testsuite diffs I asked:
> >  Might deferred be a better name for this concept anyway?
> >
> >Comments?
> >
> 
> This is a case of "you can't please everybody all the time".  I am 
> certainly happy with it.

OK.  I have some ideas for better wording of the message, but I can
take care of it later.

> Well, it only gets there on a "not-found" error from decode_line_1 so the 
> onus is on decode_line_1 and it's children to do proper cleanups which is 
> outside of this patch.

Gotcha.

About the new patch:

> +    /* Flag value for pending breakpoint.
> +       first bit  : 0 non-temporary, 1 temporary.
> +       second bit : 0 normal breakpoint, 1 hardware breakpoint. */
> +    int flag;

OK, now I can see what flag is used for.  This raises a new question. 
Should temporary breakpoints be allowed to be pending?

Oh, I guess the answer is yes.  tbreak my_dso_initialization; run.  So
this is fine.

> +/* Try and resolve a pending breakpoint.  */
> +static int
> +resolve_pending_breakpoint (struct breakpoint *b)

I love the new version of this function...

> +  input_radix = b->input_radix;
> +  rc = break_command_1 (b->addr_string, b->flag, b->from_tty, b);
> +  
> +  if (rc == GDB_RC_OK)
> +    /* Pending breakpoint has been resolved.  */
> +    printf_filtered ("Resolves pending breakpoint \"%s\"\n", b->addr_string);

Spelling error, though.

> @@ -4779,6 +4888,26 @@ create_breakpoints (struct symtabs_and_l
>  	b->ignore_count = ignore_count;
>  	b->enable_state = bp_enabled;
>  	b->disposition = disposition;
> +	/* If resolving a pending breakpoint, a check must be made to see if
> +	   the user has specified a new condition or commands for the 
> +	   breakpoint.  A new condition will override any condition that was 
> +	   initially specified with the initial breakpoint command.  */
> +	if (pending_bp)
> +	  {
> +	    char *arg;
> +	    if (pending_bp->cond_string)
> +	      {
> +		arg = pending_bp->cond_string;
> +		b->cond_string = savestring (arg, strlen (arg));
> +		b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
> +		if (*arg)
> +		  error ("Junk at end of pending breakpoint condition expression");
> +	      }
> +	    /* If there are commands associated with the breakpoint, they should 
> +	       be copied too.  */
> +	    if (pending_bp->commands)
> +	      b->commands = copy_command_lines (pending_bp->commands);
> +	  }
>  	mention (b);
>        }
>    }    

Here's the one question.

The only way to get here with a PENDING_BP is from break_command_1 from
resolve_pending_breakpoint.  So I don't think it is possible for there
to be a condition already set on B, which makes the comment about
"overriding" such a condition a little strange.  Am I right, or is
there some other way to get a condition to here?

> -static void
> -break_command_1 (char *arg, int flag, int from_tty)
> +   PENDING_BP is non-NULL when this function is being called to resolve
> +   a pending breakpoint.  */
> +
> +static int
> +break_command_1 (char *arg, int flag, int from_tty, struct breakpoint *pending_bp)
>  {
>    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.  */

Aha!  That's where that baffling comment came from, it was already
present in break_command_1.  Here we have a var named ARG, so it makes
more sense, but the variables cond_start and cond_end that it refers to
have walked down about a hundred lines.

> +  if (bpt->pending)
> +    {
> +      if (bpt->enable_state != bp_enabled)
> +	{
> +	  /* When enabling a pending breakpoint, we need to check if the breakpoint
> +	     is resolvable since shared libraries could have been loaded
> +	     after the breakpoint was disabled.  */
> +	  struct breakpoint *new_bp;
> +	  breakpoints_changed ();
> + 	  if (resolve_pending_breakpoint (bpt) == GDB_RC_OK)

new_bp is no longer used, you can remove it.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

* Re: [RFA]: pending breakpoint support  [1/3]
  2004-01-30  4:13     ` Daniel Jacobowitz
@ 2004-01-30 18:51       ` Jeff Johnston
  2004-01-30 19:09         ` Daniel Jacobowitz
  0 siblings, 1 reply; 17+ messages in thread
From: Jeff Johnston @ 2004-01-30 18:51 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

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

Daniel Jacobowitz wrote:

>On Tue, Jan 27, 2004 at 03:41:04PM -0500, J. Johnston wrote:
>  
>
>>Anyway, I have a new and improved patch attached that 
>>addresses the issues below.  I have included answers to questions below as 
>>well.
>>    
>>
>
>Thanks.  I have just one question and a couple of cosmetic issues left.
>
>I'm also still curious about pending breakpoints which resolve to
>multiple functions, et cetera.  We can revisit at a later date.
>  
>
Ok, thanks.

>  
>
>>>When reading the testsuite diffs I asked:
>>> Might deferred be a better name for this concept anyway?
>>>
>>>Comments?
>>>
>>>      
>>>
>>This is a case of "you can't please everybody all the time".  I am 
>>certainly happy with it.
>>    
>>
>
>OK.  I have some ideas for better wording of the message, but I can
>take care of it later.
>  
>
Ok.

>  
>
>>Well, it only gets there on a "not-found" error from decode_line_1 so the 
>>onus is on decode_line_1 and it's children to do proper cleanups which is 
>>outside of this patch.
>>    
>>
>
>Gotcha.
>
>About the new patch:
>
>  
>
>>+    /* Flag value for pending breakpoint.
>>+       first bit  : 0 non-temporary, 1 temporary.
>>+       second bit : 0 normal breakpoint, 1 hardware breakpoint. */
>>+    int flag;
>>    
>>
>
>OK, now I can see what flag is used for.  This raises a new question. 
>Should temporary breakpoints be allowed to be pending?
>
>Oh, I guess the answer is yes.  tbreak my_dso_initialization; run.  So
>this is fine.
>  
>
Great.

>  
>
>>+/* Try and resolve a pending breakpoint.  */
>>+static int
>>+resolve_pending_breakpoint (struct breakpoint *b)
>>    
>>
>
>I love the new version of this function...
>
>  
>
>>+  input_radix = b->input_radix;
>>+  rc = break_command_1 (b->addr_string, b->flag, b->from_tty, b);
>>+  
>>+  if (rc == GDB_RC_OK)
>>+    /* Pending breakpoint has been resolved.  */
>>+    printf_filtered ("Resolves pending breakpoint \"%s\"\n", b->addr_string);
>>    
>>
>
>Spelling error, though.
>  
>
Actually, I meant it to be:

Breakpoint set at.....
Resolves pending breakpoint "....."

So the bottom line ties to the breakpoint just set.  If this isn't very 
clear I can put "which" in front or just make it "Resolved" which I 
think you are alluding to.  If there is some other spelling error, 
please point it out as I don't see it.

>  
>
>>@@ -4779,6 +4888,26 @@ create_breakpoints (struct symtabs_and_l
>> 	b->ignore_count = ignore_count;
>> 	b->enable_state = bp_enabled;
>> 	b->disposition = disposition;
>>+	/* If resolving a pending breakpoint, a check must be made to see if
>>+	   the user has specified a new condition or commands for the 
>>+	   breakpoint.  A new condition will override any condition that was 
>>+	   initially specified with the initial breakpoint command.  */
>>+	if (pending_bp)
>>+	  {
>>+	    char *arg;
>>+	    if (pending_bp->cond_string)
>>+	      {
>>+		arg = pending_bp->cond_string;
>>+		b->cond_string = savestring (arg, strlen (arg));
>>+		b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
>>+		if (*arg)
>>+		  error ("Junk at end of pending breakpoint condition expression");
>>+	      }
>>+	    /* If there are commands associated with the breakpoint, they should 
>>+	       be copied too.  */
>>+	    if (pending_bp->commands)
>>+	      b->commands = copy_command_lines (pending_bp->commands);
>>+	  }
>> 	mention (b);
>>       }
>>   }    
>>    
>>
>
>Here's the one question.
>
>The only way to get here with a PENDING_BP is from break_command_1 from
>resolve_pending_breakpoint.  So I don't think it is possible for there
>to be a condition already set on B, which makes the comment about
>"overriding" such a condition a little strange.  Am I right, or is
>there some other way to get a condition to here?
>  
>
The scenario would be:  1. User creates a pending breakpoint with a 
condition in the break location.  2. User specifies a condition for the 
breakpoint number given back for the pending breakpoint using the 
condition command.  3. The shared library gets loaded that resolves the 
breakpoint.  The resolution of the breakpoint will find the original 
condition in the location string, but won't know about the 2nd one which 
gets stored in the pending breakpoint cond_string (see condition_command 
support for pending breakpoint).

>  
>
>>-static void
>>-break_command_1 (char *arg, int flag, int from_tty)
>>+   PENDING_BP is non-NULL when this function is being called to resolve
>>+   a pending breakpoint.  */
>>+
>>+static int
>>+break_command_1 (char *arg, int flag, int from_tty, struct breakpoint *pending_bp)
>> {
>>   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.  */
>>    
>>
>
>Aha!  That's where that baffling comment came from, it was already
>present in break_command_1.  Here we have a var named ARG, so it makes
>more sense, but the variables cond_start and cond_end that it refers to
>have walked down about a hundred lines.
>  
>
I will remove this comment in break_command_1 too.

>  
>
>>+  if (bpt->pending)
>>+    {
>>+      if (bpt->enable_state != bp_enabled)
>>+	{
>>+	  /* When enabling a pending breakpoint, we need to check if the breakpoint
>>+	     is resolvable since shared libraries could have been loaded
>>+	     after the breakpoint was disabled.  */
>>+	  struct breakpoint *new_bp;
>>+	  breakpoints_changed ();
>>+ 	  if (resolve_pending_breakpoint (bpt) == GDB_RC_OK)
>>    
>>
>
>new_bp is no longer used, you can remove it.
>  
>
Ok, fixed.

I have attached an updated patch.  Ok to commit?

-- Jeff J.

[-- Attachment #2: pbreak.patch1c --]
[-- Type: text/plain, Size: 28170 bytes --]

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	30 Jan 2004 18:48:37 -0000
@@ -385,6 +385,17 @@ struct breakpoint
 
     /* Methods associated with this breakpoint.  */
     struct breakpoint_ops *ops;
+
+    /* Was breakpoint issued from a tty?  Saved for the use of pending breakpoints.  */
+    int from_tty;
+
+    /* Flag value for pending breakpoint.
+       first bit  : 0 non-temporary, 1 temporary.
+       second bit : 0 normal breakpoint, 1 hardware breakpoint. */
+    int flag;
+
+    /* Is breakpoint pending on shlib loads?  */
+    int pending;
   };
 \f
 /* The following stuff is an abstract data type "bpstat" ("breakpoint
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.152
diff -u -p -r1.152 breakpoint.c
--- breakpoint.c	28 Jan 2004 01:39:51 -0000	1.152
+++ breakpoint.c	30 Jan 2004 18:48:38 -0000
@@ -89,7 +89,7 @@ extern void break_at_finish_at_depth_com
 
 extern void tbreak_at_finish_command (char *, int);
 
-static void break_command_1 (char *, int, int);
+static int break_command_1 (char *, int, int, struct breakpoint *);
 
 static void mention (struct breakpoint *);
 
@@ -119,6 +119,8 @@ static void condition_command (char *, i
 
 static int get_number_trailer (char **, int);
 
+static int do_captured_parse_breakpoint (struct ui_out *, void *);
+
 void set_breakpoint_count (int);
 
 typedef enum
@@ -322,7 +324,7 @@ int exception_support_initialized = 0;
 static int
 breakpoint_enabled (struct breakpoint *b)
 {
-  return b->enable_state == bp_enabled;
+  return (b->enable_state == bp_enabled && !b->pending);
 }
 
 /* Set breakpoint count to NUM.  */
@@ -556,9 +558,12 @@ condition_command (char *arg, int from_t
 	  /* I don't know if it matters whether this is the string the user
 	     typed in or the decompiled expression.  */
 	  b->cond_string = savestring (arg, strlen (arg));
-	  b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
-	  if (*arg)
-	    error ("Junk at end of expression");
+	  if (!b->pending)
+	    {
+	      b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
+	      if (*arg)
+		error ("Junk at end of expression");
+	    }
 	}
       breakpoints_changed ();
       breakpoint_modify_event (b->number);
@@ -3451,7 +3456,16 @@ print_one_breakpoint (struct breakpoint 
 	if (addressprint)
 	  {
 	    annotate_field (4);
-	    ui_out_field_core_addr (uiout, "addr", b->loc->address);
+	    if (b->pending)
+	      {
+		ui_out_field_string (uiout, "addr", "<PENDING>");
+		if (TARGET_ADDR_BIT <= 32)
+		  ui_out_spaces (uiout, 2);
+		else
+		  ui_out_spaces (uiout, 8);
+	      }
+	    else
+	      ui_out_field_core_addr (uiout, "addr", b->loc->address);
 	  }
 	annotate_field (5);
 	*last_addr = b->loc->address;
@@ -3470,6 +3484,10 @@ print_one_breakpoint (struct breakpoint 
 	    ui_out_text (uiout, ":");
 	    ui_out_field_int (uiout, "line", b->line_number);
 	  }
+	else if (b->pending)
+	  {
+	    ui_out_field_string (uiout, "pending", b->addr_string);
+	  }
 	else
 	  {
 	    print_address_symbolic (b->loc->address, stb->stream, demangle, "");
@@ -3506,7 +3524,15 @@ print_one_breakpoint (struct breakpoint 
       ui_out_field_stream (uiout, "cond", stb);
       ui_out_text (uiout, "\n");
     }
-  
+
+  if (b->pending && b->cond_string)
+    {
+      annotate_field (7);
+      ui_out_text (uiout, "\tstop only if ");
+      ui_out_field_string (uiout, "cond", b->cond_string);
+      ui_out_text (uiout, "\n");
+    }
+
   if (b->thread != -1)
     {
       /* FIXME should make an annotation for this */
@@ -3737,14 +3763,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->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->pending && (!overlay_debugging || b->loc->section == section))
 	    {
 	      others--;
 	      printf_filtered ("%d%s%s ",
@@ -3833,6 +3859,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->pending
 	&& b->owner->enable_state != bp_call_disabled
 	&& b->address == address	/* address / overlay match */
 	&& (!overlay_debugging || b->section == section)
@@ -3867,6 +3894,7 @@ check_duplicates (struct breakpoint *bpt
 	  {
 	    if (b->owner->enable_state != bp_disabled
 		&& b->owner->enable_state != bp_shlib_disabled
+		&& !b->owner->pending
 		&& b->owner->enable_state != bp_call_disabled
 		&& b->address == address	/* address / overlay match */
 		&& (!overlay_debugging || b->section == section)
@@ -4043,6 +4071,7 @@ set_raw_breakpoint (struct symtab_and_li
   b->forked_inferior_pid = 0;
   b->exec_pathname = NULL;
   b->ops = NULL;
+  b->pending = 0;
 
   /* Add this breakpoint to the end of the chain
      so that a list of breakpoints will come out in order
@@ -4281,23 +4310,90 @@ disable_breakpoints_in_shlibs (int silen
   }
 }
 
+struct captured_parse_breakpoint_args
+  {
+    char **arg_p;
+    struct symtabs_and_lines *sals_p;
+    char ***addr_string_p;
+    int *not_found_ptr;
+  };
+
+struct lang_and_radix
+  {
+    enum language lang;
+    int radix;
+  };
+
+/* Cleanup helper routine to restore the current language and
+   input radix.  */
+static void
+do_restore_lang_radix_cleanup (void *old)
+{
+  struct lang_and_radix *p = old;
+  set_language (p->lang);
+  input_radix = p->radix;
+}
+
+/* Try and resolve a pending breakpoint.  */
+static int
+resolve_pending_breakpoint (struct breakpoint *b)
+{
+  /* Try and reparse the breakpoint in case the shared library
+     is now loaded.  */
+  struct symtabs_and_lines sals;
+  struct symtab_and_line pending_sal;
+  char **cond_string = (char **) NULL;
+  char *copy_arg = b->addr_string;
+  char **addr_string;
+  char *errmsg;
+  int rc;
+  int not_found = 0;
+  struct ui_file *old_gdb_stderr;
+  struct lang_and_radix old_lr;
+  struct cleanup *old_chain;
+  
+  /* Set language, input-radix, then reissue breakpoint command. 
+     Ensure the language and input-radix are restored afterwards.  */
+  old_lr.lang = current_language->la_language;
+  old_lr.radix = input_radix;
+  old_chain = make_cleanup (do_restore_lang_radix_cleanup, &old_lr);
+  
+  set_language (b->language);
+  input_radix = b->input_radix;
+  rc = break_command_1 (b->addr_string, b->flag, b->from_tty, b);
+  
+  if (rc == GDB_RC_OK)
+    /* Pending breakpoint has been resolved.  */
+    printf_filtered ("Resolves pending breakpoint \"%s\"\n", b->addr_string);
+
+  do_cleanups (old_chain);
+  return rc;
+}
+
 /* Try to reenable any breakpoints in shared libraries.  */
 void
 re_enable_breakpoints_in_shlibs (void)
 {
-  struct breakpoint *b;
+  struct breakpoint *b, *tmp;
 
-  ALL_BREAKPOINTS (b)
+  ALL_BREAKPOINTS_SAFE (b, tmp)
+  {
     if (b->enable_state == bp_shlib_disabled)
-    {
-      char buf[1], *lib;
-
-      /* Do not reenable the breakpoint if the shared library
-         is still not mapped in.  */
-      lib = PC_SOLIB (b->loc->address);
-      if (lib != NULL && target_read_memory (b->loc->address, buf, 1) == 0)
-	b->enable_state = bp_enabled;
-    }
+      {
+	char buf[1], *lib;
+	
+	/* Do not reenable the breakpoint if the shared library
+	   is still not mapped in.  */
+	lib = PC_SOLIB (b->loc->address);
+	if (lib != NULL && target_read_memory (b->loc->address, buf, 1) == 0)
+	  b->enable_state = bp_enabled;
+      }
+    else if (b->pending && (b->enable_state == bp_enabled))
+      {
+	if (resolve_pending_breakpoint (b) == GDB_RC_OK)
+	  delete_breakpoint (b);
+      }
+  }
 }
 
 #endif
@@ -4709,14 +4805,21 @@ mention (struct breakpoint *b)
 
   if (say_where)
     {
-      if (addressprint || b->source_file == NULL)
+      if (b->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))
@@ -4729,6 +4832,11 @@ mention (struct breakpoint *b)
    SALS.sal[i] breakpoint, include the corresponding ADDR_STRING[i],
    COND[i] and COND_STRING[i] values.
 
+   The parameter PENDING_BP points to a pending breakpoint that is
+   the basis of the breakpoints currently being created.  The pending
+   breakpoint may contain a separate condition string or commands
+   that were added after the initial pending breakpoint was created.
+
    NOTE: If the function succeeds, the caller is expected to cleanup
    the arrays ADDR_STRING, COND_STRING, COND and SALS (but not the
    array contents).  If the function fails (error() is called), the
@@ -4739,7 +4847,8 @@ static void
 create_breakpoints (struct symtabs_and_lines sals, char **addr_string,
 		    struct expression **cond, char **cond_string,
 		    enum bptype type, enum bpdisp disposition,
-		    int thread, int ignore_count, int from_tty)
+		    int thread, int ignore_count, int from_tty,
+		    struct breakpoint *pending_bp)
 {
   if (type == bp_hardware_breakpoint)
     {
@@ -4779,6 +4888,26 @@ create_breakpoints (struct symtabs_and_l
 	b->ignore_count = ignore_count;
 	b->enable_state = bp_enabled;
 	b->disposition = disposition;
+	/* If resolving a pending breakpoint, a check must be made to see if
+	   the user has specified a new condition or commands for the 
+	   breakpoint.  A new condition will override any condition that was 
+	   initially specified with the initial breakpoint command.  */
+	if (pending_bp)
+	  {
+	    char *arg;
+	    if (pending_bp->cond_string)
+	      {
+		arg = pending_bp->cond_string;
+		b->cond_string = savestring (arg, strlen (arg));
+		b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
+		if (*arg)
+		  error ("Junk at end of pending breakpoint condition expression");
+	      }
+	    /* If there are commands associated with the breakpoint, they should 
+	       be copied too.  */
+	    if (pending_bp->commands)
+	      b->commands = copy_command_lines (pending_bp->commands);
+	  }
 	mention (b);
       }
   }    
@@ -4792,7 +4921,8 @@ create_breakpoints (struct symtabs_and_l
 static void
 parse_breakpoint_sals (char **address,
 		       struct symtabs_and_lines *sals,
-		       char ***addr_string)
+		       char ***addr_string,
+		       int *not_found_ptr)
 {
   char *addr_start = *address;
   *addr_string = NULL;
@@ -4833,9 +4963,11 @@ parse_breakpoint_sals (char **address,
  	      || ((strchr ("+-", (*address)[0]) != NULL)
  		  && ((*address)[1] != '['))))
 	*sals = decode_line_1 (address, 1, default_breakpoint_symtab,
-			       default_breakpoint_line, addr_string, NULL);
+			       default_breakpoint_line, addr_string, 
+			       not_found_ptr);
       else
-	*sals = decode_line_1 (address, 1, (struct symtab *) NULL, 0, addr_string, NULL);
+	*sals = decode_line_1 (address, 1, (struct symtab *) NULL, 0,
+		               addr_string, not_found_ptr);
     }
   /* For any SAL that didn't have a canonical string, fill one in. */
   if (sals->nelts > 0 && *addr_string == NULL)
@@ -4889,26 +5021,44 @@ breakpoint_sals_to_pc (struct symtabs_an
     }
 }
 
+static int
+do_captured_parse_breakpoint (struct ui_out *ui, void *data)
+{
+  struct captured_parse_breakpoint_args *args = data;
+  
+  parse_breakpoint_sals (args->arg_p, args->sals_p, args->addr_string_p, 
+		         args->not_found_ptr);
+
+  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. */
+   second bit : 0 normal breakpoint, 1 hardware breakpoint. 
 
-static void
-break_command_1 (char *arg, int flag, int from_tty)
+   PENDING_BP is non-NULL when this function is being called to resolve
+   a pending breakpoint.  */
+
+static int
+break_command_1 (char *arg, int flag, int from_tty, struct breakpoint *pending_bp)
 {
   int tempflag, hardwareflag;
   struct symtabs_and_lines sals;
   struct expression **cond = 0;
-  /* Pointers in arg to the start, and one past the end, of the
-     condition.  */
+  struct symtab_and_line pending_sal;
   char **cond_string = (char **) NULL;
+  char *copy_arg;
+  char *err_msg;
   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;
+  int not_found = 0;
 
   hardwareflag = flag & BP_HARDWAREFLAG;
   tempflag = flag & BP_TEMPFLAG;
@@ -4916,19 +5066,55 @@ 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)
-    return;
+  parse_args.arg_p = &arg;
+  parse_args.sals_p = &sals;
+  parse_args.addr_string_p = &addr_string;
+  parse_args.not_found_ptr = &not_found;
+
+  rc = catch_exceptions_with_msg (uiout, do_captured_parse_breakpoint, 
+		  		  &parse_args, NULL, &err_msg, 
+				  RETURN_MASK_ALL);
+
+  /* If caller is interested in rc value from parse, set value.  */
+
+  if (rc != GDB_RC_OK)
+    {
+      /* Check for file or function not found.  */
+      if (not_found)
+	{
+	  /* If called to resolve pending breakpoint, just return error code.  */
+	  if (pending_bp)
+	    return rc;
+
+	  error_output_message (NULL, err_msg);
+	  xfree (err_msg);
+	  if (!query ("Make breakpoint pending on future shared library load? "))
+	    return rc;
+	  copy_arg = xstrdup (addr_start);
+	  addr_string = &copy_arg;
+	  sals.nelts = 1;
+	  sals.sals = &pending_sal;
+	  pending_sal.pc = 0;
+	  pending = 1;
+	}
+      else
+	return rc;
+    }
+  else if (!sals.nelts)
+    return GDB_RC_FAIL;
 
   /* 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 *));
@@ -4955,62 +5141,94 @@ 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
      breakpoint. */
   thread = -1;			/* No specific thread yet */
-  for (i = 0; i < sals.nelts; i++)
+  if (!pending)
     {
-      char *tok = arg;
-      while (tok && *tok)
+      for (i = 0; i < sals.nelts; i++)
 	{
-	  char *end_tok;
-	  int toklen;
-	  char *cond_start = NULL;
-	  char *cond_end = NULL;
-	  while (*tok == ' ' || *tok == '\t')
-	    tok++;
-
-	  end_tok = tok;
-
-	  while (*end_tok != ' ' && *end_tok != '\t' && *end_tok != '\000')
-	    end_tok++;
-
-	  toklen = end_tok - tok;
-
-	  if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
-	    {
-	      tok = cond_start = end_tok + 1;
-	      cond[i] = parse_exp_1 (&tok, block_for_pc (sals.sals[i].pc), 0);
-	      make_cleanup (xfree, cond[i]);
-	      cond_end = tok;
-	      cond_string[i] = savestring (cond_start, cond_end - cond_start);
-	      make_cleanup (xfree, cond_string[i]);
-	    }
-	  else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
+	  char *tok = arg;
+	  while (tok && *tok)
 	    {
-	      char *tmptok;
-
-	      tok = end_tok + 1;
-	      tmptok = tok;
-	      thread = strtol (tok, &tok, 0);
-	      if (tok == tmptok)
-		error ("Junk after thread keyword.");
-	      if (!valid_thread_id (thread))
-		error ("Unknown thread %d\n", thread);
+	      char *end_tok;
+	      int toklen;
+	      char *cond_start = NULL;
+	      char *cond_end = NULL;
+	      while (*tok == ' ' || *tok == '\t')
+		tok++;
+	      
+	      end_tok = tok;
+	      
+	      while (*end_tok != ' ' && *end_tok != '\t' && *end_tok != '\000')
+		end_tok++;
+	      
+	      toklen = end_tok - tok;
+	      
+	      if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
+		{
+		  tok = cond_start = end_tok + 1;
+		  cond[i] = parse_exp_1 (&tok, block_for_pc (sals.sals[i].pc), 
+				         0);
+		  make_cleanup (xfree, cond[i]);
+		  cond_end = tok;
+		  cond_string[i] = savestring (cond_start, 
+				               cond_end - cond_start);
+		  make_cleanup (xfree, cond_string[i]);
+		}
+	      else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
+		{
+		  char *tmptok;
+		  
+		  tok = end_tok + 1;
+		  tmptok = tok;
+		  thread = strtol (tok, &tok, 0);
+		  if (tok == tmptok)
+		    error ("Junk after thread keyword.");
+		  if (!valid_thread_id (thread))
+		    error ("Unknown thread %d\n", thread);
+		}
+	      else
+		error ("Junk at end of arguments.");
 	    }
-	  else
-	    error ("Junk at end of arguments.");
 	}
+      create_breakpoints (sals, addr_string, cond, cond_string,
+			  hardwareflag ? bp_hardware_breakpoint 
+			  : bp_breakpoint,
+			  tempflag ? disp_del : disp_donttouch,
+			  thread, ignore_count, from_tty,
+			  pending_bp);
     }
+  else
+    {
+      struct symtab_and_line sal;
+      struct breakpoint *b;
 
-  create_breakpoints (sals, addr_string, cond, cond_string,
-		      hardwareflag ? bp_hardware_breakpoint : bp_breakpoint,
-		      tempflag ? disp_del : disp_donttouch,
-		      thread, ignore_count, from_tty);
+      sal.symtab = NULL;
+      sal.pc = 0;
 
+      make_cleanup (xfree, copy_arg);
+
+      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->pending = 1;
+      b->disposition = tempflag ? disp_del : disp_donttouch;
+      b->from_tty = from_tty;
+      b->flag = flag;
+      mention (b);
+    }
+  
   if (sals.nelts > 1)
     {
       warning ("Multiple breakpoints were set.");
@@ -5021,6 +5239,8 @@ break_command_1 (char *arg, int flag, in
   discard_cleanups (breakpoint_chain);
   /* But cleanup everything else. */
   do_cleanups (old_chain);
+
+  return GDB_RC_OK;
 }
 
 /* Set a breakpoint of TYPE/DISPOSITION according to ARG (function,
@@ -5057,7 +5277,7 @@ do_captured_breakpoint (void *data)
   sals.nelts = 0;
   address_end = args->address;
   addr_string = NULL;
-  parse_breakpoint_sals (&address_end, &sals, &addr_string);
+  parse_breakpoint_sals (&address_end, &sals, &addr_string, 0);
 
   if (!sals.nelts)
     return GDB_RC_NONE;
@@ -5121,7 +5341,8 @@ do_captured_breakpoint (void *data)
   create_breakpoints (sals, addr_string, cond, cond_string,
 		      args->hardwareflag ? bp_hardware_breakpoint : bp_breakpoint,
 		      args->tempflag ? disp_del : disp_donttouch,
-		      args->thread, args->ignore_count, 0/*from-tty*/);
+		      args->thread, args->ignore_count, 0/*from-tty*/, 
+		      NULL/*pending_bp*/);
 
   /* That's it. Discard the cleanups for data inserted into the
      breakpoint. */
@@ -5213,7 +5434,7 @@ break_at_finish_at_depth_command_1 (char
 	    addr_string = xstrprintf ("*0x%s %s", paddr_nz (high), extra_args);
 	  else
 	    addr_string = xstrprintf ("*0x%s", paddr_nz (high));
-	  break_command_1 (addr_string, flag, from_tty);
+	  break_command_1 (addr_string, flag, from_tty, NULL);
 	  xfree (addr_string);
 	}
       else
@@ -5296,7 +5517,7 @@ break_at_finish_command_1 (char *arg, in
 				       extra_args);
 	  else
 	    break_string = xstrprintf ("*0x%s", paddr_nz (high));
-	  break_command_1 (break_string, flag, from_tty);
+	  break_command_1 (break_string, flag, from_tty, NULL);
 	  xfree (break_string);
 	}
       else
@@ -5363,7 +5584,7 @@ resolve_sal_pc (struct symtab_and_line *
 void
 break_command (char *arg, int from_tty)
 {
-  break_command_1 (arg, 0, from_tty);
+  break_command_1 (arg, 0, from_tty, NULL);
 }
 
 void
@@ -5381,7 +5602,7 @@ break_at_finish_at_depth_command (char *
 void
 tbreak_command (char *arg, int from_tty)
 {
-  break_command_1 (arg, BP_TEMPFLAG, from_tty);
+  break_command_1 (arg, BP_TEMPFLAG, from_tty, NULL);
 }
 
 void
@@ -5393,13 +5614,13 @@ tbreak_at_finish_command (char *arg, int
 static void
 hbreak_command (char *arg, int from_tty)
 {
-  break_command_1 (arg, BP_HARDWAREFLAG, from_tty);
+  break_command_1 (arg, BP_HARDWAREFLAG, from_tty, NULL);
 }
 
 static void
 thbreak_command (char *arg, int from_tty)
 {
-  break_command_1 (arg, (BP_TEMPFLAG | BP_HARDWAREFLAG), from_tty);
+  break_command_1 (arg, (BP_TEMPFLAG | BP_HARDWAREFLAG), from_tty, NULL);
 }
 
 static void
@@ -5440,7 +5661,7 @@ stopin_command (char *arg, int from_tty)
   if (badInput)
     printf_filtered ("Usage: stop in <function | address>\n");
   else
-    break_command_1 (arg, 0, from_tty);
+    break_command_1 (arg, 0, from_tty, NULL);
 }
 
 static void
@@ -5472,7 +5693,7 @@ stopat_command (char *arg, int from_tty)
   if (badInput)
     printf_filtered ("Usage: stop at <line>\n");
   else
-    break_command_1 (arg, 0, from_tty);
+    break_command_1 (arg, 0, from_tty, NULL);
 }
 
 /* accessflag:  hw_write:  watch write, 
@@ -6679,6 +6900,7 @@ delete_breakpoint (struct breakpoint *bp
 	    && !b->loc->duplicate
 	    && b->enable_state != bp_disabled
 	    && b->enable_state != bp_shlib_disabled
+	    && !b->pending
 	    && b->enable_state != bp_call_disabled)
 	{
 	  int val;
@@ -6884,6 +7106,10 @@ 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->pending)
+	break;
+
       save_enable = b->enable_state;
       if (b->enable_state != bp_shlib_disabled)
         b->enable_state = bp_disabled;
@@ -7294,70 +7520,91 @@ do_enable_breakpoint (struct breakpoint 
 	error ("Hardware breakpoints used exceeds limit.");
     }
 
-  if (bpt->enable_state != bp_permanent)
-    bpt->enable_state = bp_enabled;
-  bpt->disposition = disposition;
-  check_duplicates (bpt);
-  breakpoints_changed ();
-
-  if (bpt->type == bp_watchpoint || 
-      bpt->type == bp_hardware_watchpoint ||
-      bpt->type == bp_read_watchpoint || 
-      bpt->type == bp_access_watchpoint)
-    {
-      if (bpt->exp_valid_block != NULL)
-	{
-	  struct frame_info *fr =
-	  fr = frame_find_by_id (bpt->watchpoint_frame);
-	  if (fr == NULL)
+  if (bpt->pending)
+    {
+      if (bpt->enable_state != bp_enabled)
+	{
+	  /* When enabling a pending breakpoint, we need to check if the breakpoint
+	     is resolvable since shared libraries could have been loaded
+	     after the breakpoint was disabled.  */
+	  breakpoints_changed ();
+ 	  if (resolve_pending_breakpoint (bpt) == GDB_RC_OK)
 	    {
-	      printf_filtered ("\
-Cannot enable watchpoint %d because the block in which its expression\n\
-is valid is not currently in scope.\n", bpt->number);
-	      bpt->enable_state = bp_disabled;
+	      delete_breakpoint (bpt);
 	      return;
 	    }
-
-	  save_selected_frame = deprecated_selected_frame;
-	  save_selected_frame_level = frame_relative_level (deprecated_selected_frame);
-	  select_frame (fr);
+	  bpt->enable_state = bp_enabled;
+	  bpt->disposition = disposition;
 	}
-
-      value_free (bpt->val);
-      mark = value_mark ();
-      bpt->val = evaluate_expression (bpt->exp);
-      release_value (bpt->val);
-      if (VALUE_LAZY (bpt->val))
-	value_fetch_lazy (bpt->val);
-
-      if (bpt->type == bp_hardware_watchpoint ||
-	  bpt->type == bp_read_watchpoint ||
+    }
+  else  /* Not a pending breakpoint.  */
+    {
+      if (bpt->enable_state != bp_permanent)
+	bpt->enable_state = bp_enabled;
+      bpt->disposition = disposition;
+      check_duplicates (bpt);
+      breakpoints_changed ();
+      
+      if (bpt->type == bp_watchpoint || 
+	  bpt->type == bp_hardware_watchpoint ||
+	  bpt->type == bp_read_watchpoint || 
 	  bpt->type == bp_access_watchpoint)
 	{
-	  int i = hw_watchpoint_used_count (bpt->type, &other_type_used);
-	  int mem_cnt = can_use_hardware_watchpoint (bpt->val);
-
-	  /* Hack around 'unused var' error for some targets here */
-	  (void) mem_cnt, i;
-	  target_resources_ok = TARGET_CAN_USE_HARDWARE_WATCHPOINT (
-				   bpt->type, i + mem_cnt, other_type_used);
-	  /* we can consider of type is bp_hardware_watchpoint, convert to 
-	     bp_watchpoint in the following condition */
-	  if (target_resources_ok < 0)
+	  if (bpt->exp_valid_block != NULL)
 	    {
-	      printf_filtered ("\
+	      struct frame_info *fr =
+		fr = frame_find_by_id (bpt->watchpoint_frame);
+	      if (fr == NULL)
+		{
+		  printf_filtered ("\
+Cannot enable watchpoint %d because the block in which its expression\n\
+is valid is not currently in scope.\n", bpt->number);
+		  bpt->enable_state = bp_disabled;
+		  return;
+		}
+	      
+	      save_selected_frame = deprecated_selected_frame;
+	      save_selected_frame_level = frame_relative_level (deprecated_selected_frame);
+	      select_frame (fr);
+	    }
+	  
+	  value_free (bpt->val);
+	  mark = value_mark ();
+	  bpt->val = evaluate_expression (bpt->exp);
+	  release_value (bpt->val);
+	  if (VALUE_LAZY (bpt->val))
+	    value_fetch_lazy (bpt->val);
+	  
+	  if (bpt->type == bp_hardware_watchpoint ||
+	      bpt->type == bp_read_watchpoint ||
+	      bpt->type == bp_access_watchpoint)
+	    {
+	      int i = hw_watchpoint_used_count (bpt->type, &other_type_used);
+	      int mem_cnt = can_use_hardware_watchpoint (bpt->val);
+	      
+	      /* Hack around 'unused var' error for some targets here */
+	      (void) mem_cnt, i;
+	      target_resources_ok = TARGET_CAN_USE_HARDWARE_WATCHPOINT (
+									bpt->type, i + mem_cnt, other_type_used);
+	      /* we can consider of type is bp_hardware_watchpoint, convert to 
+		 bp_watchpoint in the following condition */
+	      if (target_resources_ok < 0)
+		{
+		  printf_filtered ("\
 Cannot enable watchpoint %d because target watch resources\n\
 have been allocated for other watchpoints.\n", bpt->number);
-	      bpt->enable_state = bp_disabled;
-	      value_free_to_mark (mark);
-	      return;
+		  bpt->enable_state = bp_disabled;
+		  value_free_to_mark (mark);
+		  return;
+		}
 	    }
+	  
+	  if (save_selected_frame_level >= 0)
+	    select_frame (save_selected_frame);
+	  value_free_to_mark (mark);
 	}
-
-      if (save_selected_frame_level >= 0)
-	select_frame (save_selected_frame);
-      value_free_to_mark (mark);
     }
+
   if (modify_breakpoint_hook)
     modify_breakpoint_hook (bpt);
   breakpoint_modify_event (bpt->number);

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

* Re: [RFA]: pending breakpoint support  [1/3]
  2004-01-30 18:51       ` Jeff Johnston
@ 2004-01-30 19:09         ` Daniel Jacobowitz
  2004-01-30 22:46           ` Jeff Johnston
  0 siblings, 1 reply; 17+ messages in thread
From: Daniel Jacobowitz @ 2004-01-30 19:09 UTC (permalink / raw)
  To: Jeff Johnston; +Cc: gdb-patches

On Fri, Jan 30, 2004 at 01:51:07PM -0500, Jeff Johnston wrote:
> >>+  input_radix = b->input_radix;
> >>+  rc = break_command_1 (b->addr_string, b->flag, b->from_tty, b);
> >>+  
> >>+  if (rc == GDB_RC_OK)
> >>+    /* Pending breakpoint has been resolved.  */
> >>+    printf_filtered ("Resolves pending breakpoint \"%s\"\n", 
> >>b->addr_string);
> >>   
> >>
> >
> >Spelling error, though.
> > 
> >
> Actually, I meant it to be:
> 
> Breakpoint set at.....
> Resolves pending breakpoint "....."
> 
> So the bottom line ties to the breakpoint just set.  If this isn't very 
> clear I can put "which" in front or just make it "Resolved" which I 
> think you are alluding to.  If there is some other spelling error, 
> please point it out as I don't see it.

I read it and assumed that you meant "Resolved".  I think that
"resolves" is grammatically confusing here, since there's no implicit
subject.  How about "Pending breakpoint \"%s\" resolved"?

> 
> > 
> >
> >>@@ -4779,6 +4888,26 @@ create_breakpoints (struct symtabs_and_l
> >>	b->ignore_count = ignore_count;
> >>	b->enable_state = bp_enabled;
> >>	b->disposition = disposition;
> >>+	/* If resolving a pending breakpoint, a check must be made to see if
> >>+	   the user has specified a new condition or commands for the 
> >>+	   breakpoint.  A new condition will override any condition that was 
> >>+	   initially specified with the initial breakpoint command.  */
> >>+	if (pending_bp)
> >>+	  {
> >>+	    char *arg;
> >>+	    if (pending_bp->cond_string)
> >>+	      {
> >>+		arg = pending_bp->cond_string;
> >>+		b->cond_string = savestring (arg, strlen (arg));
> >>+		b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 
> >>0);
> >>+		if (*arg)
> >>+		  error ("Junk at end of pending breakpoint condition 
> >>expression");
> >>+	      }
> >>+	    /* If there are commands associated with the breakpoint, they 
> >>should +	       be copied too.  */
> >>+	    if (pending_bp->commands)
> >>+	      b->commands = copy_command_lines (pending_bp->commands);
> >>+	  }
> >>	mention (b);
> >>      }
> >>  }    
> >>   
> >>
> >
> >Here's the one question.
> >
> >The only way to get here with a PENDING_BP is from break_command_1 from
> >resolve_pending_breakpoint.  So I don't think it is possible for there
> >to be a condition already set on B, which makes the comment about
> >"overriding" such a condition a little strange.  Am I right, or is
> >there some other way to get a condition to here?
> > 
> >
> The scenario would be:  1. User creates a pending breakpoint with a 
> condition in the break location.  2. User specifies a condition for the 
> breakpoint number given back for the pending breakpoint using the 
> condition command.  3. The shared library gets loaded that resolves the 
> breakpoint.  The resolution of the breakpoint will find the original 
> condition in the location string, but won't know about the 2nd one which 
> gets stored in the pending breakpoint cond_string (see condition_command 
> support for pending breakpoint).

I'd have thought the original condition would have been removed from
addr_string already.  Can we not do that without being able to parse
the location?  Won't this produce confusing "info breakpoints" output?


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

* Re: [RFA]: pending breakpoint support  [1/3]
  2004-01-30 19:09         ` Daniel Jacobowitz
@ 2004-01-30 22:46           ` Jeff Johnston
  2004-01-30 23:45             ` Daniel Jacobowitz
  0 siblings, 1 reply; 17+ messages in thread
From: Jeff Johnston @ 2004-01-30 22:46 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

Daniel Jacobowitz wrote:

>On Fri, Jan 30, 2004 at 01:51:07PM -0500, Jeff Johnston wrote:
>  
>
>>>>+  input_radix = b->input_radix;
>>>>+  rc = break_command_1 (b->addr_string, b->flag, b->from_tty, b);
>>>>+  
>>>>+  if (rc == GDB_RC_OK)
>>>>+    /* Pending breakpoint has been resolved.  */
>>>>+    printf_filtered ("Resolves pending breakpoint \"%s\"\n", 
>>>>b->addr_string);
>>>>  
>>>>
>>>>        
>>>>
>>>Spelling error, though.
>>>
>>>
>>>      
>>>
>>Actually, I meant it to be:
>>
>>Breakpoint set at.....
>>Resolves pending breakpoint "....."
>>
>>So the bottom line ties to the breakpoint just set.  If this isn't very 
>>clear I can put "which" in front or just make it "Resolved" which I 
>>think you are alluding to.  If there is some other spelling error, 
>>please point it out as I don't see it.
>>    
>>
>
>I read it and assumed that you meant "Resolved".  I think that
>"resolves" is grammatically confusing here, since there's no implicit
>subject.  How about "Pending breakpoint \"%s\" resolved"?
>  
>
Ok.

>  
>
>>>      
>>>
>>>>@@ -4779,6 +4888,26 @@ create_breakpoints (struct symtabs_and_l
>>>>	b->ignore_count = ignore_count;
>>>>	b->enable_state = bp_enabled;
>>>>	b->disposition = disposition;
>>>>+	/* If resolving a pending breakpoint, a check must be made to see if
>>>>+	   the user has specified a new condition or commands for the 
>>>>+	   breakpoint.  A new condition will override any condition that was 
>>>>+	   initially specified with the initial breakpoint command.  */
>>>>+	if (pending_bp)
>>>>+	  {
>>>>+	    char *arg;
>>>>+	    if (pending_bp->cond_string)
>>>>+	      {
>>>>+		arg = pending_bp->cond_string;
>>>>+		b->cond_string = savestring (arg, strlen (arg));
>>>>+		b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 
>>>>0);
>>>>+		if (*arg)
>>>>+		  error ("Junk at end of pending breakpoint condition 
>>>>expression");
>>>>+	      }
>>>>+	    /* If there are commands associated with the breakpoint, they 
>>>>should +	       be copied too.  */
>>>>+	    if (pending_bp->commands)
>>>>+	      b->commands = copy_command_lines (pending_bp->commands);
>>>>+	  }
>>>>	mention (b);
>>>>     }
>>>> }    
>>>>  
>>>>
>>>>        
>>>>
>>>Here's the one question.
>>>
>>>The only way to get here with a PENDING_BP is from break_command_1 from
>>>resolve_pending_breakpoint.  So I don't think it is possible for there
>>>to be a condition already set on B, which makes the comment about
>>>"overriding" such a condition a little strange.  Am I right, or is
>>>there some other way to get a condition to here?
>>>
>>>
>>>      
>>>
>>The scenario would be:  1. User creates a pending breakpoint with a 
>>condition in the break location.  2. User specifies a condition for the 
>>breakpoint number given back for the pending breakpoint using the 
>>condition command.  3. The shared library gets loaded that resolves the 
>>breakpoint.  The resolution of the breakpoint will find the original 
>>condition in the location string, but won't know about the 2nd one which 
>>gets stored in the pending breakpoint cond_string (see condition_command 
>>support for pending breakpoint).
>>    
>>
>
>I'd have thought the original condition would have been removed from
>addr_string already.  Can we not do that without being able to parse
>the location?  Won't this produce confusing "info breakpoints" output?
>
>  
>
Well, therein lies the problem.  The word "if" might or might not be 
part of the symbol.  The
regular logic relies on parsing out the symbol first and then looking at 
the aftermath.  I don't have
that luxury so I punt.  It may be slightly confusing if the user does 
the scenario above, but the
displayed pending breakpoint info is meant to be the "original 
breakpoint string" so I don't anticipate
the user will object too much.  Ok or do you have a better idea?

-- Jeff J.


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

* Re: [RFA]: pending breakpoint support  [1/3]
  2004-01-30 22:46           ` Jeff Johnston
@ 2004-01-30 23:45             ` Daniel Jacobowitz
  2004-01-31  0:33               ` Jeff Johnston
  0 siblings, 1 reply; 17+ messages in thread
From: Daniel Jacobowitz @ 2004-01-30 23:45 UTC (permalink / raw)
  To: Jeff Johnston; +Cc: gdb-patches

On Fri, Jan 30, 2004 at 05:45:55PM -0500, Jeff Johnston wrote:
> Well, therein lies the problem.  The word "if" might or might not be 
> part of the symbol.  The
> regular logic relies on parsing out the symbol first and then looking at 
> the aftermath.  I don't have
> that luxury so I punt.  It may be slightly confusing if the user does 
> the scenario above, but the
> displayed pending breakpoint info is meant to be the "original 
> breakpoint string" so I don't anticipate
> the user will object too much.  Ok or do you have a better idea?

Right, I see.  This is fine.  The patch is OK to commit with the
"resolved" wording change - thanks for your patience!

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

* Re: [RFA]: pending breakpoint support  [1/3]
  2004-01-30 23:45             ` Daniel Jacobowitz
@ 2004-01-31  0:33               ` Jeff Johnston
  2004-02-02 21:12                 ` Jeff Johnston
  0 siblings, 1 reply; 17+ messages in thread
From: Jeff Johnston @ 2004-01-31  0:33 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

Daniel Jacobowitz wrote:

>On Fri, Jan 30, 2004 at 05:45:55PM -0500, Jeff Johnston wrote:
>  
>
>>Well, therein lies the problem.  The word "if" might or might not be 
>>part of the symbol.  The
>>regular logic relies on parsing out the symbol first and then looking at 
>>the aftermath.  I don't have
>>that luxury so I punt.  It may be slightly confusing if the user does 
>>the scenario above, but the
>>displayed pending breakpoint info is meant to be the "original 
>>breakpoint string" so I don't anticipate
>>the user will object too much.  Ok or do you have a better idea?
>>    
>>
>
>Right, I see.  This is fine.  The patch is OK to commit with the
>"resolved" wording change - thanks for your patience!
>  
>

Thanks.


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

* Re: [RFA]: pending breakpoint support  [1/3]
  2004-01-31  0:33               ` Jeff Johnston
@ 2004-02-02 21:12                 ` Jeff Johnston
  2004-02-02 21:22                   ` Daniel Jacobowitz
  0 siblings, 1 reply; 17+ messages in thread
From: Jeff Johnston @ 2004-02-02 21:12 UTC (permalink / raw)
  To: Jeff Johnston; +Cc: Daniel Jacobowitz, gdb-patches

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

Patch checked in.  I have included the final patch here with the 
reworded message.

-- Jeff J.

Jeff Johnston wrote:

> Daniel Jacobowitz wrote:
>
>> On Fri, Jan 30, 2004 at 05:45:55PM -0500, Jeff Johnston wrote:
>>  
>>
>>> Well, therein lies the problem.  The word "if" might or might not be 
>>> part of the symbol.  The
>>> regular logic relies on parsing out the symbol first and then 
>>> looking at the aftermath.  I don't have
>>> that luxury so I punt.  It may be slightly confusing if the user 
>>> does the scenario above, but the
>>> displayed pending breakpoint info is meant to be the "original 
>>> breakpoint string" so I don't anticipate
>>> the user will object too much.  Ok or do you have a better idea?
>>>   
>>
>>
>> Right, I see.  This is fine.  The patch is OK to commit with the
>> "resolved" wording change - thanks for your patience!
>>  
>>
>
> Thanks.
>
>

[-- Attachment #2: pbreak.patch1d --]
[-- Type: text/plain, Size: 28167 bytes --]

Index: breakpoint.h
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.h,v
retrieving revision 1.30
diff -u -p -r1.30 breakpoint.h
--- breakpoint.h	1 Feb 2004 18:05:09 -0000	1.30
+++ breakpoint.h	2 Feb 2004 21:10:31 -0000
@@ -386,6 +386,17 @@ struct breakpoint
 
     /* Methods associated with this breakpoint.  */
     struct breakpoint_ops *ops;
+
+    /* Was breakpoint issued from a tty?  Saved for the use of pending breakpoints.  */
+    int from_tty;
+
+    /* Flag value for pending breakpoint.
+       first bit  : 0 non-temporary, 1 temporary.
+       second bit : 0 normal breakpoint, 1 hardware breakpoint. */
+    int flag;
+
+    /* Is breakpoint pending on shlib loads?  */
+    int pending;
   };
 \f
 /* The following stuff is an abstract data type "bpstat" ("breakpoint
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.156
diff -u -p -r1.156 breakpoint.c
--- breakpoint.c	1 Feb 2004 18:05:09 -0000	1.156
+++ breakpoint.c	2 Feb 2004 21:10:31 -0000
@@ -89,7 +89,7 @@ extern void break_at_finish_at_depth_com
 
 extern void tbreak_at_finish_command (char *, int);
 
-static void break_command_1 (char *, int, int);
+static int break_command_1 (char *, int, int, struct breakpoint *);
 
 static void mention (struct breakpoint *);
 
@@ -119,6 +119,8 @@ static void condition_command (char *, i
 
 static int get_number_trailer (char **, int);
 
+static int do_captured_parse_breakpoint (struct ui_out *, void *);
+
 void set_breakpoint_count (int);
 
 typedef enum
@@ -322,7 +324,7 @@ int exception_support_initialized = 0;
 static int
 breakpoint_enabled (struct breakpoint *b)
 {
-  return b->enable_state == bp_enabled;
+  return (b->enable_state == bp_enabled && !b->pending);
 }
 
 /* Set breakpoint count to NUM.  */
@@ -556,9 +558,12 @@ condition_command (char *arg, int from_t
 	  /* I don't know if it matters whether this is the string the user
 	     typed in or the decompiled expression.  */
 	  b->cond_string = savestring (arg, strlen (arg));
-	  b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
-	  if (*arg)
-	    error ("Junk at end of expression");
+	  if (!b->pending)
+	    {
+	      b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
+	      if (*arg)
+		error ("Junk at end of expression");
+	    }
 	}
       breakpoints_changed ();
       breakpoint_modify_event (b->number);
@@ -3465,7 +3470,16 @@ print_one_breakpoint (struct breakpoint 
 	if (addressprint)
 	  {
 	    annotate_field (4);
-	    ui_out_field_core_addr (uiout, "addr", b->loc->address);
+	    if (b->pending)
+	      {
+		ui_out_field_string (uiout, "addr", "<PENDING>");
+		if (TARGET_ADDR_BIT <= 32)
+		  ui_out_spaces (uiout, 2);
+		else
+		  ui_out_spaces (uiout, 8);
+	      }
+	    else
+	      ui_out_field_core_addr (uiout, "addr", b->loc->address);
 	  }
 	annotate_field (5);
 	*last_addr = b->loc->address;
@@ -3484,6 +3498,10 @@ print_one_breakpoint (struct breakpoint 
 	    ui_out_text (uiout, ":");
 	    ui_out_field_int (uiout, "line", b->line_number);
 	  }
+	else if (b->pending)
+	  {
+	    ui_out_field_string (uiout, "pending", b->addr_string);
+	  }
 	else
 	  {
 	    print_address_symbolic (b->loc->address, stb->stream, demangle, "");
@@ -3520,7 +3538,15 @@ print_one_breakpoint (struct breakpoint 
       ui_out_field_stream (uiout, "cond", stb);
       ui_out_text (uiout, "\n");
     }
-  
+
+  if (b->pending && b->cond_string)
+    {
+      annotate_field (7);
+      ui_out_text (uiout, "\tstop only if ");
+      ui_out_field_string (uiout, "cond", b->cond_string);
+      ui_out_text (uiout, "\n");
+    }
+
   if (b->thread != -1)
     {
       /* FIXME should make an annotation for this */
@@ -3751,14 +3777,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->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->pending && (!overlay_debugging || b->loc->section == section))
 	    {
 	      others--;
 	      printf_filtered ("%d%s%s ",
@@ -3847,6 +3873,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->pending
 	&& b->owner->enable_state != bp_call_disabled
 	&& b->address == address	/* address / overlay match */
 	&& (!overlay_debugging || b->section == section)
@@ -3881,6 +3908,7 @@ check_duplicates (struct breakpoint *bpt
 	  {
 	    if (b->owner->enable_state != bp_disabled
 		&& b->owner->enable_state != bp_shlib_disabled
+		&& !b->owner->pending
 		&& b->owner->enable_state != bp_call_disabled
 		&& b->address == address	/* address / overlay match */
 		&& (!overlay_debugging || b->section == section)
@@ -4057,6 +4085,7 @@ set_raw_breakpoint (struct symtab_and_li
   b->forked_inferior_pid = 0;
   b->exec_pathname = NULL;
   b->ops = NULL;
+  b->pending = 0;
 
   /* Add this breakpoint to the end of the chain
      so that a list of breakpoints will come out in order
@@ -4295,23 +4324,90 @@ disable_breakpoints_in_shlibs (int silen
   }
 }
 
+struct captured_parse_breakpoint_args
+  {
+    char **arg_p;
+    struct symtabs_and_lines *sals_p;
+    char ***addr_string_p;
+    int *not_found_ptr;
+  };
+
+struct lang_and_radix
+  {
+    enum language lang;
+    int radix;
+  };
+
+/* Cleanup helper routine to restore the current language and
+   input radix.  */
+static void
+do_restore_lang_radix_cleanup (void *old)
+{
+  struct lang_and_radix *p = old;
+  set_language (p->lang);
+  input_radix = p->radix;
+}
+
+/* Try and resolve a pending breakpoint.  */
+static int
+resolve_pending_breakpoint (struct breakpoint *b)
+{
+  /* Try and reparse the breakpoint in case the shared library
+     is now loaded.  */
+  struct symtabs_and_lines sals;
+  struct symtab_and_line pending_sal;
+  char **cond_string = (char **) NULL;
+  char *copy_arg = b->addr_string;
+  char **addr_string;
+  char *errmsg;
+  int rc;
+  int not_found = 0;
+  struct ui_file *old_gdb_stderr;
+  struct lang_and_radix old_lr;
+  struct cleanup *old_chain;
+  
+  /* Set language, input-radix, then reissue breakpoint command. 
+     Ensure the language and input-radix are restored afterwards.  */
+  old_lr.lang = current_language->la_language;
+  old_lr.radix = input_radix;
+  old_chain = make_cleanup (do_restore_lang_radix_cleanup, &old_lr);
+  
+  set_language (b->language);
+  input_radix = b->input_radix;
+  rc = break_command_1 (b->addr_string, b->flag, b->from_tty, b);
+  
+  if (rc == GDB_RC_OK)
+    /* Pending breakpoint has been resolved.  */
+    printf_filtered ("Pending breakpoint \"%s\" resolved\n", b->addr_string);
+
+  do_cleanups (old_chain);
+  return rc;
+}
+
 /* Try to reenable any breakpoints in shared libraries.  */
 void
 re_enable_breakpoints_in_shlibs (void)
 {
-  struct breakpoint *b;
+  struct breakpoint *b, *tmp;
 
-  ALL_BREAKPOINTS (b)
+  ALL_BREAKPOINTS_SAFE (b, tmp)
+  {
     if (b->enable_state == bp_shlib_disabled)
-    {
-      char buf[1], *lib;
-
-      /* Do not reenable the breakpoint if the shared library
-         is still not mapped in.  */
-      lib = PC_SOLIB (b->loc->address);
-      if (lib != NULL && target_read_memory (b->loc->address, buf, 1) == 0)
-	b->enable_state = bp_enabled;
-    }
+      {
+	char buf[1], *lib;
+	
+	/* Do not reenable the breakpoint if the shared library
+	   is still not mapped in.  */
+	lib = PC_SOLIB (b->loc->address);
+	if (lib != NULL && target_read_memory (b->loc->address, buf, 1) == 0)
+	  b->enable_state = bp_enabled;
+      }
+    else if (b->pending && (b->enable_state == bp_enabled))
+      {
+	if (resolve_pending_breakpoint (b) == GDB_RC_OK)
+	  delete_breakpoint (b);
+      }
+  }
 }
 
 #endif
@@ -4723,14 +4819,21 @@ mention (struct breakpoint *b)
 
   if (say_where)
     {
-      if (addressprint || b->source_file == NULL)
+      if (b->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))
@@ -4743,6 +4846,11 @@ mention (struct breakpoint *b)
    SALS.sal[i] breakpoint, include the corresponding ADDR_STRING[i],
    COND[i] and COND_STRING[i] values.
 
+   The parameter PENDING_BP points to a pending breakpoint that is
+   the basis of the breakpoints currently being created.  The pending
+   breakpoint may contain a separate condition string or commands
+   that were added after the initial pending breakpoint was created.
+
    NOTE: If the function succeeds, the caller is expected to cleanup
    the arrays ADDR_STRING, COND_STRING, COND and SALS (but not the
    array contents).  If the function fails (error() is called), the
@@ -4753,7 +4861,8 @@ static void
 create_breakpoints (struct symtabs_and_lines sals, char **addr_string,
 		    struct expression **cond, char **cond_string,
 		    enum bptype type, enum bpdisp disposition,
-		    int thread, int ignore_count, int from_tty)
+		    int thread, int ignore_count, int from_tty,
+		    struct breakpoint *pending_bp)
 {
   if (type == bp_hardware_breakpoint)
     {
@@ -4793,6 +4902,26 @@ create_breakpoints (struct symtabs_and_l
 	b->ignore_count = ignore_count;
 	b->enable_state = bp_enabled;
 	b->disposition = disposition;
+	/* If resolving a pending breakpoint, a check must be made to see if
+	   the user has specified a new condition or commands for the 
+	   breakpoint.  A new condition will override any condition that was 
+	   initially specified with the initial breakpoint command.  */
+	if (pending_bp)
+	  {
+	    char *arg;
+	    if (pending_bp->cond_string)
+	      {
+		arg = pending_bp->cond_string;
+		b->cond_string = savestring (arg, strlen (arg));
+		b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
+		if (*arg)
+		  error ("Junk at end of pending breakpoint condition expression");
+	      }
+	    /* If there are commands associated with the breakpoint, they should 
+	       be copied too.  */
+	    if (pending_bp->commands)
+	      b->commands = copy_command_lines (pending_bp->commands);
+	  }
 	mention (b);
       }
   }    
@@ -4806,7 +4935,8 @@ create_breakpoints (struct symtabs_and_l
 static void
 parse_breakpoint_sals (char **address,
 		       struct symtabs_and_lines *sals,
-		       char ***addr_string)
+		       char ***addr_string,
+		       int *not_found_ptr)
 {
   char *addr_start = *address;
   *addr_string = NULL;
@@ -4847,9 +4977,11 @@ parse_breakpoint_sals (char **address,
  	      || ((strchr ("+-", (*address)[0]) != NULL)
  		  && ((*address)[1] != '['))))
 	*sals = decode_line_1 (address, 1, default_breakpoint_symtab,
-			       default_breakpoint_line, addr_string, NULL);
+			       default_breakpoint_line, addr_string, 
+			       not_found_ptr);
       else
-	*sals = decode_line_1 (address, 1, (struct symtab *) NULL, 0, addr_string, NULL);
+	*sals = decode_line_1 (address, 1, (struct symtab *) NULL, 0,
+		               addr_string, not_found_ptr);
     }
   /* For any SAL that didn't have a canonical string, fill one in. */
   if (sals->nelts > 0 && *addr_string == NULL)
@@ -4903,26 +5035,44 @@ breakpoint_sals_to_pc (struct symtabs_an
     }
 }
 
+static int
+do_captured_parse_breakpoint (struct ui_out *ui, void *data)
+{
+  struct captured_parse_breakpoint_args *args = data;
+  
+  parse_breakpoint_sals (args->arg_p, args->sals_p, args->addr_string_p, 
+		         args->not_found_ptr);
+
+  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. */
+   second bit : 0 normal breakpoint, 1 hardware breakpoint. 
 
-static void
-break_command_1 (char *arg, int flag, int from_tty)
+   PENDING_BP is non-NULL when this function is being called to resolve
+   a pending breakpoint.  */
+
+static int
+break_command_1 (char *arg, int flag, int from_tty, struct breakpoint *pending_bp)
 {
   int tempflag, hardwareflag;
   struct symtabs_and_lines sals;
   struct expression **cond = 0;
-  /* Pointers in arg to the start, and one past the end, of the
-     condition.  */
+  struct symtab_and_line pending_sal;
   char **cond_string = (char **) NULL;
+  char *copy_arg;
+  char *err_msg;
   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;
+  int not_found = 0;
 
   hardwareflag = flag & BP_HARDWAREFLAG;
   tempflag = flag & BP_TEMPFLAG;
@@ -4930,19 +5080,55 @@ 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)
-    return;
+  parse_args.arg_p = &arg;
+  parse_args.sals_p = &sals;
+  parse_args.addr_string_p = &addr_string;
+  parse_args.not_found_ptr = &not_found;
+
+  rc = catch_exceptions_with_msg (uiout, do_captured_parse_breakpoint, 
+		  		  &parse_args, NULL, &err_msg, 
+				  RETURN_MASK_ALL);
+
+  /* If caller is interested in rc value from parse, set value.  */
+
+  if (rc != GDB_RC_OK)
+    {
+      /* Check for file or function not found.  */
+      if (not_found)
+	{
+	  /* If called to resolve pending breakpoint, just return error code.  */
+	  if (pending_bp)
+	    return rc;
+
+	  error_output_message (NULL, err_msg);
+	  xfree (err_msg);
+	  if (!query ("Make breakpoint pending on future shared library load? "))
+	    return rc;
+	  copy_arg = xstrdup (addr_start);
+	  addr_string = &copy_arg;
+	  sals.nelts = 1;
+	  sals.sals = &pending_sal;
+	  pending_sal.pc = 0;
+	  pending = 1;
+	}
+      else
+	return rc;
+    }
+  else if (!sals.nelts)
+    return GDB_RC_FAIL;
 
   /* 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 *));
@@ -4969,62 +5155,94 @@ 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
      breakpoint. */
   thread = -1;			/* No specific thread yet */
-  for (i = 0; i < sals.nelts; i++)
+  if (!pending)
     {
-      char *tok = arg;
-      while (tok && *tok)
+      for (i = 0; i < sals.nelts; i++)
 	{
-	  char *end_tok;
-	  int toklen;
-	  char *cond_start = NULL;
-	  char *cond_end = NULL;
-	  while (*tok == ' ' || *tok == '\t')
-	    tok++;
-
-	  end_tok = tok;
-
-	  while (*end_tok != ' ' && *end_tok != '\t' && *end_tok != '\000')
-	    end_tok++;
-
-	  toklen = end_tok - tok;
-
-	  if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
-	    {
-	      tok = cond_start = end_tok + 1;
-	      cond[i] = parse_exp_1 (&tok, block_for_pc (sals.sals[i].pc), 0);
-	      make_cleanup (xfree, cond[i]);
-	      cond_end = tok;
-	      cond_string[i] = savestring (cond_start, cond_end - cond_start);
-	      make_cleanup (xfree, cond_string[i]);
-	    }
-	  else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
+	  char *tok = arg;
+	  while (tok && *tok)
 	    {
-	      char *tmptok;
-
-	      tok = end_tok + 1;
-	      tmptok = tok;
-	      thread = strtol (tok, &tok, 0);
-	      if (tok == tmptok)
-		error ("Junk after thread keyword.");
-	      if (!valid_thread_id (thread))
-		error ("Unknown thread %d\n", thread);
+	      char *end_tok;
+	      int toklen;
+	      char *cond_start = NULL;
+	      char *cond_end = NULL;
+	      while (*tok == ' ' || *tok == '\t')
+		tok++;
+	      
+	      end_tok = tok;
+	      
+	      while (*end_tok != ' ' && *end_tok != '\t' && *end_tok != '\000')
+		end_tok++;
+	      
+	      toklen = end_tok - tok;
+	      
+	      if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
+		{
+		  tok = cond_start = end_tok + 1;
+		  cond[i] = parse_exp_1 (&tok, block_for_pc (sals.sals[i].pc), 
+				         0);
+		  make_cleanup (xfree, cond[i]);
+		  cond_end = tok;
+		  cond_string[i] = savestring (cond_start, 
+				               cond_end - cond_start);
+		  make_cleanup (xfree, cond_string[i]);
+		}
+	      else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
+		{
+		  char *tmptok;
+		  
+		  tok = end_tok + 1;
+		  tmptok = tok;
+		  thread = strtol (tok, &tok, 0);
+		  if (tok == tmptok)
+		    error ("Junk after thread keyword.");
+		  if (!valid_thread_id (thread))
+		    error ("Unknown thread %d\n", thread);
+		}
+	      else
+		error ("Junk at end of arguments.");
 	    }
-	  else
-	    error ("Junk at end of arguments.");
 	}
+      create_breakpoints (sals, addr_string, cond, cond_string,
+			  hardwareflag ? bp_hardware_breakpoint 
+			  : bp_breakpoint,
+			  tempflag ? disp_del : disp_donttouch,
+			  thread, ignore_count, from_tty,
+			  pending_bp);
     }
+  else
+    {
+      struct symtab_and_line sal;
+      struct breakpoint *b;
 
-  create_breakpoints (sals, addr_string, cond, cond_string,
-		      hardwareflag ? bp_hardware_breakpoint : bp_breakpoint,
-		      tempflag ? disp_del : disp_donttouch,
-		      thread, ignore_count, from_tty);
+      sal.symtab = NULL;
+      sal.pc = 0;
 
+      make_cleanup (xfree, copy_arg);
+
+      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->pending = 1;
+      b->disposition = tempflag ? disp_del : disp_donttouch;
+      b->from_tty = from_tty;
+      b->flag = flag;
+      mention (b);
+    }
+  
   if (sals.nelts > 1)
     {
       warning ("Multiple breakpoints were set.");
@@ -5035,6 +5253,8 @@ break_command_1 (char *arg, int flag, in
   discard_cleanups (breakpoint_chain);
   /* But cleanup everything else. */
   do_cleanups (old_chain);
+
+  return GDB_RC_OK;
 }
 
 /* Set a breakpoint of TYPE/DISPOSITION according to ARG (function,
@@ -5071,7 +5291,7 @@ do_captured_breakpoint (void *data)
   sals.nelts = 0;
   address_end = args->address;
   addr_string = NULL;
-  parse_breakpoint_sals (&address_end, &sals, &addr_string);
+  parse_breakpoint_sals (&address_end, &sals, &addr_string, 0);
 
   if (!sals.nelts)
     return GDB_RC_NONE;
@@ -5135,7 +5355,8 @@ do_captured_breakpoint (void *data)
   create_breakpoints (sals, addr_string, cond, cond_string,
 		      args->hardwareflag ? bp_hardware_breakpoint : bp_breakpoint,
 		      args->tempflag ? disp_del : disp_donttouch,
-		      args->thread, args->ignore_count, 0/*from-tty*/);
+		      args->thread, args->ignore_count, 0/*from-tty*/, 
+		      NULL/*pending_bp*/);
 
   /* That's it. Discard the cleanups for data inserted into the
      breakpoint. */
@@ -5227,7 +5448,7 @@ break_at_finish_at_depth_command_1 (char
 	    addr_string = xstrprintf ("*0x%s %s", paddr_nz (high), extra_args);
 	  else
 	    addr_string = xstrprintf ("*0x%s", paddr_nz (high));
-	  break_command_1 (addr_string, flag, from_tty);
+	  break_command_1 (addr_string, flag, from_tty, NULL);
 	  xfree (addr_string);
 	}
       else
@@ -5310,7 +5531,7 @@ break_at_finish_command_1 (char *arg, in
 				       extra_args);
 	  else
 	    break_string = xstrprintf ("*0x%s", paddr_nz (high));
-	  break_command_1 (break_string, flag, from_tty);
+	  break_command_1 (break_string, flag, from_tty, NULL);
 	  xfree (break_string);
 	}
       else
@@ -5377,7 +5598,7 @@ resolve_sal_pc (struct symtab_and_line *
 void
 break_command (char *arg, int from_tty)
 {
-  break_command_1 (arg, 0, from_tty);
+  break_command_1 (arg, 0, from_tty, NULL);
 }
 
 void
@@ -5395,7 +5616,7 @@ break_at_finish_at_depth_command (char *
 void
 tbreak_command (char *arg, int from_tty)
 {
-  break_command_1 (arg, BP_TEMPFLAG, from_tty);
+  break_command_1 (arg, BP_TEMPFLAG, from_tty, NULL);
 }
 
 void
@@ -5407,13 +5628,13 @@ tbreak_at_finish_command (char *arg, int
 static void
 hbreak_command (char *arg, int from_tty)
 {
-  break_command_1 (arg, BP_HARDWAREFLAG, from_tty);
+  break_command_1 (arg, BP_HARDWAREFLAG, from_tty, NULL);
 }
 
 static void
 thbreak_command (char *arg, int from_tty)
 {
-  break_command_1 (arg, (BP_TEMPFLAG | BP_HARDWAREFLAG), from_tty);
+  break_command_1 (arg, (BP_TEMPFLAG | BP_HARDWAREFLAG), from_tty, NULL);
 }
 
 static void
@@ -5454,7 +5675,7 @@ stopin_command (char *arg, int from_tty)
   if (badInput)
     printf_filtered ("Usage: stop in <function | address>\n");
   else
-    break_command_1 (arg, 0, from_tty);
+    break_command_1 (arg, 0, from_tty, NULL);
 }
 
 static void
@@ -5486,7 +5707,7 @@ stopat_command (char *arg, int from_tty)
   if (badInput)
     printf_filtered ("Usage: stop at <line>\n");
   else
-    break_command_1 (arg, 0, from_tty);
+    break_command_1 (arg, 0, from_tty, NULL);
 }
 
 /* accessflag:  hw_write:  watch write, 
@@ -6693,6 +6914,7 @@ delete_breakpoint (struct breakpoint *bp
 	    && !b->loc->duplicate
 	    && b->enable_state != bp_disabled
 	    && b->enable_state != bp_shlib_disabled
+	    && !b->pending
 	    && b->enable_state != bp_call_disabled)
 	{
 	  int val;
@@ -6898,6 +7120,10 @@ 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->pending)
+	break;
+
       save_enable = b->enable_state;
       if (b->enable_state != bp_shlib_disabled)
         b->enable_state = bp_disabled;
@@ -7313,70 +7539,91 @@ do_enable_breakpoint (struct breakpoint 
 	error ("Hardware breakpoints used exceeds limit.");
     }
 
-  if (bpt->enable_state != bp_permanent)
-    bpt->enable_state = bp_enabled;
-  bpt->disposition = disposition;
-  check_duplicates (bpt);
-  breakpoints_changed ();
-
-  if (bpt->type == bp_watchpoint || 
-      bpt->type == bp_hardware_watchpoint ||
-      bpt->type == bp_read_watchpoint || 
-      bpt->type == bp_access_watchpoint)
-    {
-      if (bpt->exp_valid_block != NULL)
-	{
-	  struct frame_info *fr =
-	  fr = frame_find_by_id (bpt->watchpoint_frame);
-	  if (fr == NULL)
+  if (bpt->pending)
+    {
+      if (bpt->enable_state != bp_enabled)
+	{
+	  /* When enabling a pending breakpoint, we need to check if the breakpoint
+	     is resolvable since shared libraries could have been loaded
+	     after the breakpoint was disabled.  */
+	  breakpoints_changed ();
+ 	  if (resolve_pending_breakpoint (bpt) == GDB_RC_OK)
 	    {
-	      printf_filtered ("\
-Cannot enable watchpoint %d because the block in which its expression\n\
-is valid is not currently in scope.\n", bpt->number);
-	      bpt->enable_state = bp_disabled;
+	      delete_breakpoint (bpt);
 	      return;
 	    }
-
-	  save_selected_frame = deprecated_selected_frame;
-	  save_selected_frame_level = frame_relative_level (deprecated_selected_frame);
-	  select_frame (fr);
+	  bpt->enable_state = bp_enabled;
+	  bpt->disposition = disposition;
 	}
-
-      value_free (bpt->val);
-      mark = value_mark ();
-      bpt->val = evaluate_expression (bpt->exp);
-      release_value (bpt->val);
-      if (VALUE_LAZY (bpt->val))
-	value_fetch_lazy (bpt->val);
-
-      if (bpt->type == bp_hardware_watchpoint ||
-	  bpt->type == bp_read_watchpoint ||
+    }
+  else  /* Not a pending breakpoint.  */
+    {
+      if (bpt->enable_state != bp_permanent)
+	bpt->enable_state = bp_enabled;
+      bpt->disposition = disposition;
+      check_duplicates (bpt);
+      breakpoints_changed ();
+      
+      if (bpt->type == bp_watchpoint || 
+	  bpt->type == bp_hardware_watchpoint ||
+	  bpt->type == bp_read_watchpoint || 
 	  bpt->type == bp_access_watchpoint)
 	{
-	  int i = hw_watchpoint_used_count (bpt->type, &other_type_used);
-	  int mem_cnt = can_use_hardware_watchpoint (bpt->val);
-
-	  /* Hack around 'unused var' error for some targets here */
-	  (void) mem_cnt, i;
-	  target_resources_ok = TARGET_CAN_USE_HARDWARE_WATCHPOINT (
-				   bpt->type, i + mem_cnt, other_type_used);
-	  /* we can consider of type is bp_hardware_watchpoint, convert to 
-	     bp_watchpoint in the following condition */
-	  if (target_resources_ok < 0)
+	  if (bpt->exp_valid_block != NULL)
 	    {
-	      printf_filtered ("\
+	      struct frame_info *fr =
+		fr = frame_find_by_id (bpt->watchpoint_frame);
+	      if (fr == NULL)
+		{
+		  printf_filtered ("\
+Cannot enable watchpoint %d because the block in which its expression\n\
+is valid is not currently in scope.\n", bpt->number);
+		  bpt->enable_state = bp_disabled;
+		  return;
+		}
+	      
+	      save_selected_frame = deprecated_selected_frame;
+	      save_selected_frame_level = frame_relative_level (deprecated_selected_frame);
+	      select_frame (fr);
+	    }
+	  
+	  value_free (bpt->val);
+	  mark = value_mark ();
+	  bpt->val = evaluate_expression (bpt->exp);
+	  release_value (bpt->val);
+	  if (VALUE_LAZY (bpt->val))
+	    value_fetch_lazy (bpt->val);
+	  
+	  if (bpt->type == bp_hardware_watchpoint ||
+	      bpt->type == bp_read_watchpoint ||
+	      bpt->type == bp_access_watchpoint)
+	    {
+	      int i = hw_watchpoint_used_count (bpt->type, &other_type_used);
+	      int mem_cnt = can_use_hardware_watchpoint (bpt->val);
+	      
+	      /* Hack around 'unused var' error for some targets here */
+	      (void) mem_cnt, i;
+	      target_resources_ok = TARGET_CAN_USE_HARDWARE_WATCHPOINT (
+									bpt->type, i + mem_cnt, other_type_used);
+	      /* we can consider of type is bp_hardware_watchpoint, convert to 
+		 bp_watchpoint in the following condition */
+	      if (target_resources_ok < 0)
+		{
+		  printf_filtered ("\
 Cannot enable watchpoint %d because target watch resources\n\
 have been allocated for other watchpoints.\n", bpt->number);
-	      bpt->enable_state = bp_disabled;
-	      value_free_to_mark (mark);
-	      return;
+		  bpt->enable_state = bp_disabled;
+		  value_free_to_mark (mark);
+		  return;
+		}
 	    }
+	  
+	  if (save_selected_frame_level >= 0)
+	    select_frame (save_selected_frame);
+	  value_free_to_mark (mark);
 	}
-
-      if (save_selected_frame_level >= 0)
-	select_frame (save_selected_frame);
-      value_free_to_mark (mark);
     }
+
   if (modify_breakpoint_hook)
     modify_breakpoint_hook (bpt);
   breakpoint_modify_event (bpt->number);

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

* Re: [RFA]: pending breakpoint support  [1/3]
  2004-02-02 21:12                 ` Jeff Johnston
@ 2004-02-02 21:22                   ` Daniel Jacobowitz
  2004-02-02 22:18                     ` Jeff Johnston
  0 siblings, 1 reply; 17+ messages in thread
From: Daniel Jacobowitz @ 2004-02-02 21:22 UTC (permalink / raw)
  To: Jeff Johnston; +Cc: gdb-patches

On Mon, Feb 02, 2004 at 04:12:17PM -0500, Jeff Johnston wrote:
> Patch checked in.  I have included the final patch here with the 
> reworded message.

I suspect this deserves a NEWS entry.  Mind taking care of that?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

* Re: [RFA]: pending breakpoint support  [1/3]
  2004-02-02 21:22                   ` Daniel Jacobowitz
@ 2004-02-02 22:18                     ` Jeff Johnston
  2004-02-02 22:21                       ` Daniel Jacobowitz
  2004-02-03  6:07                       ` Eli Zaretskii
  0 siblings, 2 replies; 17+ messages in thread
From: Jeff Johnston @ 2004-02-02 22:18 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

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

Daniel Jacobowitz wrote:

>On Mon, Feb 02, 2004 at 04:12:17PM -0500, Jeff Johnston wrote:
>  
>
>>Patch checked in.  I have included the final patch here with the 
>>reworded message.
>>    
>>
>
>I suspect this deserves a NEWS entry.  Mind taking care of that?
>  
>
Ok, how about the accompanying patch?

-- Jeff J.

2004-02-02  Jeff Johnston  <jjohnstn@redhat.com>

        * NEWS: Add information about new pending breakpoint support.



[-- Attachment #2: news.patch --]
[-- Type: text/plain, Size: 859 bytes --]

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.134
diff -u -r1.134 NEWS
--- NEWS	22 Jan 2004 23:18:03 -0000	1.134
+++ NEWS	2 Feb 2004 22:16:45 -0000
@@ -3,6 +3,15 @@
 
 *** Changes since GDB 6.0:
 
+* Pending breakpoint support
+
+Support has been added to allow specifying breakpoints in shared libraries
+that have not yet been loaded.  If a breakpoint location cannot be found,
+you are queried to see if you wish to make the breakpoint pending on a
+future shared-library load.  If and when the breakpoint symbol is resolved, 
+the pending breakpoint is removed as one or more regular breakpoints are
+created.  Pending breakpoints are very useful for gcj java debugging.
+
 * Removed --with-mmalloc
 
 Support for the mmalloc memory manager has been removed, as it

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

* Re: [RFA]: pending breakpoint support  [1/3]
  2004-02-02 22:18                     ` Jeff Johnston
@ 2004-02-02 22:21                       ` Daniel Jacobowitz
  2004-02-03  6:07                       ` Eli Zaretskii
  1 sibling, 0 replies; 17+ messages in thread
From: Daniel Jacobowitz @ 2004-02-02 22:21 UTC (permalink / raw)
  To: Jeff Johnston; +Cc: gdb-patches

On Mon, Feb 02, 2004 at 05:18:26PM -0500, Jeff Johnston wrote:
> Daniel Jacobowitz wrote:
> 
> >On Mon, Feb 02, 2004 at 04:12:17PM -0500, Jeff Johnston wrote:
> > 
> >
> >>Patch checked in.  I have included the final patch here with the 
> >>reworded message.
> >>   
> >>
> >
> >I suspect this deserves a NEWS entry.  Mind taking care of that?
> > 
> >
> Ok, how about the accompanying patch?

OK.  And here I had no idea you were working on gcj support...

> 
> -- Jeff J.
> 
> 2004-02-02  Jeff Johnston  <jjohnstn@redhat.com>
> 
>        * NEWS: Add information about new pending breakpoint support.
> 
> 

> Index: NEWS
> ===================================================================
> RCS file: /cvs/src/src/gdb/NEWS,v
> retrieving revision 1.134
> diff -u -r1.134 NEWS
> --- NEWS	22 Jan 2004 23:18:03 -0000	1.134
> +++ NEWS	2 Feb 2004 22:16:45 -0000
> @@ -3,6 +3,15 @@
>  
>  *** Changes since GDB 6.0:
>  
> +* Pending breakpoint support
> +
> +Support has been added to allow specifying breakpoints in shared libraries
> +that have not yet been loaded.  If a breakpoint location cannot be found,
> +you are queried to see if you wish to make the breakpoint pending on a
> +future shared-library load.  If and when the breakpoint symbol is resolved, 
> +the pending breakpoint is removed as one or more regular breakpoints are
> +created.  Pending breakpoints are very useful for gcj java debugging.
> +
>  * Removed --with-mmalloc
>  
>  Support for the mmalloc memory manager has been removed, as it


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

* Re: [RFA]: pending breakpoint support  [1/3]
  2004-02-02 22:18                     ` Jeff Johnston
  2004-02-02 22:21                       ` Daniel Jacobowitz
@ 2004-02-03  6:07                       ` Eli Zaretskii
  2004-02-05 21:33                         ` Jeff Johnston
  1 sibling, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2004-02-03  6:07 UTC (permalink / raw)
  To: Jeff Johnston; +Cc: drow, gdb-patches

> Date: Mon, 02 Feb 2004 17:18:26 -0500
> From: Jeff Johnston <jjohnstn@redhat.com>
> >
> Ok, how about the accompanying patch?
> 
> -- Jeff J.
> 
> 2004-02-02  Jeff Johnston  <jjohnstn@redhat.com>
> 
>         * NEWS: Add information about new pending breakpoint support.

Thanks.  Is it possible to modify the wording so that it doesn't use
passive tense so much?  I'm told by native English speakers that
passive is BAD...

For example, this:

> If a breakpoint location cannot be found, you are queried to see if
> you wish to make the breakpoint pending on a future shared-library
> load.

could be rephrased like this:

  If a breakpoint location cannot be found, GDB queries you if you
  wish to make the breakpoint pending on ...

Etc., you get the point.

TIA


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

* Re: [RFA]: pending breakpoint support  [1/3]
  2004-02-03  6:07                       ` Eli Zaretskii
@ 2004-02-05 21:33                         ` Jeff Johnston
  2004-02-06 20:17                           ` Frank Ch. Eigler
  0 siblings, 1 reply; 17+ messages in thread
From: Jeff Johnston @ 2004-02-05 21:33 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: drow, gdb-patches

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

Eli Zaretskii wrote:
>>Date: Mon, 02 Feb 2004 17:18:26 -0500
>>From: Jeff Johnston <jjohnstn@redhat.com>
>>
>>Ok, how about the accompanying patch?
>>
>>-- Jeff J.
>>
>>2004-02-02  Jeff Johnston  <jjohnstn@redhat.com>
>>
>>        * NEWS: Add information about new pending breakpoint support.
> 
> 
> Thanks.  Is it possible to modify the wording so that it doesn't use
> passive tense so much?  I'm told by native English speakers that
> passive is BAD...
> 
> For example, this:
> 
> 
>>If a breakpoint location cannot be found, you are queried to see if
>>you wish to make the breakpoint pending on a future shared-library
>>load.
> 
> 
> could be rephrased like this:
> 
>   If a breakpoint location cannot be found, GDB queries you if you
>   wish to make the breakpoint pending on ...
> 
> Etc., you get the point.
> 
> TIA
> 
>

Is the attached ok to commit?


[-- Attachment #2: news.patch --]
[-- Type: text/plain, Size: 852 bytes --]

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.135
diff -u -r1.135 NEWS
--- NEWS	5 Feb 2004 19:56:33 -0000	1.135
+++ NEWS	5 Feb 2004 21:31:35 -0000
@@ -3,6 +3,15 @@
 
 *** Changes since GDB 6.0:
 
+* Pending breakpoint support
+
+Support has been added to allow specifying breakpoints in shared libraries
+that have not yet been loaded.  If a breakpoint location cannot be found,
+GDB queries you if you wish to make the breakpoint pending on a
+future shared-library load.  If and when GDB resolves the breakpoint symbol, 
+the pending breakpoint is removed as one or more regular breakpoints are
+created.  Pending breakpoints are very useful for gcj java debugging.
+
 * Removed --with-mmalloc
 
 Support for the mmalloc memory manager has been removed, as it

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

* Re: [RFA]: pending breakpoint support  [1/3]
  2004-02-05 21:33                         ` Jeff Johnston
@ 2004-02-06 20:17                           ` Frank Ch. Eigler
  2004-02-07 15:13                             ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Frank Ch. Eigler @ 2004-02-06 20:17 UTC (permalink / raw)
  To: Jeff Johnston; +Cc: gdb-patches


jjohnstn wrote:

> +* Pending breakpoint support
> +
> +Support has been added to allow specifying breakpoints in shared libraries
> +that have not yet been loaded.  If a breakpoint location cannot be found,
> +GDB queries you if you wish to make the breakpoint pending on a
> +future shared-library load.  If and when GDB resolves the breakpoint symbol, 
> +the pending breakpoint is removed as one or more regular breakpoints are
> +created.  Pending breakpoints are very useful for gcj java debugging.

If I were to word this, it'd be something like:

GDB now supports specifying breakpoints in shared libraries that have
not yet been loaded.  If GDB cannot find a breakpoint location the
user specifies, it prompts for whether the breakpoint should be
deferred, pending a future shared library load.  If and when GDB
resolves the symbol, it replaces the pending breakpoint with a regular
breakpoint.  This feature is very useful for gcj java and sid debugging.

(Does it undo this pending->regular mapping when the shlib is unloaded?)
(Oops, somehow that sid comment slipped in. :-) 

- FChE


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

* Re: [RFA]: pending breakpoint support  [1/3]
  2004-02-06 20:17                           ` Frank Ch. Eigler
@ 2004-02-07 15:13                             ` Eli Zaretskii
  0 siblings, 0 replies; 17+ messages in thread
From: Eli Zaretskii @ 2004-02-07 15:13 UTC (permalink / raw)
  To: Frank Ch. Eigler; +Cc: jjohnstn, gdb-patches

> From: fche@redhat.com (Frank Ch. Eigler)
> Date: 06 Feb 2004 15:17:30 -0500
> 
> jjohnstn wrote:
> 
> > +* Pending breakpoint support
> > +
> > +Support has been added to allow specifying breakpoints in shared libraries
> > +that have not yet been loaded.  If a breakpoint location cannot be found,
> > +GDB queries you if you wish to make the breakpoint pending on a
> > +future shared-library load.  If and when GDB resolves the breakpoint symbol, 
> > +the pending breakpoint is removed as one or more regular breakpoints are
> > +created.  Pending breakpoints are very useful for gcj java debugging.
> 
> If I were to word this, it'd be something like:
> 
> GDB now supports specifying breakpoints in shared libraries that have
> not yet been loaded.  If GDB cannot find a breakpoint location the
> user specifies, it prompts for whether the breakpoint should be
> deferred, pending a future shared library load.  If and when GDB
> resolves the symbol, it replaces the pending breakpoint with a regular
> breakpoint.  This feature is very useful for gcj java and sid debugging.

Frank, I like your wording better.  Thanks.


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

end of thread, other threads:[~2004-02-07 15:13 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-01-21 20:52 [RFA]: pending breakpoint support [1/3] Jeff Johnston
2004-01-22 22:27 ` Daniel Jacobowitz
2004-01-27 20:41   ` J. Johnston
2004-01-30  4:13     ` Daniel Jacobowitz
2004-01-30 18:51       ` Jeff Johnston
2004-01-30 19:09         ` Daniel Jacobowitz
2004-01-30 22:46           ` Jeff Johnston
2004-01-30 23:45             ` Daniel Jacobowitz
2004-01-31  0:33               ` Jeff Johnston
2004-02-02 21:12                 ` Jeff Johnston
2004-02-02 21:22                   ` Daniel Jacobowitz
2004-02-02 22:18                     ` Jeff Johnston
2004-02-02 22:21                       ` Daniel Jacobowitz
2004-02-03  6:07                       ` Eli Zaretskii
2004-02-05 21:33                         ` Jeff Johnston
2004-02-06 20:17                           ` Frank Ch. Eigler
2004-02-07 15:13                             ` Eli Zaretskii

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