[gdb/testsuite] Don't expect gdb_prompt in mi_skip_python_test

Message ID 20190727175221.GA9077@delia
State New, archived
Headers

Commit Message

Tom de Vries July 27, 2019, 5:52 p.m. UTC
  Hi,

During an openSUSE package build using 8.3 sources, I ran into this error
with gdb.python/py-mi-events.exp:
...
python print ('test')^M
&"python print ('test')\n"^M
~"test\n"^M
^done^M
(gdb) ^M
python print (sys.version_info[0])^M
&"python print (sys.version_info[0])\n"^M
~"3\n"^M
^done^M
(gdb) FAIL: gdb.python/py-mi-events.exp: check if python 3
ERROR: tcl error sourcing gdb-8.3/gdb/testsuite/gdb.python/py-mi-events.exp.
ERROR: can't read "gdb_py_is_py3k": no such variable
    while executing
"if { $gdb_py_is_py3k == 0 } {
        gdb_test_multiple "python print (sys.version_info\[1\])" \
	  "check if python 2.4" {
            -re "\[45\].*$prompt_regex..."
    (procedure "skip_python_tests_prompt" line 22)
    invoked from within
"skip_python_tests_prompt "$mi_gdb_prompt$""
    (procedure "mi_skip_python_tests" line 3)
    invoked from within
...

I managed to reproduce the error on 8.3 using a trigger patch:
...
  

Comments

Kevin Buettner July 27, 2019, 9:05 p.m. UTC | #1
On Sat, 27 Jul 2019 19:52:22 +0200
Tom de Vries <tdevries@suse.de> wrote:

> 	PR gdb/24855
> 	* lib/gdb.exp (gdb_test_multiple): Add prompt_regexp parameter.
> 	(skip_python_tests_prompt): Add prompt_regexp argument to
> 	gdb_test_multiple calls.

Your fix looks reasonable to me, but please wait a few days for
others to comment.

Kevin
  
Simon Marchi July 28, 2019, 3:18 a.m. UTC | #2
On 2019-07-27 1:52 p.m., Tom de Vries wrote:
> Hi,
> 
> During an openSUSE package build using 8.3 sources, I ran into this error
> with gdb.python/py-mi-events.exp:
> ...
> python print ('test')^M
> &"python print ('test')\n"^M
> ~"test\n"^M
> ^done^M
> (gdb) ^M
> python print (sys.version_info[0])^M
> &"python print (sys.version_info[0])\n"^M
> ~"3\n"^M
> ^done^M
> (gdb) FAIL: gdb.python/py-mi-events.exp: check if python 3
> ERROR: tcl error sourcing gdb-8.3/gdb/testsuite/gdb.python/py-mi-events.exp.
> ERROR: can't read "gdb_py_is_py3k": no such variable
>     while executing
> "if { $gdb_py_is_py3k == 0 } {
>         gdb_test_multiple "python print (sys.version_info\[1\])" \
> 	  "check if python 2.4" {
>             -re "\[45\].*$prompt_regex..."
>     (procedure "skip_python_tests_prompt" line 22)
>     invoked from within
> "skip_python_tests_prompt "$mi_gdb_prompt$""
>     (procedure "mi_skip_python_tests" line 3)
>     invoked from within
> ...
> 
> I managed to reproduce the error on 8.3 using a trigger patch:
> ...
> diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
> index 3e9f36897a..7be00ef9e8 100644
> --- a/gdb/mi/mi-interp.c
> +++ b/gdb/mi/mi-interp.c
> @@ -38,6 +38,7 @@
>  #include "cli-out.h"
>  #include "thread-fsm.h"
>  #include "cli/cli-interp.h"
> +#include "gdb_usleep.h"
> 
>  /* These are the interpreter setup, etc. functions for the MI
>     interpreter.  */
> @@ -96,7 +97,10 @@ display_mi_prompt (struct mi_interp *mi)
>  {
>    struct ui *ui = current_ui;
> 
> -  fputs_unfiltered ("(gdb) \n", mi->raw_stdout);
> +  fputs_unfiltered ("(gdb) ", mi->raw_stdout);
> +  gdb_flush (mi->raw_stdout);
> +  gdb_usleep (1000000);
> +  fputs_unfiltered ("\n", mi->raw_stdout);
>    gdb_flush (mi->raw_stdout);
>    ui->prompt_state = PROMPTED;
>  }
> ...
> 
> The error happens as follows.
> 
> On one hand, skip_python_tests_prompt uses the prompt_regexp parameter for the
> user_code argument of gdb_test_multiple:
> ...
> proc skip_python_tests_prompt { prompt_regexp } {
>     global gdb_py_is_py3k
> 
>     ...
> 
>     gdb_test_multiple "python print (sys.version_info\[0\])" "check if python 3" {
> 	-re "3.*$prompt_regexp" {
>             set gdb_py_is_py3k 1
>         }
> 	-re ".*$prompt_regexp" {
>             set gdb_py_is_py3k 0
>         }
>     }
> ...
> 
> On the otherhand, gdb_test_multiple itself uses $gdb_prompt:
> ...
>          -re "\r\n$gdb_prompt $" {
>             if ![string match "" $message] then {
>                 fail "$message"
>             }
>             set result 1
>         }
> ...
> 
> So when mi_skip_python_test calls skip_python_tests_prompt with prompt_regexp
> set to $mi_gdb_prompt:
> ...
> proc mi_skip_python_tests {} {
>     global mi_gdb_prompt
>     return [skip_python_tests_prompt "$mi_gdb_prompt$"]
> }
> ...
> and expect reads "(gdb) " (due to the trigger patch) and tries to match it,
> the user_code regexps using $prompt_regexp (set to $mi_gdb_prompt) don't match,
> but the $gdb_prompt regexp in gdb_test_multiple does match.
> 
> Fix this by adding a prompt_regexp parameter to gdb_test_multiple, and using the
> parameter in skip_python_tests_prompt.
> 
> Tested gdb.python/py-mi-events.exp in combination with trigger patch on
> x86_64-linux.
> 
> Tested on x86_64-linux.
> 
> OK for trunk?
> 
> [
> - There may be mi test-cases that use gdb_test_multiple that also need
>   fixing, but I'd rather get agreement on the fix first.
> - I initially tried to fix this by adding a gdb_test_multiple_prompt but I
>   didn't get that working.  I can post a follow-up patch doing the refactoring
>   for that if required, but I rather handle that in a separate patch, since
>   this seems to be non-trivial. ]

Hi Tom,

It seems like this failure is reproducible using "make check-read1".  If you haven't
seen that before, check it out in gdb/testsuite/README, I'm sure you'll find dozens
of other testsuite bugs with it :).

As for the issue itself, I guess the root problem is that we're using gdb_test_multiple, which
was meant for the CLI, in an MI setting.  I was afraid that the regexp made to match the various
errors, such as:

  -re "Undefined\[a-z\]* command:.*$gdb_prompt $"

wouldn't work in MI, but the way they are written, they should match this, for example:

  hello
  &"hello\n"
  &"Undefined command: \"hello\".  Try \"help\".\n"
  ^error,msg="Undefined command: \"hello\".  Try \"help\"."
  (gdb)

So the fix looks reasonable to me too (at least, I can't think of anything better at the moment).

Thanks,

Simon
  

Patch

diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 3e9f36897a..7be00ef9e8 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -38,6 +38,7 @@ 
 #include "cli-out.h"
 #include "thread-fsm.h"
 #include "cli/cli-interp.h"
+#include "gdb_usleep.h"

 /* These are the interpreter setup, etc. functions for the MI
    interpreter.  */
@@ -96,7 +97,10 @@  display_mi_prompt (struct mi_interp *mi)
 {
   struct ui *ui = current_ui;

-  fputs_unfiltered ("(gdb) \n", mi->raw_stdout);
+  fputs_unfiltered ("(gdb) ", mi->raw_stdout);
+  gdb_flush (mi->raw_stdout);
+  gdb_usleep (1000000);
+  fputs_unfiltered ("\n", mi->raw_stdout);
   gdb_flush (mi->raw_stdout);
   ui->prompt_state = PROMPTED;
 }
...

The error happens as follows.

On one hand, skip_python_tests_prompt uses the prompt_regexp parameter for the
user_code argument of gdb_test_multiple:
...
proc skip_python_tests_prompt { prompt_regexp } {
    global gdb_py_is_py3k

    ...

    gdb_test_multiple "python print (sys.version_info\[0\])" "check if python 3" {
	-re "3.*$prompt_regexp" {
            set gdb_py_is_py3k 1
        }
	-re ".*$prompt_regexp" {
            set gdb_py_is_py3k 0
        }
    }
...

On the otherhand, gdb_test_multiple itself uses $gdb_prompt:
...
         -re "\r\n$gdb_prompt $" {
            if ![string match "" $message] then {
                fail "$message"
            }
            set result 1
        }
...

So when mi_skip_python_test calls skip_python_tests_prompt with prompt_regexp
set to $mi_gdb_prompt:
...
proc mi_skip_python_tests {} {
    global mi_gdb_prompt
    return [skip_python_tests_prompt "$mi_gdb_prompt$"]
}
...
and expect reads "(gdb) " (due to the trigger patch) and tries to match it,
the user_code regexps using $prompt_regexp (set to $mi_gdb_prompt) don't match,
but the $gdb_prompt regexp in gdb_test_multiple does match.

Fix this by adding a prompt_regexp parameter to gdb_test_multiple, and using the
parameter in skip_python_tests_prompt.

Tested gdb.python/py-mi-events.exp in combination with trigger patch on
x86_64-linux.

Tested on x86_64-linux.

OK for trunk?

[
- There may be mi test-cases that use gdb_test_multiple that also need
  fixing, but I'd rather get agreement on the fix first.
- I initially tried to fix this by adding a gdb_test_multiple_prompt but I
  didn't get that working.  I can post a follow-up patch doing the refactoring
  for that if required, but I rather handle that in a separate patch, since
  this seems to be non-trivial. ]

Thanks,
- Tom

[gdb/testsuite] Don't expect gdb_prompt in mi_skip_python_test

gdb/testsuite/ChangeLog:

2019-07-27  Tom de Vries  <tdevries@suse.de>

	PR gdb/24855
	* lib/gdb.exp (gdb_test_multiple): Add prompt_regexp parameter.
	(skip_python_tests_prompt): Add prompt_regexp argument to
	gdb_test_multiple calls.

---
 gdb/testsuite/lib/gdb.exp | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 54cee2ec41..4c7d3b5546 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -695,7 +695,7 @@  proc gdb_internal_error_resync {} {
 }
 
 
-# gdb_test_multiple COMMAND MESSAGE EXPECT_ARGUMENTS
+# gdb_test_multiple COMMAND MESSAGE EXPECT_ARGUMENTS PROMPT_REGEXP
 # Send a command to gdb; test the result.
 #
 # COMMAND is the command to execute, send to GDB with send_gdb.  If
@@ -707,6 +707,8 @@  proc gdb_internal_error_resync {} {
 #   context; action elements will be executed in the caller's context.
 #   Unlike patterns for gdb_test, these patterns should generally include
 #   the final newline and prompt.
+# PROMPT_REGEXP is a regexp matching the expected prompt after the command
+#   output.  If empty, defaults to "$gdb_prompt $"
 #
 # Returns:
 #    1 if the test failed, according to a built-in failure pattern
@@ -744,7 +746,7 @@  proc gdb_internal_error_resync {} {
 # expected from $gdb_spawn_id.  IOW, callers do not need to worry
 # about resetting "-i" back to $gdb_spawn_id explicitly.
 #
-proc gdb_test_multiple { command message user_code } {
+proc gdb_test_multiple { command message user_code { prompt_regexp "" } } {
     global verbose use_gdb_stub
     global gdb_prompt pagination_prompt
     global GDB
@@ -754,6 +756,10 @@  proc gdb_test_multiple { command message user_code } {
     upvar expect_out expect_out
     global any_spawn_id
 
+    if { "$prompt_regexp" == "" } {
+	set prompt_regexp "$gdb_prompt $"
+    }
+
     if { $message == "" } {
 	set message $command
     }
@@ -913,7 +919,7 @@  proc gdb_test_multiple { command message user_code } {
     }
 
     append code {
-	-re "Ending remote debugging.*$gdb_prompt $" {
+	-re "Ending remote debugging.*$prompt_regexp" {
 	    if ![isnative] then {
 		warning "Can`t communicate to remote target."
 	    }
@@ -921,17 +927,17 @@  proc gdb_test_multiple { command message user_code } {
 	    gdb_start
 	    set result -1
 	}
-	-re "Undefined\[a-z\]* command:.*$gdb_prompt $" {
+	-re "Undefined\[a-z\]* command:.*$prompt_regexp" {
 	    perror "Undefined command \"$command\"."
 	    fail "$message"
 	    set result 1
 	}
-	-re "Ambiguous command.*$gdb_prompt $" {
+	-re "Ambiguous command.*$prompt_regexp" {
 	    perror "\"$command\" is not a unique command name."
 	    fail "$message"
 	    set result 1
 	}
-	-re "$inferior_exited_re with code \[0-9\]+.*$gdb_prompt $" {
+	-re "$inferior_exited_re with code \[0-9\]+.*$prompt_regexp" {
 	    if ![string match "" $message] then {
 		set errmsg "$message (the program exited)"
 	    } else {
@@ -940,7 +946,7 @@  proc gdb_test_multiple { command message user_code } {
 	    fail "$errmsg"
 	    set result -1
 	}
-	-re "$inferior_exited_re normally.*$gdb_prompt $" {
+	-re "$inferior_exited_re normally.*$prompt_regexp" {
 	    if ![string match "" $message] then {
 		set errmsg "$message (the program exited)"
 	    } else {
@@ -949,7 +955,7 @@  proc gdb_test_multiple { command message user_code } {
 	    fail "$errmsg"
 	    set result -1
 	}
-	-re "The program is not being run.*$gdb_prompt $" {
+	-re "The program is not being run.*$prompt_regexp" {
 	    if ![string match "" $message] then {
 		set errmsg "$message (the program is no longer running)"
 	    } else {
@@ -958,7 +964,7 @@  proc gdb_test_multiple { command message user_code } {
 	    fail "$errmsg"
 	    set result -1
 	}
-	-re "\r\n$gdb_prompt $" {
+	-re "\r\n$prompt_regexp" {
 	    if ![string match "" $message] then {
 		fail "$message"
 	    }
@@ -972,13 +978,13 @@  proc gdb_test_multiple { command message user_code } {
 	}
 	-re "\\((y or n|y or \\\[n\\\]|\\\[y\\\] or n)\\) " {
 	    send_gdb "n\n" answer
-	    gdb_expect -re "$gdb_prompt $"
+	    gdb_expect -re "$prompt_regexp"
 	    fail "$message (got interactive prompt)"
 	    set result -1
 	}
 	-re "\\\[0\\\] cancel\r\n\\\[1\\\] all.*\r\n> $" {
 	    send_gdb "0\n"
-	    gdb_expect -re "$gdb_prompt $"
+	    gdb_expect -re "$prompt_regexp"
 	    fail "$message (got breakpoint menu)"
 	    set result -1
 	}
@@ -1869,7 +1875,7 @@  proc skip_python_tests_prompt { prompt_regexp } {
 	    return 1
 	}
 	-re "$prompt_regexp" {}
-    }
+    } "$prompt_regexp"
 
     gdb_test_multiple "python print (sys.version_info\[0\])" "check if python 3" {
 	-re "3.*$prompt_regexp" {
@@ -1878,7 +1884,7 @@  proc skip_python_tests_prompt { prompt_regexp } {
 	-re ".*$prompt_regexp" {
             set gdb_py_is_py3k 0
         }
-    }
+    } "$prompt_regexp"
 
     return 0
 }