[PATCH v6 1/3] MI: add the -catch-load and -catch-unload commands

[PATCH v6 1/3] MI: add the -catch-load and -catch-unload commands

[Mircea Gherzan]

They are equivalent to "catch load" and "catch unload" from CLI.

Rationale: GUIs might be interested in catching solib load or
unload events.

2012-11-16  Mircea Gherzan  <mircea.gherzan@intel.com>

	* Makefile.in (SUBDIR_MI_OBS): Add mi-cmd-catch.o.
	(SUBDIR_MI_SRCS): Add mi/mi-cmd-catch.c.
	* breakpoint.c (add_solib_catchpoint): New function based on
	that can be used by both CLI and MI, factored out from
	catch_load_or_unload.
	(catch_load_or_unload): Strip it down and make it use the
	new add_solib_catchpoint.
	* breakpoint.h (add_solib_catchpoint): Declare it.
	* mi/mi-cmd-catch.c: New file.
	* mi/mi-cmds.c (mi_cmds): Add the handlers for -catch-load
	and -catch-unload.
	* mi/mi-cmds.h: Declare the handlers for -catch-load and
	-catch-unload.

Signed-off-by: Mircea Gherzan <mircea.gherzan@intel.com>
---
 gdb/Makefile.in       |    9 ++++-
 gdb/breakpoint.c      |   33 ++++++++++++-----
 gdb/breakpoint.h      |    5 +++
 gdb/mi/mi-cmd-catch.c |   93 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/mi/mi-cmds.c      |    2 +
 gdb/mi/mi-cmds.h      |    2 +
 6 files changed, 132 insertions(+), 12 deletions(-)
 create mode 100644 gdb/mi/mi-cmd-catch.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 3dd7b85..40c7e80 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -203,13 +203,14 @@ SUBDIR_CLI_CFLAGS=
 #
 SUBDIR_MI_OBS = \
 	mi-out.o mi-console.o \
-	mi-cmds.o mi-cmd-env.o mi-cmd-var.o mi-cmd-break.o mi-cmd-stack.o \
+	mi-cmds.o mi-cmd-catch.o mi-cmd-env.o \
+	mi-cmd-var.o mi-cmd-break.o mi-cmd-stack.o \
 	mi-cmd-file.o mi-cmd-disas.o mi-symbol-cmds.o mi-cmd-target.o \
 	mi-cmd-info.o mi-interp.o \
 	mi-main.o mi-parse.o mi-getopt.o
 SUBDIR_MI_SRCS = \
 	mi/mi-out.c mi/mi-console.c \
-	mi/mi-cmds.c mi/mi-cmd-env.c \
+	mi/mi-cmds.c mi/mi-cmd-catch.c mi/mi-cmd-env.c \
 	mi/mi-cmd-var.c mi/mi-cmd-break.c mi/mi-cmd-stack.c \
 	mi/mi-cmd-file.c mi/mi-cmd-disas.c mi/mi-symbol-cmds.c \
 	mi/mi-cmd-target.c mi/mi-cmd-info.c mi/mi-interp.c \
@@ -1833,6 +1834,10 @@ mi-cmd-break.o: $(srcdir)/mi/mi-cmd-break.c
 	$(COMPILE) $(srcdir)/mi/mi-cmd-break.c
 	$(POSTCOMPILE)
 
+mi-cmd-catch.o: $(srcdir)/mi/mi-cmd-catch.c
+	$(COMPILE) $(srcdir)/mi/mi-cmd-catch.c
+	$(POSTCOMPILE)
+
 mi-cmd-disas.o: $(srcdir)/mi/mi-cmd-disas.c
 	$(COMPILE) $(srcdir)/mi/mi-cmd-disas.c
 	$(POSTCOMPILE)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 5749fa7..1c95c04 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7847,20 +7847,16 @@ print_recreate_catch_solib (struct breakpoint *b, struct ui_file *fp)
 
 static struct breakpoint_ops catch_solib_breakpoint_ops;
 
-/* A helper function that does all the work for "catch load" and
-   "catch unload".  */
+/* Shared helper function (MI and CLI) for creating and installing
+   a shared object event catchpoint.  */
 
-static void
-catch_load_or_unload (char *arg, int from_tty, int is_load,
-		      struct cmd_list_element *command)
+void
+add_solib_catchpoint (char *arg, int is_load, int is_temp, int enabled)
 {
   struct solib_catchpoint *c;
-  struct gdbarch *gdbarch = get_current_arch ();
-  int tempflag;
+  struct gdbarch *arch = get_current_arch ();
   struct cleanup *cleanup;
 
-  tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
-
   if (!arg)
     arg = "";
   arg = skip_spaces (arg);
@@ -7884,13 +7880,30 @@ catch_load_or_unload (char *arg, int from_tty, int is_load,
     }
 
   c->is_load = is_load;
-  init_catchpoint (&c->base, gdbarch, tempflag, NULL,
+  init_catchpoint (&c->base, arch, is_temp, NULL,
 		   &catch_solib_breakpoint_ops);
 
+  c->base.enable_state = enabled ? bp_enabled : bp_disabled;
+
   discard_cleanups (cleanup);
   install_breakpoint (0, &c->base, 1);
 }
 
+/* A helper function that does all the work for "catch load" and
+   "catch unload".  */
+
+static void
+catch_load_or_unload (char *arg, int from_tty, int is_load,
+		      struct cmd_list_element *command)
+{
+  int tempflag;
+  const int enabled = 1;
+
+  tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
+
+  add_solib_catchpoint (arg, is_load, tempflag, enabled);
+}
+
 static void
 catch_load_command_1 (char *arg, int from_tty,
 		      struct cmd_list_element *command)
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 8b1bcb7..11e432c 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1405,6 +1405,11 @@ extern void disable_breakpoints_in_shlibs (void);
 /* This function returns TRUE if ep is a catchpoint.  */
 extern int is_catchpoint (struct breakpoint *);
 
+/* Shared helper function (MI and CLI) for creating and installing
+   a shared object event catchpoint.  */
+extern void add_solib_catchpoint (char *arg, int is_load, int is_temp,
+                                  int enabled);
+
 /* Enable breakpoints and delete when hit.  Called with ARG == NULL
    deletes all breakpoints.  */
 extern void delete_command (char *arg, int from_tty);
diff --git a/gdb/mi/mi-cmd-catch.c b/gdb/mi/mi-cmd-catch.c
new file mode 100644
index 0000000..42bf035
--- /dev/null
+++ b/gdb/mi/mi-cmd-catch.c
@@ -0,0 +1,93 @@
+/* MI Command Set - catch commands.
+   Copyright (C) 2012 Free Software Foundation, Inc.
+
+   Contributed by Intel Corporation.
+
+   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 "arch-utils.h"
+#include "breakpoint.h"
+#include "libiberty.h"
+#include "mi-cmds.h"
+#include "mi-getopt.h"
+
+/* Common path for the -catch-load and -catch-unload.  */
+
+static void
+mi_catch_load_unload (int load, char *argv[], int argc)
+{
+  char *actual_cmd = load ? "-catch-load" : "-catch-unload";
+  int temp = 0;
+  int enabled = 1;
+  int oind = 0;
+  char *oarg;
+  enum opt
+    {
+      OPT_TEMP,
+      OPT_DISABLED,
+    };
+  static const struct mi_opt opts[] =
+    {
+      { "t", OPT_TEMP, 0 },
+      { "d", OPT_DISABLED, 0 },
+      { 0, 0, 0 }
+    };
+
+  for (;;)
+    {
+      int opt = mi_getopt (actual_cmd, argc, argv, opts,
+                           &oind, &oarg);
+
+      if (opt < 0)
+        break;
+
+      switch ((enum opt) opt)
+        {
+        case OPT_TEMP:
+          temp = 1;
+          break;
+        case OPT_DISABLED:
+          enabled = 0;
+          break;
+        }
+    }
+
+  if (oind >= argc)
+    error (_("-catch-load/unload: Missing <library name>"));
+  if (oind < argc -1)
+    error (_("-catch-load/unload: Garbage following the <library name>"));
+
+  add_solib_catchpoint (argv[oind], load, temp, enabled);
+}
+
+/* Handler for the -catch-load.  */
+
+void
+mi_cmd_catch_load (char *cmd, char *argv[], int argc)
+{
+  mi_catch_load_unload (1, argv, argc);
+}
+
+
+/* Handler for the -catch-unload.  */
+
+void
+mi_cmd_catch_unload (char *cmd, char *argv[], int argc)
+{
+  mi_catch_load_unload (0, argv, argc);
+}
+
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 572625f..f8a8285 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -67,6 +67,8 @@ static struct mi_cmd mi_cmds[] =
 		   &mi_suppress_notification.breakpoint),
   DEF_MI_CMD_MI_1 ("break-watch", mi_cmd_break_watch,
 		   &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_MI ("catch-load", mi_cmd_catch_load),
+  DEF_MI_CMD_MI ("catch-unload", mi_cmd_catch_unload),
   DEF_MI_CMD_MI ("data-disassemble", mi_cmd_disassemble),
   DEF_MI_CMD_MI ("data-evaluate-expression", mi_cmd_data_evaluate_expression),
   DEF_MI_CMD_MI ("data-list-changed-registers",
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index cf1a5eb..da8df48 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -43,6 +43,8 @@ extern mi_cmd_argv_ftype mi_cmd_break_insert;
 extern mi_cmd_argv_ftype mi_cmd_break_commands;
 extern mi_cmd_argv_ftype mi_cmd_break_passcount;
 extern mi_cmd_argv_ftype mi_cmd_break_watch;
+extern mi_cmd_argv_ftype mi_cmd_catch_load;
+extern mi_cmd_argv_ftype mi_cmd_catch_unload;
 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;
-- 
1.7.1

[PATCH v6 3/3] MI: tests for -catch-load/-catch-unload

[Mircea Gherzan]

From: Keven Boell <keven.boell@intel.com>

Added basic MI tests for the -catch-load and
-catch-unload MI commands.

2012-11-19 Keven Boell <keven.boell@intel.com>

gdb/testsuite:
	* gdb.mi/mi-catch-load-so.c: New. Clone of the
	catch load test library source file.
	* gdb.mi/mi-catch-load.c: New. Clone of the catch
	load test source file.
	* gdb.mi/mi-catch-load.exp: New. Test file for
	basic MI -catch-load and -catch-unload tests.

Signed-off-by: Keven Boell <keven.boell@intel.com>
---
 gdb/testsuite/gdb.mi/mi-catch-load-so.c |   22 ++++++++
 gdb/testsuite/gdb.mi/mi-catch-load.c    |   33 +++++++++++
 gdb/testsuite/gdb.mi/mi-catch-load.exp  |   89 +++++++++++++++++++++++++++++++
 3 files changed, 144 insertions(+), 0 deletions(-)
 create mode 100755 gdb/testsuite/gdb.mi/mi-catch-load-so.c
 create mode 100755 gdb/testsuite/gdb.mi/mi-catch-load.c
 create mode 100755 gdb/testsuite/gdb.mi/mi-catch-load.exp

diff --git a/gdb/testsuite/gdb.mi/mi-catch-load-so.c b/gdb/testsuite/gdb.mi/mi-catch-load-so.c
new file mode 100755
index 0000000..8c3d0d6
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-catch-load-so.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2012 Free Software Foundation, Inc.
+
+   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/>.  */
+
+int
+f (void)
+{
+  return 23;
+}
diff --git a/gdb/testsuite/gdb.mi/mi-catch-load.c b/gdb/testsuite/gdb.mi/mi-catch-load.c
new file mode 100755
index 0000000..0035c88
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-catch-load.c
@@ -0,0 +1,33 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2012 Free Software Foundation, Inc.
+
+   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 <dlfcn.h>
+#include <stdio.h>
+
+char *libname = "mi-catch-load-so.so";
+
+int
+main ()
+{
+  void *h;
+
+  h = dlopen (libname, RTLD_LAZY);
+
+  dlclose (h);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.mi/mi-catch-load.exp b/gdb/testsuite/gdb.mi/mi-catch-load.exp
new file mode 100755
index 0000000..e7f9b1e
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-catch-load.exp
@@ -0,0 +1,89 @@
+# Copyright 2012 Free Software Foundation, Inc.
+# 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/>.
+#
+load_lib mi-support.exp
+
+if {[skip_shlib_tests]} {
+    return -1
+}
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+if {[get_compiler_info]} {
+    warning "Could not get compiler info"
+    untested mi-catch-load.exp
+    return -1
+}
+
+standard_testfile mi-catch-load.c
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug shlib_load}] != "" } {
+    untested mi-catch-load.exp
+    return -1
+}
+
+set testfile2 mi-catch-load-so
+set srcfile2 ${testfile2}.c
+set binfile2 ${objdir}/${subdir}/${testfile2}.so
+set binfile2_dlopen [shlib_target_file ${testfile2}.so]
+if { [gdb_compile_shlib "${srcdir}/${subdir}/${srcfile2}" ${binfile2} {debug}] != "" } {
+    untested mi-catch-load.exp
+    return -1
+}
+mi_run_to_main
+
+# test -catch-load
+mi_gdb_test "111-gdb-set auto-solib-add on" ".*" "catch-load: auto-solib-add on"
+mi_gdb_test "222-catch-load -t mi-catch-load-so.so*" ".*breakpoint-created.*type=\"catchpoint\".*" "catch-load: catch load"
+mi_gdb_test "333-exec-continue" ".*" "catch-load: run"
+
+gdb_expect {
+    -re ".*stopped.*reason=\"solib-event\".*added=.*\r\n.*\r\n$mi_gdb_prompt$" {
+        pass "catch-load: solib-event stop"
+    }
+    -re ".*\r\n$mi_gdb_prompt$" {
+        fail "catch-load: solib-event stop"
+    }
+    timeout {
+        fail "(timeout) catch-load: solib-event stop"
+    }
+}
+
+mi_gdb_exit
+
+
+if [mi_gdb_start] {
+    continue
+}
+mi_run_to_main
+
+# test -catch-unload
+mi_gdb_test "111-gdb-set auto-solib-add on" ".*" "catch-unload: auto-solib-add on"
+mi_gdb_test "222-catch-unload -t mi-catch-load-so.so*" ".*breakpoint-created.*type=\"catchpoint\".*" "catch-unload: catch unload"
+mi_gdb_test "333-exec-continue" ".*" "catch-unload: run"
+
+gdb_expect {
+    -re ".*stopped.*reason=\"solib-event\".*removed=.*\r\n.*\r\n$mi_gdb_prompt$" {
+        pass "catch-unload: solib-event stop"
+    }
+    -re ".*\r\n$mi_gdb_prompt$" {
+        fail "catch-unload: solib-event stop"
+    }
+    timeout {
+        fail "(timeout) catch-unload: solib-event stop"
+    }
+}
-- 
1.7.1

[PATCH v6 2/3] MI: document the -catch-load/-unload commands

[Mircea Gherzan]

2012-11-16 Mircea Gherzan <mircea.gherzan@intel.com>

gdb/doc:
	* gdb.texinfo (GDB/MI Catchpoint Commands): New section.

gdb/:
	* NEWS: mention the -catch-load/-catch-unload MI commands.

Signed-off-by: Mircea Gherzan <mircea.gherzan@intel.com>
---
 gdb/NEWS            |    2 +
 gdb/doc/gdb.texinfo |   70 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 3b09e5f..06ede52 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -99,6 +99,8 @@ show print type typedefs
      has been requested.
   ** New optional parameter COUNT added to the "-data-write-memory-bytes" 
      command, to allow pattern filling of memory areas.
+  ** New commands "-catch-load"/"-catch-unload" added for intercepting
+     library load/unload events.
 
 * GDB now supports the "mini debuginfo" section, .gnu_debugdata.
   You must have the LZMA library available when configuring GDB for this
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index afe3845..cadbb34 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -27139,6 +27139,7 @@ may repeat one or more times.
 * GDB/MI Simple Examples::
 * GDB/MI Command Description Format::
 * GDB/MI Breakpoint Commands::
+* GDB/MI Catchpoint Commands::
 * GDB/MI Program Context::
 * GDB/MI Thread Commands::
 * GDB/MI Ada Tasking Commands::
@@ -28713,6 +28714,75 @@ times="1"@}]@}
 (gdb)
 @end smallexample
 
+
+@c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+@node GDB/MI Catchpoint Commands
+@section @sc{gdb/mi} Catchpoint Commands
+
+This section documents @sc{gdb/mi} commands for manipulating
+catchpoints.
+
+@subheading The @code{-catch-load} Command
+@findex -catch-load
+
+@subsubheading Synopsis
+
+@smallexample
+ -catch-load [ -t ] [ -d ] @var{regexp}
+@end smallexample
+
+Add a catchpoint for library load events.  If the @samp{-t} option is used,
+the catchpoint is a temporary one (@pxref{Set Breaks, ,Setting
+Breakpoints}).  If the @samp{-d} option is used, the catchpoint is created
+in a disabled state.  The @samp{regexp} argument is a regular
+expression used to match the name of the loaded library.
+
+
+@subsubheading @value{GDBN} Command
+
+The corresponding @value{GDBN} command is @samp{catch load}.
+
+@subsubheading Example
+
+@smallexample
+-catch-load -t foo.so
+=breakpoint-created,bkpt=@{number="1",type="catchpoint",disp="del",
+enabled="y",what="load of library matching foo.so",times="0"@}
+^done
+(gdb)
+@end smallexample
+
+
+@subheading The @code{-catch-unload} Command
+@findex -catch-unload
+
+@subsubheading Synopsis
+
+@smallexample
+ -catch-unload [ -t ] [ -d ] @var{regexp}
+@end smallexample
+
+Add a catchpoint for library unload events.  If the @samp{-t} option is
+used, the catchpoint is a temporary one (@pxref{Set Breaks, ,Setting
+Breakpoints}).  If the @samp{-d} option is used, the catchpoint is
+created in a disabled state.  The @samp{regexp} argument is a regular
+expression used to match the name of the unloaded library.
+
+@subsubheading @value{GDBN} Command
+
+The corresponding @value{GDBN} command is @samp{catch unload}.
+
+@subsubheading Example
+
+@smallexample
+-catch-unload -d bar.so
+=breakpoint-created,bkpt=@{number="1",type="catchpoint",disp="keep",
+enabled="n",what="unload of library matching bar.so",times="0"@}
+^done
+(gdb)
+@end smallexample
+
+
 @c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 @node GDB/MI Program Context
 @section @sc{gdb/mi}  Program Context
-- 
1.7.1

Re: [PATCH v6 2/3] MI: document the -catch-load/-unload commands

[Eli Zaretskii]

> From: Mircea Gherzan <mircea.gherzan@intel.com>
> Cc: keven.boell@intel.com, mircea.gherzan@intel.com, marc.khouzam@ericsson.com,        tromey@redhat.com, vladimir@codesourcery.com
> Date: Tue, 27 Nov 2012 16:53:10 +0100
> 
> 2012-11-16 Mircea Gherzan <mircea.gherzan@intel.com>
> 
> gdb/doc:
> 	* gdb.texinfo (GDB/MI Catchpoint Commands): New section.
> 
> gdb/:
> 	* NEWS: mention the -catch-load/-catch-unload MI commands.

Thanks, this is fine.

Re: [PATCH v6 1/3] MI: add the -catch-load and -catch-unload commands

[Tom Tromey]

>>>>> "Mircea" == Mircea Gherzan <mircea.gherzan@intel.com> writes:

Mircea> They are equivalent to "catch load" and "catch unload" from CLI.
Mircea> Rationale: GUIs might be interested in catching solib load or
Mircea> unload events.

Thanks.

Mircea> +/* Shared helper function (MI and CLI) for creating and installing
Mircea> +   a shared object event catchpoint.  */

Mircea> -static void
Mircea> -catch_load_or_unload (char *arg, int from_tty, int is_load,
Mircea> -		      struct cmd_list_element *command)
Mircea> +void
Mircea> +add_solib_catchpoint (char *arg, int is_load, int is_temp, int enabled)

More documentation for the arguments would be nice.

I wouldn't mind hearing Pedro's thoughts on this change, particularly
the breakpoint part.

Mircea> +  DEF_MI_CMD_MI ("catch-load", mi_cmd_catch_load),
Mircea> +  DEF_MI_CMD_MI ("catch-unload", mi_cmd_catch_unload),

I notice that these don't suppress the async breakpoint notification.

It seems more consistent to me to suppress the notification and then
print the new breakpoint information as the result of this command.
I'm curious to know your rationale for this approach.

Tom

Re: [PATCH v6 3/3] MI: tests for -catch-load/-catch-unload

[Tom Tromey]

>>>>> "Mircea" == Mircea Gherzan <mircea.gherzan@intel.com> writes:

Mircea> +mi_gdb_test "333-exec-continue" ".*" "catch-unload: run"
Mircea> +
Mircea> +gdb_expect {
Mircea> +    -re ".*stopped.*reason=\"solib-event\".*removed=.*\r\n.*\r\n$mi_gdb_prompt$" {
Mircea> +        pass "catch-unload: solib-event stop"
Mircea> +    }
Mircea> +    -re ".*\r\n$mi_gdb_prompt$" {
Mircea> +        fail "catch-unload: solib-event stop"
Mircea> +    }
Mircea> +    timeout {
Mircea> +        fail "(timeout) catch-unload: solib-event stop"
Mircea> +    }
Mircea> +}

I am not sure but this kind of thing seems racy to me.
Can't that ".*" in the mi_gdb_test suck up an arbitrary amount of gdb
output, perhaps enough to cause gdb_expect not to see the data it needs
to match?
I think some other approach is needed.

Tom

Re: [PATCH v6 1/3] MI: add the -catch-load and -catch-unload commands

[Mircea Gherzan]

Thanks for your review.

On 27.11.2012 20:05, Tom Tromey wrote:
> I wouldn't mind hearing Pedro's thoughts on this change, particularly
> the breakpoint part.
>
> Mircea> +  DEF_MI_CMD_MI ("catch-load", mi_cmd_catch_load),
> Mircea> +  DEF_MI_CMD_MI ("catch-unload", mi_cmd_catch_unload),
>
> I notice that these don't suppress the async breakpoint notification.
>
> It seems more consistent to me to suppress the notification and then
> print the new breakpoint information as the result of this command.

This has been fixed in v7 of the series.

Mircea

-- 

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Peter Gleissner, Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH v6 1/3] MI: add the -catch-load and -catch-unload commands

[Tom Tromey]

>>>>> "Mircea" == Mircea Gherzan <mircea.gherzan@intel.com> writes:

>> It seems more consistent to me to suppress the notification and then
>> print the new breakpoint information as the result of this command.

Mircea> This has been fixed in v7 of the series.

There were some other review comments you haven't addressed yet: more
comments for the one function in breakpoint.c, and possible raciness in
the test case.

Tom

Re: [PATCH v6 1/3] MI: add the -catch-load and -catch-unload commands

[Mircea Gherzan]

Am 29.11.2012 22:08, schrieb Tom Tromey:
>>>>>> "Mircea" == Mircea Gherzan <mircea.gherzan@intel.com> writes:
> 
>>> It seems more consistent to me to suppress the notification and then
>>> print the new breakpoint information as the result of this command.
> 
> Mircea> This has been fixed in v7 of the series.
> 
> There were some other review comments you haven't addressed yet: more
> comments for the one function in breakpoint.c, and possible raciness in
> the test case.

The possible raciness has also been addressed in v7, since no more async
notifications are present: we just use the ^done reply to carry the
catchpoint info. Furthermore, "mi_send_resuming_command" is used now.

I just wanted to collect some feedback regarding the suppression of the
async notifications before sending v8 (hopefully the last iteration).


-- 
Mircea

Re: [PATCH v6 1/3] MI: add the -catch-load and -catch-unload commands

[Tom Tromey]

>>>>> "Mircea" == Mircea Gherzan <mgherzan@gmail.com> writes:

Mircea> The possible raciness has also been addressed in v7, since no more async
Mircea> notifications are present: we just use the ^done reply to carry the
Mircea> catchpoint info. Furthermore, "mi_send_resuming_command" is used now.

Thanks.

Mircea> I just wanted to collect some feedback regarding the suppression of the
Mircea> async notifications before sending v8 (hopefully the last iteration).

I'll look at it today.

Tom

Re: [PATCH v6 1/3] MI: add the -catch-load and -catch-unload commands

[Tom Tromey]

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

Mircea> I just wanted to collect some feedback regarding the suppression of the
Mircea> async notifications before sending v8 (hopefully the last iteration).

Tom> I'll look at it today.

The approach looks reasonable enough to me, but Vladimir's critique also
seems on-point to me.

thanks,
Tom

Re: [PATCH v6 1/3] MI: add the -catch-load and -catch-unload commands

[Mircea Gherzan]

On 30.11.2012 19:38, Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:
>
> Mircea> I just wanted to collect some feedback regarding the suppression of the
> Mircea> async notifications before sending v8 (hopefully the last iteration).
>
> Tom> I'll look at it today.
>
> The approach looks reasonable enough to me, but Vladimir's critique also
> seems on-point to me.

This has been dealt with in v8 of the series.

Thanks,
Mircea


-- 

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052