bug in mi when setting breakpoint

[Dodji Seketeli <dodji@seketeli.org>]

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

Hello,

With the mi2 interpreter, when one issues a break command
(either '-break-insert <classname::function-name>' or the cli
'break <classname::function-name>' command)
in the context of a C++ inferior, an overloaded
function resolution can happen.

In that case, the breakpoint setting code
asks the user to choose the overloaded function it wants to break in.
To do so, the breakpoint setting code displays something like:

~"[0] cancel\n[1] all\n"
~"[2] classname::function_name(int) at fooprog.cc:65\n"
~"[3] classname::function_name() at fooprog.cc:59\n"
~"> "

The last line of this "question" is the default prompt indicating the end
of the question.

In gdb 6.7.1, that prompt is missing *only* when using the MI interpreter.
It is present in the CLI interpreter. And this is a regression from 6.6
where the prompt was present with both interpreters.

The prompt is really important for graphical front-end tools willing
to parse that "question" so that they can display display it
back to the user in a nice windowed manner.
As the question does not really respect the GDB/MI output format where the
output should be ended by a "(gdb)" string, the prompt is the only way th
front end can detect the end of the "question".

So I tried to produce the attached patch to pinpoint the problem and hopefully
propose a fix.

Cheers,

-- 
Dodji Seketeli
http://www.seketeli.org/dodji


[-- Attachment #2: fix-prompt.patch --]
[-- Type: text/x-diff, Size: 6272 bytes --]


The spirit of the patch is to enable the prompt when using the MI
interpreter. I tried to enable the prompt for both "-break-insert"
and "break" commands because the "break" command offers features that the
"-break-insert" command does not offer yet, so it is very useful, even in an
MI context.

The patch applies to the gdb-6.7.1 tarball.

Index: gdb-6.7.1/gdb/interps.c
===================================================================
--- gdb-6.7.1.orig/gdb/interps.c	2007-12-16 08:56:50.000000000 +0100
+++ gdb-6.7.1/gdb/interps.c	2007-12-16 10:13:50.000000000 +0100
@@ -289,10 +289,13 @@
     return current_interpreter->quiet_p;
 }
 
-static int
+int
 interp_set_quiet (struct interp *interp, int quiet)
 {
-  int old_val = interp->quiet_p;
+  int old_val = 0;
+  if (!interp)
+    interp = current_interpreter;
+  old_val = interp->quiet_p;
   interp->quiet_p = quiet;
   return old_val;
 }
Index: gdb-6.7.1/gdb/interps.h
===================================================================
--- gdb-6.7.1.orig/gdb/interps.h	2007-12-16 08:55:24.000000000 +0100
+++ gdb-6.7.1/gdb/interps.h	2007-12-16 09:03:11.000000000 +0100
@@ -34,6 +34,7 @@
 extern struct gdb_exception interp_exec (struct interp *interp,
 					 const char *command);
 extern int interp_quiet_p (struct interp *interp);
+int interp_set_quiet (struct interp *interp, int quiet);
 
 typedef void *(interp_init_ftype) (void);
 typedef int (interp_resume_ftype) (void *data);
Index: gdb-6.7.1/gdb/mi/mi-cmd-break.c
===================================================================
--- gdb-6.7.1.orig/gdb/mi/mi-cmd-break.c	2007-12-16 10:04:45.000000000 +0100
+++ gdb-6.7.1/gdb/mi/mi-cmd-break.c	2007-12-16 10:36:11.000000000 +0100
@@ -26,6 +26,7 @@
 #include "mi-getopt.h"
 #include "gdb-events.h"
 #include "gdb.h"
+#include "interps.h"
 
 enum
   {
@@ -76,6 +77,7 @@
   char *condition = NULL;
   enum gdb_rc rc;
   struct gdb_events *old_hooks;
+  int was_cur_interp_quiet = 0;
   enum opt
     {
       HARDWARE_OPT, TEMP_OPT /*, REGEXP_OPT */ , CONDITION_OPT,
@@ -133,6 +135,14 @@
 
   /* Now we have what we need, let's insert the breakpoint! */
   old_hooks = deprecated_set_gdb_event_hooks (&breakpoint_hooks);
+
+  /*
+   * don't quiet the current interpreter because otherwise,
+   * if the user is asked (by the breakpoint setting code) to resolve
+   * function overload ambiguity, the question won't be followed by a
+   * prompt.
+   */
+  was_cur_interp_quiet = interp_set_quiet (NULL, 0);
   switch (type)
     {
     case REG_BP:
@@ -161,6 +171,8 @@
 		      _("mi_cmd_break_insert: Bad switch."));
     }
   deprecated_set_gdb_event_hooks (old_hooks);
+  /*set the interpreter back to its previous quiet-ness state*/
+  interp_set_quiet (NULL, was_cur_interp_quiet);
 
   if (rc == GDB_RC_FAIL)
     return MI_CMD_ERROR;
Index: gdb-6.7.1/gdb/mi/mi-interp.c
===================================================================
--- gdb-6.7.1.orig/gdb/mi/mi-interp.c	2007-12-16 09:57:56.000000000 +0100
+++ gdb-6.7.1/gdb/mi/mi-interp.c	2007-12-16 10:33:33.000000000 +0100
@@ -31,6 +31,14 @@
 #include "mi-out.h"
 #include "mi-console.h"
 
+/*
+ * this is a global variable
+ * to detect if we are in the context
+ * of setting a breakpoint using a cli command, while being initially
+ * in an MI interpreter.
+ */
+extern int in_cli_break_context;
+
 struct mi_interp
 {
   /* MI's output channels */
@@ -156,7 +164,10 @@
 static int
 mi_interpreter_prompt_p (void *data)
 {
-  return 0;
+  if (interp_quiet_p (NULL)) {
+    return 0;
+  }
+  return 1;
 }
 
 static void
@@ -185,6 +196,7 @@
   enum mi_cmd_result result = MI_CMD_DONE;
   int i;
   struct interp_procs *procs;
+  int was_interp_quiet = 0;
 
   if (argc < 2)
     {
@@ -199,6 +211,17 @@
       return MI_CMD_ERROR;
     }
 
+  /*
+   * if we are executing a cli "break" command, don't be quiet.
+   * Otherwise, c++ functions overload resolution questions asked to the
+   * user won't be followed by the mandatory prompt.
+   */
+  if (in_cli_break_context)
+    {
+      /*save the previouse quiet-ness status*/
+      was_interp_quiet = interp_set_quiet (interp_to_use, 0);
+    }
+
   if (!interp_exec_p (interp_to_use))
     {
       mi_error_message = xstrprintf ("mi_cmd_interpreter_exec: interpreter \"%s\" does not support command execution",
@@ -206,6 +229,7 @@
       return MI_CMD_ERROR;
     }
 
+
   /* Insert the MI out hooks, making sure to also call the interpreter's hooks
      if it has any. */
   /* KRS: We shouldn't need this... Events should be installed and they should
@@ -235,8 +259,15 @@
       sync_execution = 0;
     }
 
+  if (in_cli_break_context)
+    {
+      /*restore the previouse quiet-ness status*/
+      interp_set_quiet (interp_to_use, was_interp_quiet);
+    }
+
   mi_remove_notify_hooks ();
 
+
   /* Okay, now let's see if the command set the inferior going...
      Tricky point - have to do this AFTER resetting the interpreter, since
      changing the interpreter will clear out all the continuations for
Index: gdb-6.7.1/gdb/mi/mi-main.c
===================================================================
--- gdb-6.7.1.orig/gdb/mi/mi-main.c	2007-12-16 09:41:12.000000000 +0100
+++ gdb-6.7.1/gdb/mi/mi-main.c	2007-12-16 10:33:08.000000000 +0100
@@ -120,6 +120,14 @@
 static void print_diff_now (struct mi_timestamp *start);
 static void print_diff (struct mi_timestamp *start, struct mi_timestamp *end);
 
+/*
+ * this is a global variable
+ * to detect if we are in the context
+ * of setting a breakpoint using a cli command, while being initially
+ * in an MI interpreter.
+ */
+int in_cli_break_context;
+
 enum mi_cmd_result
 mi_cmd_gdb_exit (char *command, char **argv, int argc)
 {
@@ -1170,7 +1178,16 @@
 	/* Call the "console" interpreter.  */
 	argv[0] = "console";
 	argv[1] = context->command;
+	if (context->command && (strncmp (context->command, "b", 1) == 0))
+	  {
+	    /*
+	     * we are being asked to execute a break command, using the cli
+	     * interpreter, while being in a MI interpreter.
+	     */
+	    in_cli_break_context = 1;
+	  }
 	args->rc = mi_cmd_interpreter_exec ("-interpreter-exec", argv, 2);
+	in_cli_break_context = 0;
 
 	/* If we changed interpreters, DON'T print out anything.  */
 	if (current_interp_named_p (INTERP_MI)