[patch] Fix "interpreter-exec mi" output causing false FAILs

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[patch] Fix "interpreter-exec mi" output causing false FAILs

Jan Kratochvil-2
Hi,

(gdb) interpreter-exec mi -break-list
^done,BreakpointTable={[...]}
(gdb)
(gdb) q

which can cause false
        FAIL: gdb.dwarf2/dw2-filename.exp: info sources
with "read1" from
        reproducer for races of expect incomplete reads
        http://sourceware.org/bugzilla/show_bug.cgi?id=12649

No regressions on {x86_64,x86_64-m32,i686}-fedora17-linux-gnu.


Thanks,
Jan


2012-03-07  Jan Kratochvil  <[hidden email]>

        Fix double prompt of 'interpreter-exec mi'.
        * mi/mi-interp.c (mi_execute_command_input_handler): New prototype.
        (mi_interpreter_resume): use it.
        (mi_execute_command_input_handler): New function.
        * mi/mi-main.c (mi_execute_command): Move prompt printing to
        mi_execute_command_input_handler.

--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -40,6 +40,7 @@
    interpreter.  */
 
 static void mi_execute_command_wrapper (char *cmd);
+static void mi_execute_command_input_handler (char *cmd);
 static void mi_command_loop (int mi_version);
 
 /* These are hooks that we put in place while doing interpreter_exec
@@ -151,7 +152,7 @@ mi_interpreter_resume (void *data)
   /* These overwrite some of the initialization done in
      _intialize_event_loop.  */
   call_readline = gdb_readline2;
-  input_handler = mi_execute_command_wrapper;
+  input_handler = mi_execute_command_input_handler;
   add_file_handler (input_fd, stdin_event_handler, 0);
   async_command_editing_p = 0;
   /* FIXME: This is a total hack for now.  PB's use of the MI
@@ -297,6 +298,17 @@ mi_execute_command_wrapper (char *cmd)
   mi_execute_command (cmd, stdin == instream);
 }
 
+/* mi_execute_command_wrapper wrapper suitable for INPUT_HANDLER.  */
+
+static void
+mi_execute_command_input_handler (char *cmd)
+{
+  mi_execute_command_wrapper (cmd);
+
+  fputs_unfiltered ("(gdb) \n", raw_stdout);
+  gdb_flush (raw_stdout);
+}
+
 static void
 mi1_command_loop (void)
 {
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2027,9 +2027,6 @@ mi_execute_command (char *cmd, int from_tty)
 
       mi_parse_free (command);
     }
-
-  fputs_unfiltered ("(gdb) \n", raw_stdout);
-  gdb_flush (raw_stdout);
 }
 
 static void
Reply | Threaded
Open this post in threaded view
|

Re: [patch] Fix "interpreter-exec mi" output causing false FAILs

Joel Brobecker
Hey guys,

I just noticed this email sitting in my gdb-patches mbox, and I am
wondering if it has been reviewed?


On Wed, Mar 07, 2012 at 10:15:34AM +0100, Jan Kratochvil wrote:

> Hi,
>
> (gdb) interpreter-exec mi -break-list
> ^done,BreakpointTable={[...]}
> (gdb)
> (gdb) q
>
> which can cause false
> FAIL: gdb.dwarf2/dw2-filename.exp: info sources
> with "read1" from
> reproducer for races of expect incomplete reads
> http://sourceware.org/bugzilla/show_bug.cgi?id=12649
>
> No regressions on {x86_64,x86_64-m32,i686}-fedora17-linux-gnu.
>
>
> Thanks,
> Jan
>
>
> 2012-03-07  Jan Kratochvil  <[hidden email]>
>
> Fix double prompt of 'interpreter-exec mi'.
> * mi/mi-interp.c (mi_execute_command_input_handler): New prototype.
> (mi_interpreter_resume): use it.
> (mi_execute_command_input_handler): New function.
> * mi/mi-main.c (mi_execute_command): Move prompt printing to
> mi_execute_command_input_handler.
>
> --- a/gdb/mi/mi-interp.c
> +++ b/gdb/mi/mi-interp.c
> @@ -40,6 +40,7 @@
>     interpreter.  */
>  
>  static void mi_execute_command_wrapper (char *cmd);
> +static void mi_execute_command_input_handler (char *cmd);
>  static void mi_command_loop (int mi_version);
>  
>  /* These are hooks that we put in place while doing interpreter_exec
> @@ -151,7 +152,7 @@ mi_interpreter_resume (void *data)
>    /* These overwrite some of the initialization done in
>       _intialize_event_loop.  */
>    call_readline = gdb_readline2;
> -  input_handler = mi_execute_command_wrapper;
> +  input_handler = mi_execute_command_input_handler;
>    add_file_handler (input_fd, stdin_event_handler, 0);
>    async_command_editing_p = 0;
>    /* FIXME: This is a total hack for now.  PB's use of the MI
> @@ -297,6 +298,17 @@ mi_execute_command_wrapper (char *cmd)
>    mi_execute_command (cmd, stdin == instream);
>  }
>  
> +/* mi_execute_command_wrapper wrapper suitable for INPUT_HANDLER.  */
> +
> +static void
> +mi_execute_command_input_handler (char *cmd)
> +{
> +  mi_execute_command_wrapper (cmd);
> +
> +  fputs_unfiltered ("(gdb) \n", raw_stdout);
> +  gdb_flush (raw_stdout);
> +}
> +
>  static void
>  mi1_command_loop (void)
>  {
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -2027,9 +2027,6 @@ mi_execute_command (char *cmd, int from_tty)
>  
>        mi_parse_free (command);
>      }
> -
> -  fputs_unfiltered ("(gdb) \n", raw_stdout);
> -  gdb_flush (raw_stdout);
>  }
>  
>  static void

--
Joel
Reply | Threaded
Open this post in threaded view
|

Re: [patch] Fix "interpreter-exec mi" output causing false FAILs

Vladimir Prus-3
On 14/03/12 05:33, Joel Brobecker wrote:
> Hey guys,
>
> I just noticed this email sitting in my gdb-patches mbox, and I am
> wondering if it has been reviewed?

I don't know whether it had been reviewed -- but it looks just fine to me.

> On Wed, Mar 07, 2012 at 10:15:34AM +0100, Jan Kratochvil wrote:
>> Hi,
>>
>> (gdb) interpreter-exec mi -break-list
>> ^done,BreakpointTable={[...]}
>> (gdb)
>> (gdb) q
>>
>> which can cause false
>> FAIL: gdb.dwarf2/dw2-filename.exp: info sources
>> with "read1" from
>> reproducer for races of expect incomplete reads
>> http://sourceware.org/bugzilla/show_bug.cgi?id=12649
>>
>> No regressions on {x86_64,x86_64-m32,i686}-fedora17-linux-gnu.
>>
>>
>> Thanks,
>> Jan
>>
>>
>> 2012-03-07  Jan Kratochvil<[hidden email]>
>>
>> Fix double prompt of 'interpreter-exec mi'.
>> * mi/mi-interp.c (mi_execute_command_input_handler): New prototype.
>> (mi_interpreter_resume): use it.
>> (mi_execute_command_input_handler): New function.
>> * mi/mi-main.c (mi_execute_command): Move prompt printing to
>> mi_execute_command_input_handler.
>>
>> --- a/gdb/mi/mi-interp.c
>> +++ b/gdb/mi/mi-interp.c
>> @@ -40,6 +40,7 @@
>>      interpreter.  */
>>
>>   static void mi_execute_command_wrapper (char *cmd);
>> +static void mi_execute_command_input_handler (char *cmd);
>>   static void mi_command_loop (int mi_version);
>>
>>   /* These are hooks that we put in place while doing interpreter_exec
>> @@ -151,7 +152,7 @@ mi_interpreter_resume (void *data)
>>     /* These overwrite some of the initialization done in
>>        _intialize_event_loop.  */
>>     call_readline = gdb_readline2;
>> -  input_handler = mi_execute_command_wrapper;
>> +  input_handler = mi_execute_command_input_handler;
>>     add_file_handler (input_fd, stdin_event_handler, 0);
>>     async_command_editing_p = 0;
>>     /* FIXME: This is a total hack for now.  PB's use of the MI
>> @@ -297,6 +298,17 @@ mi_execute_command_wrapper (char *cmd)
>>     mi_execute_command (cmd, stdin == instream);
>>   }
>>
>> +/* mi_execute_command_wrapper wrapper suitable for INPUT_HANDLER.  */
>> +
>> +static void
>> +mi_execute_command_input_handler (char *cmd)
>> +{
>> +  mi_execute_command_wrapper (cmd);
>> +
>> +  fputs_unfiltered ("(gdb) \n", raw_stdout);
>> +  gdb_flush (raw_stdout);
>> +}
>> +
>>   static void
>>   mi1_command_loop (void)
>>   {
>> --- a/gdb/mi/mi-main.c
>> +++ b/gdb/mi/mi-main.c
>> @@ -2027,9 +2027,6 @@ mi_execute_command (char *cmd, int from_tty)
>>
>>         mi_parse_free (command);
>>       }
>> -
>> -  fputs_unfiltered ("(gdb) \n", raw_stdout);
>> -  gdb_flush (raw_stdout);
>>   }
>>
>>   static void
>


--
Vladimir Prus
CodeSourcery / Mentor Graphics
http://www.mentor.com/embedded-software/
Reply | Threaded
Open this post in threaded view
|

[commit] [patch] Fix "interpreter-exec mi" output causing false FAILs

Jan Kratochvil-2
On Wed, 14 Mar 2012 08:25:36 +0100, Vladimir Prus wrote:
> I don't know whether it had been reviewed -- but it looks just fine to me.

Thanks.

I thought it is wrong as it does not correctly follow how prompt is set but
I did not want to go too deep from fixing a testsuite race.  But I see now it
seems to be fully correct because MI uses separate prompt.

Added a non-racy testcase.

Checked in.


Regards,
Jan


http://sourceware.org/ml/gdb-cvs/2012-03/msg00188.html

--- src/gdb/ChangeLog 2012/03/14 01:47:45 1.14008
+++ src/gdb/ChangeLog 2012/03/14 07:58:01 1.14009
@@ -1,3 +1,12 @@
+2012-03-14  Jan Kratochvil  <[hidden email]>
+
+ Fix double prompt of 'interpreter-exec mi'.
+ * mi/mi-interp.c (mi_execute_command_input_handler): New prototype.
+ (mi_interpreter_resume): use it.
+ (mi_execute_command_input_handler): New function.
+ * mi/mi-main.c (mi_execute_command): Move prompt printing to
+ mi_execute_command_input_handler.
+
 2012-03-13  Josh Matthews  <[hidden email]>  (tiny change)
 
  * darwin-nat-info.c (_initialize_darwin_info_commands): Add
--- src/gdb/testsuite/ChangeLog 2012/03/14 01:39:11 1.3136
+++ src/gdb/testsuite/ChangeLog 2012/03/14 07:58:05 1.3137
@@ -1,3 +1,7 @@
+2012-03-14  Jan Kratochvil  <[hidden email]>
+
+ * gdb.mi/mi2-prompt.exp: New file.
+
 2012-03-13  Joel Brobecker  <[hidden email]>
 
  * gdb.base/enum_cond.c, gdb.base/enum_cond.exp: New testcase.
--- src/gdb/mi/mi-interp.c 2012/03/06 22:48:53 1.72
+++ src/gdb/mi/mi-interp.c 2012/03/14 07:58:05 1.73
@@ -40,6 +40,7 @@
    interpreter.  */
 
 static void mi_execute_command_wrapper (char *cmd);
+static void mi_execute_command_input_handler (char *cmd);
 static void mi_command_loop (int mi_version);
 
 /* These are hooks that we put in place while doing interpreter_exec
@@ -151,7 +152,7 @@
   /* These overwrite some of the initialization done in
      _intialize_event_loop.  */
   call_readline = gdb_readline2;
-  input_handler = mi_execute_command_wrapper;
+  input_handler = mi_execute_command_input_handler;
   add_file_handler (input_fd, stdin_event_handler, 0);
   async_command_editing_p = 0;
   /* FIXME: This is a total hack for now.  PB's use of the MI
@@ -297,6 +298,17 @@
   mi_execute_command (cmd, stdin == instream);
 }
 
+/* mi_execute_command_wrapper wrapper suitable for INPUT_HANDLER.  */
+
+static void
+mi_execute_command_input_handler (char *cmd)
+{
+  mi_execute_command_wrapper (cmd);
+
+  fputs_unfiltered ("(gdb) \n", raw_stdout);
+  gdb_flush (raw_stdout);
+}
+
 static void
 mi1_command_loop (void)
 {
--- src/gdb/mi/mi-main.c 2012/03/13 01:16:07 1.213
+++ src/gdb/mi/mi-main.c 2012/03/14 07:58:05 1.214
@@ -2029,9 +2029,6 @@
 
       mi_parse_free (command);
     }
-
-  fputs_unfiltered ("(gdb) \n", raw_stdout);
-  gdb_flush (raw_stdout);
 }
 
 static void
--- src/gdb/testsuite/gdb.mi/mi2-prompt.exp
+++ src/gdb/testsuite/gdb.mi/mi2-prompt.exp 2012-03-14 07:59:42.074554000 +0000
@@ -0,0 +1,38 @@
+# Copyright 2011 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
+set MIFLAGS "-i=mi2"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+# Check console 'set prompt' does not affect the MI output.
+
+mi_gdb_test {-interpreter-exec console "set prompt (banana) "} {\^done} \
+    "console set prompt"
+mi_gdb_test "-break-list" ".*}" "-break-list"
+
+gdb_exit
+gdb_start
+
+# Check 'set prompt' affects console output even for "interpreter-exec mi".
+
+set gdb_prompt {\(banana\)}
+gdb_test_no_output "set prompt (banana) "
+
+gdb_test "interpreter-exec mi -break-list" "\r\n\\^done,.*}"