[patch/MI] Implementation for break-catch command

[patch/MI] Implementation for break-catch command

[Aleksandar Ristovski]

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

Hello,

The attached diff implements -break-catch command for MI protocol (includes the documentation changes).

Thanks,

Aleksandar Ristovski
QNX Software Systems

2008-05-28  Aleksandar Ristovski  <aristovski@qnx.com>

	* breakpoint.c (catch_command_1): Remove static declaration.
	(print_mention_exception_catchpoint): Output information only
	when non-MI protocol is active.
	* breakpoint.h (catch_command_1): New declaration.
	* doc/gdb.texinfo (break-catch): Remove @ignore and add text for
	break-catch command.
	* mi/mi-cmd-break.c (mi_cmd_break_catch): New function.
	* mi/mi-cmds.c (build_table): Add function for break-catch.
	* mi/mi-cmds.h (mi_cmd_break_catch): New declaration.

[-- Attachment #2: break-catch-200805281107.diff --]
[-- Type: text/plain, Size: 8186 bytes --]

Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.322
diff -u -p -r1.322 breakpoint.c
--- gdb/breakpoint.c	28 May 2008 14:04:21 -0000	1.322
+++ gdb/breakpoint.c	28 May 2008 15:06:11 -0000
@@ -67,8 +67,6 @@
 static void until_break_command_continuation (struct continuation_arg *arg, 
 					      int error);
 
-static void catch_command_1 (char *, int, int);
-
 static void enable_delete_command (char *, int);
 
 static void enable_delete_breakpoint (struct breakpoint *);
@@ -6549,13 +6547,16 @@ print_mention_exception_catchpoint (stru
   int bp_temp;
   int bp_throw;
 
-  bp_temp = b->loc->owner->disposition == disp_del;
-  bp_throw = strstr (b->addr_string, "throw") != NULL;
-  ui_out_text (uiout, bp_temp ? _("Temporary catchpoint ")
-			      : _("Catchpoint "));
-  ui_out_field_int (uiout, "bkptno", b->number);
-  ui_out_text (uiout, bp_throw ? _(" (throw)")
-			       : _(" (catch)"));
+  if (!ui_out_is_mi_like_p (uiout))
+    {
+      bp_temp = b->loc->owner->disposition == disp_del;
+      bp_throw = strstr (b->addr_string, "throw") != NULL;
+      ui_out_text (uiout, bp_temp ? _("Temporary catchpoint ")
+				  : _("Catchpoint "));
+      ui_out_field_int (uiout, "bkptno", b->number);
+      ui_out_text (uiout, bp_throw ? _(" (throw)")
+				   : _(" (catch)"));
+    }
 }
 
 static struct breakpoint_ops gnu_v3_exception_catchpoint_ops = {
@@ -6691,7 +6692,7 @@ catch_assert_command (char *arg, int tem
                                    tempflag, from_tty);
 }
 
-static void
+void
 catch_command_1 (char *arg, int tempflag, int from_tty)
 {
 
Index: gdb/breakpoint.h
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.h,v
retrieving revision 1.72
diff -u -p -r1.72 breakpoint.h
--- gdb/breakpoint.h	4 May 2008 19:38:59 -0000	1.72
+++ gdb/breakpoint.h	28 May 2008 15:06:12 -0000
@@ -865,4 +865,6 @@ void breakpoint_restore_shadows (gdb_byt
 
 extern int breakpoints_always_inserted_mode (void);
 
+extern void catch_command_1 (char *arg, int tempflag, int from_tty);
+
 #endif /* !defined (BREAKPOINT_H) */
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.498
diff -u -p -r1.498 gdb.texinfo
--- gdb/doc/gdb.texinfo	22 May 2008 21:21:41 -0000	1.498
+++ gdb/doc/gdb.texinfo	28 May 2008 15:06:23 -0000
@@ -18486,10 +18486,50 @@ line="5",times="0",ignore="3"@}]@}
 (gdb)
 @end smallexample
 
-@ignore
 @subheading The @code{-break-catch} Command
 @findex -break-catch
 
+@subsubheading Synopsis
+
+@smallexample
+ -break-catch [ -t ] [ @var{event} ]
+@end smallexample
+
+@noindent
+Where @var{event} can be one of:
+
+@itemize @bullet
+@item catch
+@item throw
+@end itemize
+
+The possible optional parameters of this command are:
+
+@table @samp
+@item -t
+Insert a temporary catchpoint.
+@end table
+
+@subsubheading Result
+
+The result is in the form:
+
+@smallexample
+^done,bkpt=@{number="@var{number}",type="@var{type}",disp="del"|"keep",
+enabled="y"|"n",addr="@var{hex}",what="exception catch"|"exception throw",
+times="@var{times}",original-location="@var{funcname}"@}
+@end smallexample
+
+@noindent
+where @var{number} is the @value{GDBN} number for this breakpoint,
+@var{funcname} is the name of the function where the catchpoint was
+inserted (target specific) and @var{times} the number of times that 
+the breakpoint has been hit (always 0 for -break-catch but may be 
+greater for -break-info or -break-list which use the same output).
+
+Note: this format is open to change.
+
+@ignore
 @subheading The @code{-break-commands} Command
 @findex -break-commands
 @end ignore
Index: gdb/mi/mi-cmd-break.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-break.c,v
retrieving revision 1.19
diff -u -p -r1.19 mi-cmd-break.c
--- gdb/mi/mi-cmd-break.c	1 Feb 2008 16:24:46 -0000	1.19
+++ gdb/mi/mi-cmd-break.c	28 May 2008 15:06:23 -0000
@@ -241,3 +241,106 @@ mi_cmd_break_watch (char *command, char 
     }
   return MI_CMD_DONE;
 }
+
+/* Implements the -break-catch command.
+   See the MI manual for the list of possible options.  */
+
+enum mi_cmd_result
+mi_cmd_break_catch (char *command, char **argv, int argc)
+{
+  char *event_name = NULL;
+  enum bp_type type = REG_BP;
+  int temp_p = 0;
+  int thread = -1;
+  int ignore_count = 0;
+  char *condition = NULL;
+  int pending = 0;
+  struct gdb_exception e;
+  struct gdb_events *old_hooks;
+  enum opt
+    {
+      HARDWARE_OPT, TEMP_OPT /*, REGEXP_OPT */ , CONDITION_OPT,
+      IGNORE_COUNT_OPT, THREAD_OPT, PENDING_OPT
+    };
+  static struct mi_opt opts[] =
+  {
+    {"h", HARDWARE_OPT, 0},
+    {"t", TEMP_OPT, 0},
+    {"c", CONDITION_OPT, 1},
+    {"i", IGNORE_COUNT_OPT, 1},
+    {"p", THREAD_OPT, 1},
+    {"f", PENDING_OPT, 0},
+    { 0, 0, 0 }
+  };
+
+  /* Parse arguments. It could be -r or -h or -t, <location> or ``--''
+     to denote the end of the option list. */
+  int optind = 0;
+  char *optarg;
+  while (1)
+    {
+      int opt = mi_getopt ("mi_cmd_break_catch", argc, argv, opts, &optind, &optarg);
+      if (opt < 0)
+	break;
+      switch ((enum opt) opt)
+	{
+	case TEMP_OPT:
+	  temp_p = 1;
+	  break;
+	case HARDWARE_OPT:
+//	  type = HW_BP;
+	  warning (_("Hardware flag ignored for catchpoints"));
+	  break;
+#if 0
+	case REGEXP_OPT:
+	  type = REGEXP_BP;
+	  break;
+#endif
+	case CONDITION_OPT:
+	  condition = optarg;
+	  warning (_("Condition not yet implemented for catchpoints"));
+	  break;
+	case IGNORE_COUNT_OPT:
+	  ignore_count = atol (optarg);
+	  warning (_("Ignore count not yet implemented for catchpoints"));
+	  break;
+	case THREAD_OPT:
+	  thread = atol (optarg);
+	  warning (_("Thread option not yet implemented for catchpoints"));
+	  break;
+	case PENDING_OPT:
+	  pending = 1;
+	  warning (_("Pending flag ignored for catchpoints"));
+	  break;
+	}
+    }
+
+  if (optind >= argc)
+    error (_("mi_cmd_break_catch: Missing <event name>"));
+  if (optind < argc - 1)
+    error (_("mi_cmd_break_catch: Garbage following <event name>"));
+  event_name = argv[optind];
+
+  /* Now we have what we need, let's insert the breakpoint! */
+  old_hooks = deprecated_set_gdb_event_hooks (&breakpoint_hooks);
+  /* Make sure we restore hooks even if exception is thrown.  */
+  TRY_CATCH (e, RETURN_MASK_ALL)
+    {
+      switch (type)
+	{
+	case REG_BP:
+	  catch_command_1 (event_name, temp_p, 0);
+	  break;
+      default:
+	  internal_error (__FILE__, __LINE__,
+			  _("mi_cmd_break_catch: Bad switch."));
+	}
+    }
+  deprecated_set_gdb_event_hooks (old_hooks);
+  if (e.reason < 0)
+    throw_exception (e);
+
+  return MI_CMD_DONE;
+}
+
+
Index: gdb/mi/mi-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.c,v
retrieving revision 1.32
diff -u -p -r1.32 mi-cmds.c
--- gdb/mi/mi-cmds.c	24 Apr 2008 13:28:06 -0000	1.32
+++ gdb/mi/mi-cmds.c	28 May 2008 15:06:23 -0000
@@ -33,7 +33,7 @@ static void build_table (struct mi_cmd *
 struct mi_cmd mi_cmds[] =
 {
   { "break-after", { "ignore", 1 }, NULL },
-  { "break-catch", { NULL, 0 }, NULL },
+  { "break-catch", { NULL, 0 }, mi_cmd_break_catch},
   { "break-commands", { NULL, 0 }, NULL },
   { "break-condition", { "cond", 1 }, NULL },
   { "break-delete", { "delete breakpoint", 1 }, NULL },
Index: gdb/mi/mi-cmds.h
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.h,v
retrieving revision 1.30
diff -u -p -r1.30 mi-cmds.h
--- gdb/mi/mi-cmds.h	24 Apr 2008 13:28:06 -0000	1.30
+++ gdb/mi/mi-cmds.h	28 May 2008 15:06:23 -0000
@@ -54,6 +54,7 @@ typedef enum mi_cmd_result (mi_cmd_argv_
 /* Function implementing each command */
 extern mi_cmd_argv_ftype mi_cmd_break_insert;
 extern mi_cmd_argv_ftype mi_cmd_break_watch;
+extern mi_cmd_argv_ftype mi_cmd_break_catch;
 extern mi_cmd_argv_ftype mi_cmd_disassemble;
 extern mi_cmd_argv_ftype mi_cmd_data_evaluate_expression;
 extern mi_cmd_argv_ftype mi_cmd_data_list_register_names;

Re: [patch/MI] Implementation for break-catch command

[Vladimir Prus]

Aleksandar Ristovski wrote:

> Hello,
> 
> The attached diff implements -break-catch command for MI protocol (includes the documentation
> changes).
> 
> Thanks,
> 
> Aleksandar Ristovski
> QNX Software Systems
> 
> 2008-05-28  Aleksandar Ristovski  <aristovski@qnx.com>
> 
> * breakpoint.c (catch_command_1): Remove static declaration.
> (print_mention_exception_catchpoint): Output information only
> when non-MI protocol is active.
> * breakpoint.h (catch_command_1): New declaration.
> * doc/gdb.texinfo (break-catch): Remove @ignore and add text for
> break-catch command.
> * mi/mi-cmd-break.c (mi_cmd_break_catch): New function.

This one accepts and parses a lot of options, and then ignores them. Some
options, like -f, will never make any sense for -break-catch. Can you
please only leave necessary code there?

Also note that the condition is actually meaningful for catchpoints
and catch_exception_command_1/handle_gnu_v3_exceptions actually
tries to set condition. Should setting a condition via MI work too?

> * mi/mi-cmds.c (build_table): Add function for break-catch.

It's not build_table, it's mi_cmds. "diff -p" sometimes lies.

- Volodya

Re: [patch/MI] Implementation for break-catch command

[Aleksandar Ristovski]

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

Vladimir Prus wrote:
>> * mi/mi-cmd-break.c (mi_cmd_break_catch): New function.
> 
> This one accepts and parses a lot of options, and then ignores them. Some
> options, like -f, will never make any sense for -break-catch. Can you
> please only leave necessary code there?

Yes, I wasn't sure what to do about them. -f doesn't apply, but -p for example should apply but has not been implemented (yet). I put a warning for now for each valid unimplemented option.

> 
> Also note that the condition is actually meaningful for catchpoints
> and catch_exception_command_1/handle_gnu_v3_exceptions actually
> tries to set condition. Should setting a condition via MI work too?

Definitely.

> 
>> * mi/mi-cmds.c (build_table): Add function for break-catch.
> 
> It's not build_table, it's mi_cmds. "diff -p" sometimes lies.

Yep, and I sometimes just blindly type what diff says. Thanks.


New diff attached. Cc-ing Eli for doc. changes review.


New ChangeLog:

2008-05-28  Aleksandar Ristovski  <aristovski@qnx.com>

	* breakpoint.c (catch_command_1): Remove static declaration.
	(print_mention_exception_catchpoint): Output information only
	when non-MI protocol is active.
	* breakpoint.h (catch_command_1): New declaration.
	* doc/gdb.texinfo (break-catch): Remove @ignore and add text for
	break-catch command.
	* mi/mi-cmd-break.c (mi_cmd_break_catch): New function.
	* mi/mi-cmds.c (mi_cmds): Add function for break-catch.
	* mi/mi-cmds.h (mi_cmd_break_catch): New declaration.

[-- Attachment #2: break-catch-200805281309.diff --]
[-- Type: text/plain, Size: 8135 bytes --]

Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.322
diff -u -p -r1.322 breakpoint.c
--- gdb/breakpoint.c	28 May 2008 14:04:21 -0000	1.322
+++ gdb/breakpoint.c	28 May 2008 17:08:19 -0000
@@ -67,8 +67,6 @@
 static void until_break_command_continuation (struct continuation_arg *arg, 
 					      int error);
 
-static void catch_command_1 (char *, int, int);
-
 static void enable_delete_command (char *, int);
 
 static void enable_delete_breakpoint (struct breakpoint *);
@@ -6549,13 +6547,16 @@ print_mention_exception_catchpoint (stru
   int bp_temp;
   int bp_throw;
 
-  bp_temp = b->loc->owner->disposition == disp_del;
-  bp_throw = strstr (b->addr_string, "throw") != NULL;
-  ui_out_text (uiout, bp_temp ? _("Temporary catchpoint ")
-			      : _("Catchpoint "));
-  ui_out_field_int (uiout, "bkptno", b->number);
-  ui_out_text (uiout, bp_throw ? _(" (throw)")
-			       : _(" (catch)"));
+  if (!ui_out_is_mi_like_p (uiout))
+    {
+      bp_temp = b->loc->owner->disposition == disp_del;
+      bp_throw = strstr (b->addr_string, "throw") != NULL;
+      ui_out_text (uiout, bp_temp ? _("Temporary catchpoint ")
+				  : _("Catchpoint "));
+      ui_out_field_int (uiout, "bkptno", b->number);
+      ui_out_text (uiout, bp_throw ? _(" (throw)")
+				   : _(" (catch)"));
+    }
 }
 
 static struct breakpoint_ops gnu_v3_exception_catchpoint_ops = {
@@ -6691,7 +6692,7 @@ catch_assert_command (char *arg, int tem
                                    tempflag, from_tty);
 }
 
-static void
+void
 catch_command_1 (char *arg, int tempflag, int from_tty)
 {
 
Index: gdb/breakpoint.h
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.h,v
retrieving revision 1.72
diff -u -p -r1.72 breakpoint.h
--- gdb/breakpoint.h	4 May 2008 19:38:59 -0000	1.72
+++ gdb/breakpoint.h	28 May 2008 17:08:21 -0000
@@ -865,4 +865,6 @@ void breakpoint_restore_shadows (gdb_byt
 
 extern int breakpoints_always_inserted_mode (void);
 
+extern void catch_command_1 (char *arg, int tempflag, int from_tty);
+
 #endif /* !defined (BREAKPOINT_H) */
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.498
diff -u -p -r1.498 gdb.texinfo
--- gdb/doc/gdb.texinfo	22 May 2008 21:21:41 -0000	1.498
+++ gdb/doc/gdb.texinfo	28 May 2008 17:08:25 -0000
@@ -18486,10 +18486,50 @@ line="5",times="0",ignore="3"@}]@}
 (gdb)
 @end smallexample
 
-@ignore
 @subheading The @code{-break-catch} Command
 @findex -break-catch
 
+@subsubheading Synopsis
+
+@smallexample
+ -break-catch [ -t ] [ @var{event} ]
+@end smallexample
+
+@noindent
+Where @var{event} can be one of:
+
+@itemize @bullet
+@item catch
+@item throw
+@end itemize
+
+The possible optional parameters of this command are:
+
+@table @samp
+@item -t
+Insert a temporary catchpoint.
+@end table
+
+@subsubheading Result
+
+The result is in the form:
+
+@smallexample
+^done,bkpt=@{number="@var{number}",type="@var{type}",disp="del"|"keep",
+enabled="y"|"n",addr="@var{hex}",what="exception catch"|"exception throw",
+times="@var{times}",original-location="@var{funcname}"@}
+@end smallexample
+
+@noindent
+where @var{number} is the @value{GDBN} number for this breakpoint,
+@var{funcname} is the name of the function where the catchpoint was
+inserted (target specific) and @var{times} the number of times that 
+the breakpoint has been hit (always 0 for -break-catch but may be 
+greater for -break-info or -break-list which use the same output).
+
+Note: this format is open to change.
+
+@ignore
 @subheading The @code{-break-commands} Command
 @findex -break-commands
 @end ignore
Index: gdb/mi/mi-cmd-break.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-break.c,v
retrieving revision 1.19
diff -u -p -r1.19 mi-cmd-break.c
--- gdb/mi/mi-cmd-break.c	1 Feb 2008 16:24:46 -0000	1.19
+++ gdb/mi/mi-cmd-break.c	28 May 2008 17:08:26 -0000
@@ -241,3 +241,104 @@ mi_cmd_break_watch (char *command, char 
     }
   return MI_CMD_DONE;
 }
+
+/* Implements the -break-catch command.
+   See the MI manual for the list of possible options.  */
+
+enum mi_cmd_result
+mi_cmd_break_catch (char *command, char **argv, int argc)
+{
+  char *event_name = NULL;
+  enum bp_type type = REG_BP;
+  int temp_p = 0;
+  int thread = -1;
+  int ignore_count = 0;
+  char *condition = NULL;
+  int pending = 0;
+  struct gdb_exception e;
+  struct gdb_events *old_hooks;
+  char argument[250];
+  enum opt
+    {
+      HARDWARE_OPT, TEMP_OPT /*, REGEXP_OPT */ , CONDITION_OPT,
+      IGNORE_COUNT_OPT, THREAD_OPT, PENDING_OPT
+    };
+  static struct mi_opt opts[] =
+  {
+    {"h", HARDWARE_OPT, 0},
+    {"t", TEMP_OPT, 0},
+    {"c", CONDITION_OPT, 1},
+    {"i", IGNORE_COUNT_OPT, 1},
+    {"p", THREAD_OPT, 1},
+    { 0, 0, 0 }
+  };
+
+  /* Parse arguments. It could be -r or -h or -t, <location> or ``--''
+     to denote the end of the option list. */
+  int optind = 0;
+  char *optarg;
+  while (1)
+    {
+      int opt = mi_getopt ("mi_cmd_break_catch", argc, argv, opts, &optind, &optarg);
+      if (opt < 0)
+	break;
+      switch ((enum opt) opt)
+	{
+	case TEMP_OPT:
+	  temp_p = 1;
+	  break;
+	case HARDWARE_OPT:
+	  warning (_("Hardware flag ignored for catchpoints"));
+	  break;
+#if 0
+	case REGEXP_OPT:
+	  type = REGEXP_BP;
+	  break;
+#endif
+	case CONDITION_OPT:
+	  condition = optarg;
+	  break;
+	case IGNORE_COUNT_OPT:
+	  ignore_count = atol (optarg);
+	  warning (_("Ignore count not yet implemented for catchpoints"));
+	  break;
+	case THREAD_OPT:
+	  thread = atol (optarg);
+	  warning (_("Thread option not yet implemented for catchpoints"));
+	  break;
+	}
+    }
+
+  if (optind >= argc)
+    error (_("mi_cmd_break_catch: Missing <event name>"));
+  if (optind < argc - 1)
+    error (_("mi_cmd_break_catch: Garbage following <event name>"));
+  event_name = argv[optind];
+
+  if (condition != NULL)
+    snprintf (argument, sizeof (argument), "%s %s", event_name, condition);
+  else
+    strcpy (argument, event_name);
+  /* Now we have what we need, let's insert the breakpoint! */
+  old_hooks = deprecated_set_gdb_event_hooks (&breakpoint_hooks);
+  /* Make sure we restore hooks even if exception is thrown.  */
+  TRY_CATCH (e, RETURN_MASK_ALL)
+    {
+      switch (type)
+	{
+	case REG_BP:
+	  catch_command_1 (argument, temp_p, 0);
+	  break;
+      default:
+	  internal_error (__FILE__, __LINE__,
+			  _("mi_cmd_break_catch: Bad switch."));
+	}
+    }
+  deprecated_set_gdb_event_hooks (old_hooks);
+  if (e.reason < 0)
+    throw_exception (e);
+
+  return MI_CMD_DONE;
+}
+
+
Index: gdb/mi/mi-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.c,v
retrieving revision 1.32
diff -u -p -r1.32 mi-cmds.c
--- gdb/mi/mi-cmds.c	24 Apr 2008 13:28:06 -0000	1.32
+++ gdb/mi/mi-cmds.c	28 May 2008 17:08:26 -0000
@@ -33,7 +33,7 @@ static void build_table (struct mi_cmd *
 struct mi_cmd mi_cmds[] =
 {
   { "break-after", { "ignore", 1 }, NULL },
-  { "break-catch", { NULL, 0 }, NULL },
+  { "break-catch", { NULL, 0 }, mi_cmd_break_catch},
   { "break-commands", { NULL, 0 }, NULL },
   { "break-condition", { "cond", 1 }, NULL },
   { "break-delete", { "delete breakpoint", 1 }, NULL },
Index: gdb/mi/mi-cmds.h
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.h,v
retrieving revision 1.30
diff -u -p -r1.30 mi-cmds.h
--- gdb/mi/mi-cmds.h	24 Apr 2008 13:28:06 -0000	1.30
+++ gdb/mi/mi-cmds.h	28 May 2008 17:08:26 -0000
@@ -54,6 +54,7 @@ typedef enum mi_cmd_result (mi_cmd_argv_
 /* Function implementing each command */
 extern mi_cmd_argv_ftype mi_cmd_break_insert;
 extern mi_cmd_argv_ftype mi_cmd_break_watch;
+extern mi_cmd_argv_ftype mi_cmd_break_catch;
 extern mi_cmd_argv_ftype mi_cmd_disassemble;
 extern mi_cmd_argv_ftype mi_cmd_data_evaluate_expression;
 extern mi_cmd_argv_ftype mi_cmd_data_list_register_names;

Re: [patch/MI] Implementation for break-catch command

[Eli Zaretskii]

> From:  Aleksandar Ristovski <aristovski@qnx.com>
> Date:  Wed, 28 May 2008 13:49:00 -0400
> Cc:  Eli Zaretskii <eliz@gnu.org>
> 
> Cc-ing Eli for doc. changes review.

Fine with me, except for the following gotchas:

> +@subsubheading Synopsis
> +
> +@smallexample
> + -break-catch [ -t ] [ @var{event} ]
> +@end smallexample
> +
> +@noindent
> +Where @var{event} can be one of:

This "where" should not be capitalized, since it does not begin a new
sentence.

> +@smallexample
> +^done,bkpt=@{number="@var{number}",type="@var{type}",disp="del"|"keep",
> +enabled="y"|"n",addr="@var{hex}",what="exception catch"|"exception throw",
> +times="@var{times}",original-location="@var{funcname}"@}
> +@end smallexample

The lines here are too long: anything longer than 66 characters runs a
risk of overflowing the page margin in the printed manual.  So please
break them into shorter lines.

> +where @var{number} is the @value{GDBN} number for this breakpoint,

"breakpoint" or "catchpoint"?

> +the breakpoint has been hit (always 0 for -break-catch but may be 
> +greater for -break-info or -break-list which use the same output).

Please give commands you mention here the @code markup.

Thanks.

Re: [patch/MI] Implementation for break-catch command

[Aleksandar Ristovski]

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

Eli Zaretskii wrote:
>> +Where @var{event} can be one of:
> 
> This "where" should not be capitalized, since it does not begin a new
> sentence.

Fixed.

>> +@end smallexample
> 
> The lines here are too long: anything longer than 66 characters runs a
> risk of overflowing the page margin in the printed manual.  So please
> break them into shorter lines.

Fixed.

> 
>> +where @var{number} is the @value{GDBN} number for this breakpoint,
> 
> "breakpoint" or "catchpoint"?

Catchpoint, thanks.

>> +greater for -break-info or -break-list which use the same output).
> 
> Please give commands you mention here the @code markup.

Done.


Also cleaned up the new function (Vladimir?).


2008-05-29  Aleksandar Ristovski  <aristovski@qnx.com>

	* breakpoint.c (catch_command_1): Remove static declaration.
	(print_mention_exception_catchpoint): Output information only
	when non-MI protocol is active.
	* breakpoint.h (catch_command_1): New declaration.
	* doc/gdb.texinfo (break-catch): Remove @ignore and add text for
	break-catch command.
	* mi/mi-cmd-break.c (mi_cmd_break_catch): New function.
	* mi/mi-cmds.c (mi_cmds): Add function for break-catch.
	* mi/mi-cmds.h (mi_cmd_break_catch): New declaration.

[-- Attachment #2: break-catch-200805291030.diff --]
[-- Type: text/plain, Size: 7934 bytes --]

/* MI Command Set - breakpoint and watchpoint commands.
   Copyright (C) 2000, 2001, 2002, 2007, 2008 Free Software Foundation, Inc.
   Contributed by Cygnus Solutions (a Red Hat company).

   This file is part of GDB.

   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 3 of the License, or
   (at your option) any later version.

   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.

   You should have received a copy of the GNU General Public License
   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

#include "defs.h"
#include "mi-cmds.h"
#include "ui-out.h"
#include "mi-out.h"
#include "breakpoint.h"
#include "gdb_string.h"
#include "mi-getopt.h"
#include "gdb-events.h"
#include "gdb.h"
#include "exceptions.h"

enum
  {
    FROM_TTY = 0
  };

/* Output a single breakpoint. */

static void
breakpoint_notify (int b)
{
  gdb_breakpoint_query (uiout, b, NULL);
}


struct gdb_events breakpoint_hooks =
{
  breakpoint_notify,
  breakpoint_notify,
  breakpoint_notify,
};


enum bp_type
  {
    REG_BP,
    HW_BP,
    REGEXP_BP
  };

/* Implements the -break-insert command.
   See the MI manual for the list of possible options.  */

enum mi_cmd_result
mi_cmd_break_insert (char *command, char **argv, int argc)
{
  char *address = NULL;
  enum bp_type type = REG_BP;
  int temp_p = 0;
  int thread = -1;
  int ignore_count = 0;
  char *condition = NULL;
  int pending = 0;
  struct gdb_exception e;
  struct gdb_events *old_hooks;
  enum opt
    {
      HARDWARE_OPT, TEMP_OPT /*, REGEXP_OPT */ , CONDITION_OPT,
      IGNORE_COUNT_OPT, THREAD_OPT, PENDING_OPT
    };
  static struct mi_opt opts[] =
  {
    {"h", HARDWARE_OPT, 0},
    {"t", TEMP_OPT, 0},
    {"c", CONDITION_OPT, 1},
    {"i", IGNORE_COUNT_OPT, 1},
    {"p", THREAD_OPT, 1},
    {"f", PENDING_OPT, 0},
    { 0, 0, 0 }
  };

  /* Parse arguments. It could be -r or -h or -t, <location> or ``--''
     to denote the end of the option list. */
  int optind = 0;
  char *optarg;
  while (1)
    {
      int opt = mi_getopt ("mi_cmd_break_insert", argc, argv, opts, &optind, &optarg);
      if (opt < 0)
	break;
      switch ((enum opt) opt)
	{
	case TEMP_OPT:
	  temp_p = 1;
	  break;
	case HARDWARE_OPT:
	  type = HW_BP;
	  break;
#if 0
	case REGEXP_OPT:
	  type = REGEXP_BP;
	  break;
#endif
	case CONDITION_OPT:
	  condition = optarg;
	  break;
	case IGNORE_COUNT_OPT:
	  ignore_count = atol (optarg);
	  break;
	case THREAD_OPT:
	  thread = atol (optarg);
	  break;
	case PENDING_OPT:
	  pending = 1;
	  break;
	}
    }

  if (optind >= argc)
    error (_("mi_cmd_break_insert: Missing <location>"));
  if (optind < argc - 1)
    error (_("mi_cmd_break_insert: Garbage following <location>"));
  address = argv[optind];

  /* Now we have what we need, let's insert the breakpoint! */
  old_hooks = deprecated_set_gdb_event_hooks (&breakpoint_hooks);
  /* Make sure we restore hooks even if exception is thrown.  */
  TRY_CATCH (e, RETURN_MASK_ALL)
    {
      switch (type)
	{
	case REG_BP:
	  set_breakpoint (address, condition,
			  0 /*hardwareflag */ , temp_p,
			  thread, ignore_count,
			  pending);
	  break;
	case HW_BP:
	  set_breakpoint (address, condition,
			  1 /*hardwareflag */ , temp_p,
			  thread, ignore_count,
			  pending);
	  break;
#if 0
	case REGEXP_BP:
	  if (temp_p)
	    error (_("mi_cmd_break_insert: Unsupported tempoary regexp breakpoint"));
	  else
	    rbreak_command_wrapper (address, FROM_TTY);
	  return MI_CMD_DONE;
	  break;
#endif
	default:
	  internal_error (__FILE__, __LINE__,
			  _("mi_cmd_break_insert: Bad switch."));
	}
    }
  deprecated_set_gdb_event_hooks (old_hooks);
  if (e.reason < 0)
    throw_exception (e);

  return MI_CMD_DONE;
}

enum wp_type
{
  REG_WP,
  READ_WP,
  ACCESS_WP
};

/* Insert a watchpoint. The type of watchpoint is specified by the
   first argument: 
   -break-watch <expr> --> insert a regular wp.  
   -break-watch -r <expr> --> insert a read watchpoint.
   -break-watch -a <expr> --> insert an access wp. */

enum mi_cmd_result
mi_cmd_break_watch (char *command, char **argv, int argc)
{
  char *expr = NULL;
  enum wp_type type = REG_WP;
  enum opt
    {
      READ_OPT, ACCESS_OPT
    };
  static struct mi_opt opts[] =
  {
    {"r", READ_OPT, 0},
    {"a", ACCESS_OPT, 0},
    { 0, 0, 0 }
  };

  /* Parse arguments. */
  int optind = 0;
  char *optarg;
  while (1)
    {
      int opt = mi_getopt ("mi_cmd_break_watch", argc, argv, opts, &optind, &optarg);
      if (opt < 0)
	break;
      switch ((enum opt) opt)
	{
	case READ_OPT:
	  type = READ_WP;
	  break;
	case ACCESS_OPT:
	  type = ACCESS_WP;
	  break;
	}
    }
  if (optind >= argc)
    error (_("mi_cmd_break_watch: Missing <expression>"));
  if (optind < argc - 1)
    error (_("mi_cmd_break_watch: Garbage following <expression>"));
  expr = argv[optind];

  /* Now we have what we need, let's insert the watchpoint! */
  switch (type)
    {
    case REG_WP:
      watch_command_wrapper (expr, FROM_TTY);
      break;
    case READ_WP:
      rwatch_command_wrapper (expr, FROM_TTY);
      break;
    case ACCESS_WP:
      awatch_command_wrapper (expr, FROM_TTY);
      break;
    default:
      error (_("mi_cmd_break_watch: Unknown watchpoint type."));
    }
  return MI_CMD_DONE;
}

/* Implements the -break-catch command.
   See the MI manual for the list of possible options.  */

enum mi_cmd_result
mi_cmd_break_catch (char *command, char **argv, int argc)
{
  char *event_name = NULL;
  enum bp_type type = REG_BP;
  int temp_p = 0;
  int thread = -1;
  int ignore_count = 0;
  char *condition = NULL;
  struct gdb_exception e;
  struct gdb_events *old_hooks;
  char argument[250];
  enum opt
    {
      HARDWARE_OPT, TEMP_OPT, CONDITION_OPT,
      IGNORE_COUNT_OPT, THREAD_OPT
    };
  static struct mi_opt opts[] =
  {
    {"h", HARDWARE_OPT, 0},
    {"t", TEMP_OPT, 0},
    {"c", CONDITION_OPT, 1},
    {"i", IGNORE_COUNT_OPT, 1},
    {"p", THREAD_OPT, 1},
    { 0, 0, 0 }
  };

  /* Parse arguments. It could be -r or -h or -t, <location> or ``--''
     to denote the end of the option list. */
  int optind = 0;
  char *optarg;
  while (1)
    {
      int opt = mi_getopt ("mi_cmd_break_catch", argc, argv, opts, &optind, &optarg);
      if (opt < 0)
	break;
      switch ((enum opt) opt)
	{
	case TEMP_OPT:
	  temp_p = 1;
	  break;
	case HARDWARE_OPT:
	  warning (_("Hardware flag ignored for catchpoints"));
	  break;
	case CONDITION_OPT:
	  condition = optarg;
	  break;
	case IGNORE_COUNT_OPT:
	  ignore_count = atol (optarg);
	  warning (_("Ignore count not yet implemented for catchpoints"));
	  break;
	case THREAD_OPT:
	  thread = atol (optarg);
	  warning (_("Thread option not yet implemented for catchpoints"));
	  break;
	}
    }

  if (optind >= argc)
    error (_("mi_cmd_break_catch: Missing <event name>"));
  if (optind < argc - 1)
    error (_("mi_cmd_break_catch: Garbage following <event name>"));
  event_name = argv[optind];

  if (condition != NULL)
    snprintf (argument, sizeof (argument), "%s %s", event_name, condition);
  else
    strcpy (argument, event_name);
  /* Now we have what we need, let's insert the breakpoint! */
  old_hooks = deprecated_set_gdb_event_hooks (&breakpoint_hooks);
  /* Make sure we restore hooks even if exception is thrown.  */
  TRY_CATCH (e, RETURN_MASK_ALL)
    {
      switch (type)
	{
	case REG_BP:
	  catch_command_1 (argument, temp_p, 0);
	  break;
      default:
	  internal_error (__FILE__, __LINE__,
			  _("mi_cmd_break_catch: Bad switch."));
	}
    }
  deprecated_set_gdb_event_hooks (old_hooks);
  if (e.reason < 0)
    throw_exception (e);

  return MI_CMD_DONE;
}

Re: [patch/MI] Implementation for break-catch command

[Aleksandar Ristovski]

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

... if only it was the diff and not the .c file. 

Sorry about that. This time the diff is attached. Really.


Aleksandar Ristovski wrote:
> Eli Zaretskii wrote:
>>> +Where @var{event} can be one of:
>>
>> This "where" should not be capitalized, since it does not begin a new
>> sentence.
> 
> Fixed.
> 
>>> +@end smallexample
>>
>> The lines here are too long: anything longer than 66 characters runs a
>> risk of overflowing the page margin in the printed manual.  So please
>> break them into shorter lines.
> 
> Fixed.
> 
>>
>>> +where @var{number} is the @value{GDBN} number for this breakpoint,
>>
>> "breakpoint" or "catchpoint"?
> 
> Catchpoint, thanks.
> 
>>> +greater for -break-info or -break-list which use the same output).
>>
>> Please give commands you mention here the @code markup.
> 
> Done.
> 
> 
> Also cleaned up the new function (Vladimir?).
> 
> 
> 2008-05-29  Aleksandar Ristovski  <aristovski@qnx.com>
> 
>     * breakpoint.c (catch_command_1): Remove static declaration.
>     (print_mention_exception_catchpoint): Output information only
>     when non-MI protocol is active.
>     * breakpoint.h (catch_command_1): New declaration.
>     * doc/gdb.texinfo (break-catch): Remove @ignore and add text for
>     break-catch command.
>     * mi/mi-cmd-break.c (mi_cmd_break_catch): New function.
>     * mi/mi-cmds.c (mi_cmds): Add function for break-catch.
>     * mi/mi-cmds.h (mi_cmd_break_catch): New declaration.
> 


[-- Attachment #2: break-catch-200805291030.diff --]
[-- Type: text/plain, Size: 8038 bytes --]

Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.322
diff -u -p -r1.322 breakpoint.c
--- gdb/breakpoint.c	28 May 2008 14:04:21 -0000	1.322
+++ gdb/breakpoint.c	29 May 2008 14:44:59 -0000
@@ -67,8 +67,6 @@
 static void until_break_command_continuation (struct continuation_arg *arg, 
 					      int error);
 
-static void catch_command_1 (char *, int, int);
-
 static void enable_delete_command (char *, int);
 
 static void enable_delete_breakpoint (struct breakpoint *);
@@ -6549,13 +6547,16 @@ print_mention_exception_catchpoint (stru
   int bp_temp;
   int bp_throw;
 
-  bp_temp = b->loc->owner->disposition == disp_del;
-  bp_throw = strstr (b->addr_string, "throw") != NULL;
-  ui_out_text (uiout, bp_temp ? _("Temporary catchpoint ")
-			      : _("Catchpoint "));
-  ui_out_field_int (uiout, "bkptno", b->number);
-  ui_out_text (uiout, bp_throw ? _(" (throw)")
-			       : _(" (catch)"));
+  if (!ui_out_is_mi_like_p (uiout))
+    {
+      bp_temp = b->loc->owner->disposition == disp_del;
+      bp_throw = strstr (b->addr_string, "throw") != NULL;
+      ui_out_text (uiout, bp_temp ? _("Temporary catchpoint ")
+				  : _("Catchpoint "));
+      ui_out_field_int (uiout, "bkptno", b->number);
+      ui_out_text (uiout, bp_throw ? _(" (throw)")
+				   : _(" (catch)"));
+    }
 }
 
 static struct breakpoint_ops gnu_v3_exception_catchpoint_ops = {
@@ -6691,7 +6692,7 @@ catch_assert_command (char *arg, int tem
                                    tempflag, from_tty);
 }
 
-static void
+void
 catch_command_1 (char *arg, int tempflag, int from_tty)
 {
 
Index: gdb/breakpoint.h
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.h,v
retrieving revision 1.72
diff -u -p -r1.72 breakpoint.h
--- gdb/breakpoint.h	4 May 2008 19:38:59 -0000	1.72
+++ gdb/breakpoint.h	29 May 2008 14:45:00 -0000
@@ -865,4 +865,6 @@ void breakpoint_restore_shadows (gdb_byt
 
 extern int breakpoints_always_inserted_mode (void);
 
+extern void catch_command_1 (char *arg, int tempflag, int from_tty);
+
 #endif /* !defined (BREAKPOINT_H) */
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.498
diff -u -p -r1.498 gdb.texinfo
--- gdb/doc/gdb.texinfo	22 May 2008 21:21:41 -0000	1.498
+++ gdb/doc/gdb.texinfo	29 May 2008 14:45:07 -0000
@@ -18486,10 +18486,52 @@ line="5",times="0",ignore="3"@}]@}
 (gdb)
 @end smallexample
 
-@ignore
 @subheading The @code{-break-catch} Command
 @findex -break-catch
 
+@subsubheading Synopsis
+
+@smallexample
+ -break-catch [ -t ] [ @var{event} ]
+@end smallexample
+
+@noindent
+where @var{event} can be one of:
+
+@itemize @bullet
+@item catch
+@item throw
+@end itemize
+
+The possible optional parameters of this command are:
+
+@table @samp
+@item -t
+Insert a temporary catchpoint.
+@end table
+
+@subsubheading Result
+
+The result is in the form:
+
+@smallexample
+^done,bkpt=@{number="@var{number}",type="@var{type}",
+disp="del"|"keep",enabled="y"|"n",addr="@var{hex}",
+what="exception catch"|"exception throw",times="@var{times}",
+original-location="@var{funcname}"@}
+@end smallexample
+
+@noindent
+where @var{number} is the @value{GDBN} number for this catchpoint,
+@var{funcname} is the name of the function where the catchpoint
+was inserted (target specific) and @var{times} the number of times
+that the catchpoint has been hit (always 0 for @code{-break-catch}
+but may be greater for @code{-break-info} or @code{-break-list} 
+which use the same output).
+
+Note: this format is open to change.
+
+@ignore
 @subheading The @code{-break-commands} Command
 @findex -break-commands
 @end ignore
Index: gdb/mi/mi-cmd-break.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-break.c,v
retrieving revision 1.19
diff -u -p -r1.19 mi-cmd-break.c
--- gdb/mi/mi-cmd-break.c	1 Feb 2008 16:24:46 -0000	1.19
+++ gdb/mi/mi-cmd-break.c	29 May 2008 14:45:07 -0000
@@ -241,3 +241,98 @@ mi_cmd_break_watch (char *command, char 
     }
   return MI_CMD_DONE;
 }
+
+/* Implements the -break-catch command.
+   See the MI manual for the list of possible options.  */
+
+enum mi_cmd_result
+mi_cmd_break_catch (char *command, char **argv, int argc)
+{
+  char *event_name = NULL;
+  enum bp_type type = REG_BP;
+  int temp_p = 0;
+  int thread = -1;
+  int ignore_count = 0;
+  char *condition = NULL;
+  struct gdb_exception e;
+  struct gdb_events *old_hooks;
+  char argument[250];
+  enum opt
+    {
+      HARDWARE_OPT, TEMP_OPT, CONDITION_OPT,
+      IGNORE_COUNT_OPT, THREAD_OPT
+    };
+  static struct mi_opt opts[] =
+  {
+    {"h", HARDWARE_OPT, 0},
+    {"t", TEMP_OPT, 0},
+    {"c", CONDITION_OPT, 1},
+    {"i", IGNORE_COUNT_OPT, 1},
+    {"p", THREAD_OPT, 1},
+    { 0, 0, 0 }
+  };
+
+  /* Parse arguments. It could be -r or -h or -t, <location> or ``--''
+     to denote the end of the option list. */
+  int optind = 0;
+  char *optarg;
+  while (1)
+    {
+      int opt = mi_getopt ("mi_cmd_break_catch", argc, argv, opts, &optind, &optarg);
+      if (opt < 0)
+	break;
+      switch ((enum opt) opt)
+	{
+	case TEMP_OPT:
+	  temp_p = 1;
+	  break;
+	case HARDWARE_OPT:
+	  warning (_("Hardware flag ignored for catchpoints"));
+	  break;
+	case CONDITION_OPT:
+	  condition = optarg;
+	  break;
+	case IGNORE_COUNT_OPT:
+	  ignore_count = atol (optarg);
+	  warning (_("Ignore count not yet implemented for catchpoints"));
+	  break;
+	case THREAD_OPT:
+	  thread = atol (optarg);
+	  warning (_("Thread option not yet implemented for catchpoints"));
+	  break;
+	}
+    }
+
+  if (optind >= argc)
+    error (_("mi_cmd_break_catch: Missing <event name>"));
+  if (optind < argc - 1)
+    error (_("mi_cmd_break_catch: Garbage following <event name>"));
+  event_name = argv[optind];
+
+  if (condition != NULL)
+    snprintf (argument, sizeof (argument), "%s %s", event_name, condition);
+  else
+    strcpy (argument, event_name);
+  /* Now we have what we need, let's insert the breakpoint! */
+  old_hooks = deprecated_set_gdb_event_hooks (&breakpoint_hooks);
+  /* Make sure we restore hooks even if exception is thrown.  */
+  TRY_CATCH (e, RETURN_MASK_ALL)
+    {
+      switch (type)
+	{
+	case REG_BP:
+	  catch_command_1 (argument, temp_p, 0);
+	  break;
+      default:
+	  internal_error (__FILE__, __LINE__,
+			  _("mi_cmd_break_catch: Bad switch."));
+	}
+    }
+  deprecated_set_gdb_event_hooks (old_hooks);
+  if (e.reason < 0)
+    throw_exception (e);
+
+  return MI_CMD_DONE;
+}
+
+
Index: gdb/mi/mi-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.c,v
retrieving revision 1.32
diff -u -p -r1.32 mi-cmds.c
--- gdb/mi/mi-cmds.c	24 Apr 2008 13:28:06 -0000	1.32
+++ gdb/mi/mi-cmds.c	29 May 2008 14:45:08 -0000
@@ -33,7 +33,7 @@ static void build_table (struct mi_cmd *
 struct mi_cmd mi_cmds[] =
 {
   { "break-after", { "ignore", 1 }, NULL },
-  { "break-catch", { NULL, 0 }, NULL },
+  { "break-catch", { NULL, 0 }, mi_cmd_break_catch},
   { "break-commands", { NULL, 0 }, NULL },
   { "break-condition", { "cond", 1 }, NULL },
   { "break-delete", { "delete breakpoint", 1 }, NULL },
Index: gdb/mi/mi-cmds.h
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.h,v
retrieving revision 1.30
diff -u -p -r1.30 mi-cmds.h
--- gdb/mi/mi-cmds.h	24 Apr 2008 13:28:06 -0000	1.30
+++ gdb/mi/mi-cmds.h	29 May 2008 14:45:08 -0000
@@ -54,6 +54,7 @@ typedef enum mi_cmd_result (mi_cmd_argv_
 /* Function implementing each command */
 extern mi_cmd_argv_ftype mi_cmd_break_insert;
 extern mi_cmd_argv_ftype mi_cmd_break_watch;
+extern mi_cmd_argv_ftype mi_cmd_break_catch;
 extern mi_cmd_argv_ftype mi_cmd_disassemble;
 extern mi_cmd_argv_ftype mi_cmd_data_evaluate_expression;
 extern mi_cmd_argv_ftype mi_cmd_data_list_register_names;

Re: [patch/MI] Implementation for break-catch command

[Vladimir Prus]

On Sunday 01 June 2008 03:40:06 Nick Roberts wrote:
>  > >  static void enable_delete_breakpoint (struct breakpoint *);
>  > > @@ -6549,13 +6547,16 @@ print_mention_exception_catchpoint (stru
>  > >    int bp_temp;
>  > >    int bp_throw;
>  > >  
>  > > -  bp_temp = b->loc->owner->disposition == disp_del;
>  > > -  bp_throw = strstr (b->addr_string, "throw") != NULL;
>  > > -  ui_out_text (uiout, bp_temp ? _("Temporary catchpoint ")
>  > > -                             : _("Catchpoint "));
>  > > -  ui_out_field_int (uiout, "bkptno", b->number);
>  > > -  ui_out_text (uiout, bp_throw ? _(" (throw)")
>  > > -                              : _(" (catch)"));
>  > > +  if (!ui_out_is_mi_like_p (uiout))
>  > > +    {
>  > > +      bp_temp = b->loc->owner->disposition == disp_del;
>  > > +      bp_throw = strstr (b->addr_string, "throw") != NULL;
>  > > +      ui_out_text (uiout, bp_temp ? _("Temporary catchpoint ")
>  > > +                                 : _("Catchpoint "));
>  > > +      ui_out_field_int (uiout, "bkptno", b->number);
>  > > +      ui_out_text (uiout, bp_throw ? _(" (throw)")
>  > > +                                  : _(" (catch)"));
>  > > +    }
>  > 
>  > Uh, seems like there's some messiness in MI here. For ordinary breakpoints,
>  > MI installs the 'breakpoint_notify' hook that prints all the information,
>  > and 'mention' specifically avoids printing anything for MI. For watchpoints,
>  > however, no hooks are installed, and 'mention' does print something --
>  > basically just id and expression.
>  > 
>  > For -break-catch, you choose to install hooks, and disable printing anything
>  > in 'mention' -- seems fine to me.
> 
> I think all these "breakpoints" should be changed to use observers.  This
> seems to just add to the ugliness.

They definitely should.

- Volodya 

Re: [patch/MI] Implementation for break-catch command

[Aleksandar Ristovski]

Vladimir Prus wrote:
> On Sunday 01 June 2008 03:40:06 Nick Roberts wrote:
>>
>> I think all these "breakpoints" should be changed to use observers.  This
>> seems to just add to the ugliness.
> 
> They definitely should.

Well then, what is the preferred way? I can tweak my patch to take your comments into consideration, but if a switch to observers is going to happen soon, then I guess the break-catch should get this new stuff right away? Implementing observer_notify for all the breakpoints would break the compatibility and require quite a bit of work on the testsuite side.

Either way, Vladimir, it's your call - let me know.

Re: [patch/MI] Implementation for break-catch command

[Vladimir Prus]

On Monday 02 June 2008 17:22:45 you wrote:
> Vladimir Prus wrote:
> > On Sunday 01 June 2008 03:40:06 Nick Roberts wrote:
> >>
> >> I think all these "breakpoints" should be changed to use observers.  This
> >> seems to just add to the ugliness.
> > 
> > They definitely should.
> 
> Well then, what is the preferred way? I can tweak my patch to 
> take your comments into consideration, but if a switch to observers is 
> going to happen soon, then I guess the break-catch should get 
> this new stuff right away? Implementing observer_notify for all the breakpoints
> would break the compatibility and require quite a bit of work on the testsuite side.
> 
> Either way, Vladimir, it's your call - let me know.

I don't think you should worry about observers -- your patch makes a nice self-contained
improvement, and it does not make sense to mix it with any other cleanups. For now, using
hooks is fine, and my comment about observers does not call for any changes in this
patch.
I don't think any of my comments will be invalidated when we switch to observers.

Thanks,
Volodya

Re: [patch/MI] Implementation for break-catch command

[Aleksandar Ristovski]

Vladimir Prus wrote:
> On Thursday 29 May 2008 18:48:36 Aleksandar Ristovski wrote:
>> Index: gdb/breakpoint.c
> 
> First of all, I would like a some testcases to be added with this patch.
> 
> I have some questions about behaviour, too:
> 
> 1. -break-catch throw
>    ^done,bkpt={number="3",type="breakpoint",disp="keep",enabled="y",
>                addr="0xb7ef6e05",what="exception throw",times="0",
>                original-location="__cxa_throw"}
>    (gdb)
> 
> The way this breakpoint is printed seems not sufficiently precise. Suppose
> such a breakpoint was added via CLI. GUI would like to detect this, and
> add a breakpoint to the breakpoint list as "catch" breakpoint. How can we
> detect it is a catch breakpoint? The "what" field appears to be of a fairly
> ad-hoc use in MI. 

A few questions:

This is the output from -break-list:

^done,BreakpointTable={nr_rows="3",nr_cols="6",hdr=[
{width="7",alignment="-1",col_name="number",colhdr="Num"},
{width="14",alignment="-1",col_name="type",colhdr="Type"},
{width="4",alignment="-1",col_name="disp",colhdr="Disp"},
{width="3",alignment="-1",col_name="enabled",colhdr="Enb"},
{width="10",alignment="-1",col_name="addr",colhdr="Address"},
{width="40",alignment="2",col_name="what",colhdr="What"}],
body=[bkpt=
{number="1",type="breakpoint",disp="keep",enabled="y",addr="0x080488c4",func="main",file="/tmp/catch.cc",fullname="/tmp/catch.cc",line="9",times="0",original-location="main"},
...

The fields listed in the header do not match the body. Should they? What is the point of the header if rows can have just any field?

> Should the type of breakpoint be actually 'exception-throw'?
> 

I agree, 'breakpoint' is not precise. I think the type should be "catch " + event name, e.g. "catch catch", "catch throw" "catch signal" to keep terminology consistent (there are "catch load" "catch unload"...). 

Reason field (for stopped message) - I would suggest simply specifying the type again instead of introducing a new string, e.g. '*stopped,reason="catch catch"'.

Thanks,

Aleksandar

Re: [patch/MI] Implementation for break-catch command

[Vladimir Prus]

On Monday 02 June 2008 20:06:07 you wrote:
> Vladimir Prus wrote:
> > On Thursday 29 May 2008 18:48:36 Aleksandar Ristovski wrote:
> >> Index: gdb/breakpoint.c
> > 
> > First of all, I would like a some testcases to be added with this patch.
> > 
> > I have some questions about behaviour, too:
> > 
> > 1. -break-catch throw
> >    ^done,bkpt={number="3",type="breakpoint",disp="keep",enabled="y",
> >                addr="0xb7ef6e05",what="exception throw",times="0",
> >                original-location="__cxa_throw"}
> >    (gdb)
> > 
> > The way this breakpoint is printed seems not sufficiently precise. Suppose
> > such a breakpoint was added via CLI. GUI would like to detect this, and
> > add a breakpoint to the breakpoint list as "catch" breakpoint. How can we
> > detect it is a catch breakpoint? The "what" field appears to be of a fairly
> > ad-hoc use in MI. 
> 
> A few questions:
> 
> This is the output from -break-list:
> 
> ^done,BreakpointTable={nr_rows="3",nr_cols="6",hdr=[
> {width="7",alignment="-1",col_name="number",colhdr="Num"},
> {width="14",alignment="-1",col_name="type",colhdr="Type"},
> {width="4",alignment="-1",col_name="disp",colhdr="Disp"},
> {width="3",alignment="-1",col_name="enabled",colhdr="Enb"},
> {width="10",alignment="-1",col_name="addr",colhdr="Address"},
> {width="40",alignment="2",col_name="what",colhdr="What"}],
> body=[bkpt=
> {number="1",type="breakpoint",disp="keep",enabled="y",addr="0x080488c4",func="main",file="/tmp/catch.cc",fullname="/tmp/catch.cc",line="9",times="0",original-location="main"},
> ...
> 
> The fields listed in the header do not match the body.

Well :-)
 
> Should they? 

Not really. Given that no frontend now is broken due to the fact that they don't match,
not need to fix anything.

> What is the point of the header if rows can have just any field?  

It's historical baggage. This 'table' thing, to the best of my knowledge, is used only for
breakpoints, and makes no sense -- clearly, no frontend is going to dynamically adjust
the columns of its breakpoint table according to gdb responses :-)

> > Should the type of breakpoint be actually 'exception-throw'?
> > 
> 
> I agree, 'breakpoint' is not precise. I think the type should be "catch " + event name, e.g. "catch catch", "catch throw" "catch signal" to keep terminology consistent (there are "catch load" "catch unload"...). 

I agree.

> 
> Reason field (for stopped message) - I would suggest simply specifying the type 
> again instead of introducing a new string, e.g. '*stopped,reason="catch catch"'. 

Seems OK to me.

- Volodya

Re: [patch/MI] Implementation for break-catch command

[Aleksandar Ristovski]

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

Vladimir Prus wrote:
> 
> Seems OK to me.
> 
> - Volodya
> 

I completely forgot about this patch (and then I don't believe my wife when she says I am always forgetting things...)

In any case... the code has changed meanwhile so the attached patch takes care of that (removed deprecated_set_gdb_event_hooks).

Eli - you have already said this was OK for the docs, but maybe you want to re-check (the doc part has not changed).

Thanks,

Aleksandar Ristovski
QNX Software Systems





[-- Attachment #2: break-catch-200807311000.diff.ChangeLog --]
[-- Type: text/plain, Size: 523 bytes --]

2008-07-31  Aleksandar Ristovski  <aristovski@qnx.com>

	* breakpoint.c (catch_command_1): Remove static declaration.
	(print_mention_exception_catchpoint): Output information only
	when non-MI protocol is active.
	* breakpoint.h (catch_command_1): New declaration.
	* doc/gdb.texinfo (break-catch): Remove @ignore and add text for
	break-catch command.
	* mi/mi-cmd-break.c (mi_cmd_break_catch): New function.
	* mi/mi-cmds.c (mi_cmds): Add function for break-catch.
	* mi/mi-cmds.h (mi_cmd_break_catch): New declaration.

[-- Attachment #3: break-catch-200807311000.diff --]
[-- Type: text/plain, Size: 8269 bytes --]

Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.338
diff -u -p -r1.338 breakpoint.c
--- gdb/breakpoint.c	28 Jul 2008 17:53:52 -0000	1.338
+++ gdb/breakpoint.c	31 Jul 2008 13:56:00 -0000
@@ -61,8 +61,6 @@
 
 /* Prototypes for local functions. */
 
-static void catch_command_1 (char *, int, int);
-
 static void enable_delete_command (char *, int);
 
 static void enable_delete_breakpoint (struct breakpoint *);
@@ -6556,13 +6554,16 @@ print_mention_exception_catchpoint (stru
   int bp_temp;
   int bp_throw;
 
-  bp_temp = b->loc->owner->disposition == disp_del;
-  bp_throw = strstr (b->addr_string, "throw") != NULL;
-  ui_out_text (uiout, bp_temp ? _("Temporary catchpoint ")
-			      : _("Catchpoint "));
-  ui_out_field_int (uiout, "bkptno", b->number);
-  ui_out_text (uiout, bp_throw ? _(" (throw)")
-			       : _(" (catch)"));
+  if (!ui_out_is_mi_like_p (uiout))
+    {
+      bp_temp = b->loc->owner->disposition == disp_del;
+      bp_throw = strstr (b->addr_string, "throw") != NULL;
+      ui_out_text (uiout, bp_temp ? _("Temporary catchpoint ")
+				  : _("Catchpoint "));
+      ui_out_field_int (uiout, "bkptno", b->number);
+      ui_out_text (uiout, bp_throw ? _(" (throw)")
+				   : _(" (catch)"));
+    }
 }
 
 static struct breakpoint_ops gnu_v3_exception_catchpoint_ops = {
@@ -6698,7 +6699,7 @@ catch_assert_command (char *arg, int tem
                                    tempflag, from_tty);
 }
 
-static void
+void
 catch_command_1 (char *arg, int tempflag, int from_tty)
 {
 
Index: gdb/breakpoint.h
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.h,v
retrieving revision 1.76
diff -u -p -r1.76 breakpoint.h
--- gdb/breakpoint.h	25 Jul 2008 16:12:03 -0000	1.76
+++ gdb/breakpoint.h	31 Jul 2008 13:56:00 -0000
@@ -882,4 +882,6 @@ extern int breakpoints_always_inserted_m
    in our opinion won't ever trigger.  */
 extern void breakpoint_retire_moribund (void);
 
+extern void catch_command_1 (char *arg, int tempflag, int from_tty);
+
 #endif /* !defined (BREAKPOINT_H) */
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.509
diff -u -p -r1.509 gdb.texinfo
--- gdb/doc/gdb.texinfo	18 Jul 2008 20:55:33 -0000	1.509
+++ gdb/doc/gdb.texinfo	31 Jul 2008 13:56:11 -0000
@@ -18610,10 +18610,52 @@ line="5",times="0",ignore="3"@}]@}
 (gdb)
 @end smallexample
 
-@ignore
 @subheading The @code{-break-catch} Command
 @findex -break-catch
 
+@subsubheading Synopsis
+
+@smallexample
+ -break-catch [ -t ] [ @var{event} ]
+@end smallexample
+
+@noindent
+where @var{event} can be one of:
+
+@itemize @bullet
+@item catch
+@item throw
+@end itemize
+
+The possible optional parameters of this command are:
+
+@table @samp
+@item -t
+Insert a temporary catchpoint.
+@end table
+
+@subsubheading Result
+
+The result is in the form:
+
+@smallexample
+^done,bkpt=@{number="@var{number}",type="@var{type}",
+disp="del"|"keep",enabled="y"|"n",addr="@var{hex}",
+what="exception catch"|"exception throw",times="@var{times}",
+original-location="@var{funcname}"@}
+@end smallexample
+
+@noindent
+where @var{number} is the @value{GDBN} number for this catchpoint,
+@var{funcname} is the name of the function where the catchpoint
+was inserted (target specific) and @var{times} the number of times
+that the catchpoint has been hit (always 0 for @code{-break-catch}
+but may be greater for @code{-break-info} or @code{-break-list} 
+which use the same output).
+
+Note: this format is open to change.
+
+@ignore
 @subheading The @code{-break-commands} Command
 @findex -break-commands
 @end ignore
Index: gdb/mi/mi-cmd-break.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-break.c,v
retrieving revision 1.21
diff -u -p -r1.21 mi-cmd-break.c
--- gdb/mi/mi-cmd-break.c	25 Jul 2008 16:12:03 -0000	1.21
+++ gdb/mi/mi-cmd-break.c	31 Jul 2008 13:56:11 -0000
@@ -245,3 +245,105 @@ mi_cmd_break_watch (char *command, char 
       error (_("mi_cmd_break_watch: Unknown watchpoint type."));
     }
 }
+
+/* Implements the -break-catch command.
+   See the MI manual for the list of possible options.  */
+
+void
+mi_cmd_break_catch (char *command, char **argv, int argc)
+{
+  char *event_name = NULL;
+  enum bp_type type = REG_BP;
+  int temp_p = 0;
+  int thread = -1;
+  int ignore_count = 0;
+  char *condition = NULL;
+  struct gdb_exception e;
+  struct gdb_events *old_hooks;
+  char argument[250];
+  enum opt
+    {
+      HARDWARE_OPT, TEMP_OPT, CONDITION_OPT,
+      IGNORE_COUNT_OPT, THREAD_OPT
+    };
+  static struct mi_opt opts[] =
+  {
+    {"h", HARDWARE_OPT, 0},
+    {"t", TEMP_OPT, 0},
+    {"c", CONDITION_OPT, 1},
+    {"i", IGNORE_COUNT_OPT, 1},
+    {"p", THREAD_OPT, 1},
+    { 0, 0, 0 }
+  };
+
+  /* Parse arguments. It could be -r or -h or -t, <location> or ``--''
+     to denote the end of the option list. */
+  int optind = 0;
+  char *optarg;
+  while (1)
+    {
+      int opt = mi_getopt ("mi_cmd_break_catch", argc, argv, opts, &optind, &optarg);
+      if (opt < 0)
+	break;
+      switch ((enum opt) opt)
+	{
+	case TEMP_OPT:
+	  temp_p = 1;
+	  break;
+	case HARDWARE_OPT:
+	  warning (_("Hardware flag ignored for catchpoints"));
+	  break;
+	case CONDITION_OPT:
+	  condition = optarg;
+	  break;
+	case IGNORE_COUNT_OPT:
+	  ignore_count = atol (optarg);
+	  warning (_("Ignore count not yet implemented for catchpoints"));
+	  break;
+	case THREAD_OPT:
+	  thread = atol (optarg);
+	  warning (_("Thread option not yet implemented for catchpoints"));
+	  break;
+	}
+    }
+
+  if (optind >= argc)
+    error (_("mi_cmd_break_catch: Missing <event name>"));
+  if (optind < argc - 1)
+    error (_("mi_cmd_break_catch: Garbage following <event name>"));
+  event_name = argv[optind];
+
+  if (condition != NULL)
+    snprintf (argument, sizeof (argument), "%s %s", event_name, condition);
+  else
+    strcpy (argument, event_name);
+
+  /* Now we have what we need, let's insert the breakpoint! */
+  if (! mi_breakpoint_observers_installed)
+    {
+      observer_attach_breakpoint_created (breakpoint_notify);
+      observer_attach_breakpoint_modified (breakpoint_notify);
+      observer_attach_breakpoint_deleted (breakpoint_notify);
+      mi_breakpoint_observers_installed = 1;
+    }
+
+  mi_can_breakpoint_notify = 1;
+  /* Make sure we restore hooks even if exception is thrown.  */
+  TRY_CATCH (e, RETURN_MASK_ALL)
+    {
+      switch (type)
+	{
+	case REG_BP:
+	  catch_command_1 (argument, temp_p, 0);
+	  break;
+      default:
+	  internal_error (__FILE__, __LINE__,
+			  _("mi_cmd_break_catch: Bad switch."));
+	}
+    }
+  mi_can_breakpoint_notify = 0;
+  if (e.reason < 0)
+    throw_exception (e);
+}
+
+
Index: gdb/mi/mi-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.c,v
retrieving revision 1.34
diff -u -p -r1.34 mi-cmds.c
--- gdb/mi/mi-cmds.c	4 Jul 2008 09:04:36 -0000	1.34
+++ gdb/mi/mi-cmds.c	31 Jul 2008 13:56:11 -0000
@@ -33,7 +33,7 @@ static void build_table (struct mi_cmd *
 struct mi_cmd mi_cmds[] =
 {
   { "break-after", { "ignore", 1 }, NULL },
-  { "break-catch", { NULL, 0 }, NULL },
+  { "break-catch", { NULL, 0 }, mi_cmd_break_catch},
   { "break-commands", { NULL, 0 }, NULL },
   { "break-condition", { "cond", 1 }, NULL },
   { "break-delete", { "delete breakpoint", 1 }, NULL },
Index: gdb/mi/mi-cmds.h
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.h,v
retrieving revision 1.32
diff -u -p -r1.32 mi-cmds.h
--- gdb/mi/mi-cmds.h	29 Jun 2008 17:36:36 -0000	1.32
+++ gdb/mi/mi-cmds.h	31 Jul 2008 13:56:11 -0000
@@ -38,6 +38,7 @@ typedef void (mi_cmd_argv_ftype) (char *
 /* Function implementing each command */
 extern mi_cmd_argv_ftype mi_cmd_break_insert;
 extern mi_cmd_argv_ftype mi_cmd_break_watch;
+extern mi_cmd_argv_ftype mi_cmd_break_catch;
 extern mi_cmd_argv_ftype mi_cmd_disassemble;
 extern mi_cmd_argv_ftype mi_cmd_data_evaluate_expression;
 extern mi_cmd_argv_ftype mi_cmd_data_list_register_names;