diff mbox

testsuite: Update py-mi-objfile to expect console format print

Message ID 1459851290-13123-1-git-send-email-catalin.udma@freescale.com
State New, archived
Headers

Commit Message

Catalin Udma April 5, 2016, 10:14 a.m. UTC 
  Starting with this commit:

  commit e7ea3ec7c623be0b9be1eaf4929794652a84be11
  Author: Catalin Udma <catalin.udma@freescale.com>
  Date:   Mon Feb 29 16:16:19 2016 +0200

    python: Use console format for output of gdb.execute command

the python script would use console format for gdb.execute output.
This patch fixes py-mi-objfile to expect the console format when is
checking for gdb.execute printing.

gdb/testsuite/ChangeLog:
2016-04-05  Catalin Udma  <catalin.udma@freescale.com>

        * gdb.python/py-mi-objfile-gdb.py: Use "list main" command
        * gdb.python/py-mi-objfile-gdb.exp: Match "list main"
        output in console format.

Signed-off-by: Catalin Udma <catalin.udma@freescale.com>
---
 gdb/testsuite/gdb.python/py-mi-objfile-gdb.py | 2 +-
 gdb/testsuite/gdb.python/py-mi-objfile.exp    | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

Comments

Pedro Alves April 5, 2016, 10:59 a.m. UTC | #1 
On 04/05/2016 11:14 AM, Catalin Udma wrote:
> --- a/gdb/testsuite/gdb.python/py-mi-objfile-gdb.py
> +++ b/gdb/testsuite/gdb.python/py-mi-objfile-gdb.py
> @@ -24,4 +24,4 @@ import gdb
>  gdb.execute("set width 101", to_string=True)
>  # And finally a command that will use the original MI stream, which in a
>  # buggy gdb will use just-freed data.
> -gdb.execute("list")
> +gdb.execute("list main")

Do the comments in the test still make sense?
From the comment, sounds like the test might no longer be covering
what it intended?  I.e. do we "use the original MI stream" ?

Thanks,
Pedro Alves
Catalin-Dan Udma April 5, 2016, 11:45 a.m. UTC | #2 
> On 04/05/2016 11:14 AM, Catalin Udma wrote:
> > --- a/gdb/testsuite/gdb.python/py-mi-objfile-gdb.py
> > +++ b/gdb/testsuite/gdb.python/py-mi-objfile-gdb.py
> > @@ -24,4 +24,4 @@ import gdb
> >  gdb.execute("set width 101", to_string=True)
> >  # And finally a command that will use the original MI stream, which in a
> >  # buggy gdb will use just-freed data.
> > -gdb.execute("list")
> > +gdb.execute("list main")
> 
> Do the comments in the test still make sense?
> From the comment, sounds like the test might no longer be covering
> what it intended?  I.e. do we "use the original MI stream" ?
> 
> Thanks,
> Pedro Alves

[Catalin Udma] Indeed, the original crash is no longer reproducible: I manually reverted the fixes
for that crash (gdb/cli/cli-logging.c, gdb/mi/mi-out.c) and the crash is not reproducible anymore 
when running py-mi-objfile.exp.
It reproduces if I revert also this patch " python: Use console format for output of gdb.execute command"

Updating the comments would be enough and let the test validate the same scenario? 

"+# And finally a command that will use the console stream without redirection" ?
Pedro Alves April 5, 2016, 11:59 a.m. UTC | #3 
On 04/05/2016 12:45 PM, Catalin-Dan Udma wrote:
>> On 04/05/2016 11:14 AM, Catalin Udma wrote:
>>> --- a/gdb/testsuite/gdb.python/py-mi-objfile-gdb.py
>>> +++ b/gdb/testsuite/gdb.python/py-mi-objfile-gdb.py
>>> @@ -24,4 +24,4 @@ import gdb
>>>  gdb.execute("set width 101", to_string=True)
>>>  # And finally a command that will use the original MI stream, which in a
>>>  # buggy gdb will use just-freed data.
>>> -gdb.execute("list")
>>> +gdb.execute("list main")
>>
>> Do the comments in the test still make sense?
>> From the comment, sounds like the test might no longer be covering
>> what it intended?  I.e. do we "use the original MI stream" ?
>>
>> Thanks,
>> Pedro Alves
> 
> [Catalin Udma] Indeed, the original crash is no longer reproducible: I manually reverted the fixes
> for that crash (gdb/cli/cli-logging.c, gdb/mi/mi-out.c) and the crash is not reproducible anymore 
> when running py-mi-objfile.exp.
> It reproduces if I revert also this patch " python: Use console format for output of gdb.execute command"
> 
> Updating the comments would be enough and let the test validate the same scenario? 

I think it's good to keep validating the same scenario.

Though maybe there's something else we could add that uses the
MI stream, that'd also expose the crash?  Maybe

 gdb.execute ("interpreter-exec mi \"-gdb-show width\"")

would work?

> "+# And finally a command that will use the console stream without redirection" ?

Thanks,
Pedro Alves
Catalin-Dan Udma April 5, 2016, 1:08 p.m. UTC | #4 
> 
> On 04/05/2016 12:45 PM, Catalin-Dan Udma wrote:
> >> On 04/05/2016 11:14 AM, Catalin Udma wrote:
> >>> --- a/gdb/testsuite/gdb.python/py-mi-objfile-gdb.py
> >>> +++ b/gdb/testsuite/gdb.python/py-mi-objfile-gdb.py
> >>> @@ -24,4 +24,4 @@ import gdb
> >>>  gdb.execute("set width 101", to_string=True)
> >>>  # And finally a command that will use the original MI stream, which in a
> >>>  # buggy gdb will use just-freed data.
> >>> -gdb.execute("list")
> >>> +gdb.execute("list main")
> >>
> >> Do the comments in the test still make sense?
> >> From the comment, sounds like the test might no longer be covering
> >> what it intended?  I.e. do we "use the original MI stream" ?
> >>
> >> Thanks,
> >> Pedro Alves
> >
> > [Catalin Udma] Indeed, the original crash is no longer reproducible: I manually
> reverted the fixes
> > for that crash (gdb/cli/cli-logging.c, gdb/mi/mi-out.c) and the crash is not
> reproducible anymore
> > when running py-mi-objfile.exp.
> > It reproduces if I revert also this patch " python: Use console format for
> output of gdb.execute command"
> >
> > Updating the comments would be enough and let the test validate the same
> scenario?
> 
> I think it's good to keep validating the same scenario.
> 
> Though maybe there's something else we could add that uses the
> MI stream, that'd also expose the crash?  Maybe
> 
>  gdb.execute ("interpreter-exec mi \"-gdb-show width\"")
> 
> would work?
> 
> > "+# And finally a command that will use the console stream without
> redirection" ?
> 
> Thanks,
> Pedro Alves

[Catalin Udma] 
gdb.execute ("interpreter-exec mi \"-gdb-show width\"", to_string = True)
would crash the gdb with or without any of the patches in discussion.
(MI patch for crash redirection, and use console for python gdb.execute)

I propose to log this new bug, and for now just update the comments in this test.
When the new bug would be fixed, the this test would be updated with the new
scenario.
Pedro Alves April 5, 2016, 1:11 p.m. UTC | #5 
On 04/05/2016 02:08 PM, Catalin-Dan Udma wrote:

> [Catalin Udma] 
> gdb.execute ("interpreter-exec mi \"-gdb-show width\"", to_string = True)
> would crash the gdb with or without any of the patches in discussion.
> (MI patch for crash redirection, and use console for python gdb.execute)

Bah...

> 
> I propose to log this new bug, and for now just update the comments in this test.
> When the new bug would be fixed, the this test would be updated with the new
> scenario.

OK.

Thanks,
Pedro Alves
Pedro Alves April 5, 2016, 1:13 p.m. UTC | #6 
On 04/05/2016 02:11 PM, Pedro Alves wrote:
> On 04/05/2016 02:08 PM, Catalin-Dan Udma wrote:
> 
>> [Catalin Udma] 
>> gdb.execute ("interpreter-exec mi \"-gdb-show width\"", to_string = True)
>> would crash the gdb with or without any of the patches in discussion.
>> (MI patch for crash redirection, and use console for python gdb.execute)
> 
> Bah...

Looking again, note I suggested:

 gdb.execute ("interpreter-exec mi \"-gdb-show width\"")

not:

 gdb.execute ("interpreter-exec mi \"-gdb-show width\"", to_string = True)

TBC, does the former also likewise crash with or without patches?

Thanks,
Pedro Alves
Catalin-Dan Udma April 5, 2016, 1:36 p.m. UTC | #7 
> On 04/05/2016 02:11 PM, Pedro Alves wrote:
> > On 04/05/2016 02:08 PM, Catalin-Dan Udma wrote:
> >
> >> [Catalin Udma]
> >> gdb.execute ("interpreter-exec mi \"-gdb-show width\"", to_string = True)
> >> would crash the gdb with or without any of the patches in discussion.
> >> (MI patch for crash redirection, and use console for python gdb.execute)
> >
> > Bah...
> 
> Looking again, note I suggested:
> 
>  gdb.execute ("interpreter-exec mi \"-gdb-show width\"")
> 
> not:
> 
>  gdb.execute ("interpreter-exec mi \"-gdb-show width\"", to_string = True)
> 
> TBC, does the former also likewise crash with or without patches?
> 
> Thanks,
> Pedro Alves
The former gdb.execute ("interpreter-exec mi \"-gdb-show width\"") does not crash the gdb
in any combination of the two patches in discussion.
Regards,
Catalin
Pedro Alves April 5, 2016, 1:47 p.m. UTC | #8 
On 04/05/2016 02:36 PM, Catalin-Dan Udma wrote:

> The former gdb.execute ("interpreter-exec mi \"-gdb-show width\"") does not crash the gdb
> in any combination of the two patches in discussion.

Thanks.  Let's indeed just update the comments then.
Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/testsuite/gdb.python/py-mi-objfile-gdb.py b/gdb/testsuite/gdb.python/py-mi-objfile-gdb.py
index 492d71c..ff1d983 100644
--- a/gdb/testsuite/gdb.python/py-mi-objfile-gdb.py
+++ b/gdb/testsuite/gdb.python/py-mi-objfile-gdb.py
@@ -24,4 +24,4 @@  import gdb
 gdb.execute("set width 101", to_string=True)
 # And finally a command that will use the original MI stream, which in a
 # buggy gdb will use just-freed data.
-gdb.execute("list")
+gdb.execute("list main")
diff --git a/gdb/testsuite/gdb.python/py-mi-objfile.exp b/gdb/testsuite/gdb.python/py-mi-objfile.exp
index 073e4d9..bfa43a2 100644
--- a/gdb/testsuite/gdb.python/py-mi-objfile.exp
+++ b/gdb/testsuite/gdb.python/py-mi-objfile.exp
@@ -53,5 +53,6 @@  if [is_remote host] {
 
 # PR 18833.  This will cause an unpatched gdb to crash.
 mi_gdb_test "-file-exec-and-symbols ${filename}" \
-    ".*\\^done,line=.*${srcfile}\"" \
+    ".*main \\(.*void\\).*\
+    return 0;.*\\^done" \
     "file-exec-and-symbols operation"