On 28-07-19 05:18, Simon Marchi wrote:
> 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".

It is indeed.

> If you haven't
> seen that before, check it out in gdb/testsuite/README,

Thanks for the pointer. I've rewritten the rationale to use this concept
rather than the trigger patch to reproduce the failure.

> I'm sure you'll find dozens
> of other testsuite bugs with it :).

I did a testrun overnight, and yes, that's indeed the case :)

> 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).
> 

Committed as below.

Thanks,
- Tom