Re: [PATCH] Improve forward progress test in gdb.python/python.exp

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Carl Love <cel@us.ibm.com> [2021-08-11 12:14:32 -0700]:

> On Wed, 2021-08-11 at 16:58 +0100, Andrew Burgess wrote:
> > 
> > > > I wonder if we can simplify the logic here?  We always expect the
> > > > pc
> > > > to be bigger, right?  So we can just test that in all
> > > > cases.  Once we
> > > > know that is being tested, we can make the line number test
> > > > simpler,
> > > > like this:
> > > > 
> > > >   gdb_py_test_silent_cmd "python pc_rtn =
> > > > gdb.selected_frame().pc()" \
> > > >       "Get pc at func2 return site" 1
> > > > 
> > > >   gdb_test "python print (pc_rtn > pc_call)" "True" \
> > > >       "test resume address greater then call address"
> > > > 
> > > >   gdb_test "python print (gdb.find_pc_line(pc_rtn).line >= line)"
> > > > "True" \
> > > >       "test find_pc_line with resume address"
> > > > 
> > > > I wrapped the lines to keep them under 80 characters, which is
> > > > the
> > > > correct style where possible.
> > > > 
> > > > What do you think?
> > > 
> > > The above test sequence adds the PC check but doesn't change the
> > > line
> > > test.  So Powerpc will still fail the line test and thus we haven't
> > > fixed the existing failure on Powerpc.
> > 
> > It did change, previously the line check was '>', now I'm suggesting
> > we use '>='.
> 
> Ah, I missed the new equal sign in the test.  Note, I just read thru
> the changes, I didn't actually run them.
> > 
> > In your original email you wrote:
> > 
> > > The current gdb.python/python.exp test fails on powerpc.  The test
> > > checks to see if the source code line for the function call is less
> > > than the line number after the function call.  The issue on powerpc
> > > is
> > > the assembly instructions for the branch and the NOP following the
> > > branch map to the same source code line.
> > 
> > You suggest that the NOP is mapped to the _same_ source line, so I
> > would expect the new test to pass, even on ppc.
> 
> Yes, the updated line test with the equal will pass on PPC64. As noted,
> I missed the equal sign in your message. 
> 
> I have updated the patch with your changes and tested it on PPC64.  The
> test now passes with no failures on PPC64.
> 
>                         Carl 
> 
> ----------------------------------------------------------------
> Improve forward progress test in python.exp
> 
> The test steps into func2 and than does an up to get back to the previous
> frame. The test checks that the line number you are at after the up command
> is greater than the line where the function was called from. The
> assembly/codegen for the powerpc target includes a NOP after the
> branch-link.
> 
> func2 (); /* Break at func2 call site. /
> 10000694: 59 00 00 48 bl 100006ec
> 10000698: 00 00 00 60 nop
> return 0; / Break to end. */
> 1000069c: 00 00 20 39 li r9,0
> 
> The PC at the instruction following the branch-link is 0x10000698 which
> GDB.find_pc_line() maps to the same line number as the bl instruction.
> GDB did move past the branch-link location thus making forward progress.
> 
> The following proposed fix adds an additional PC check to see if forward
> progress was made.  The line test is changed from greater than to greater
> than or equal.

LGTM.

Thanks,
Andrew


> ---
>  gdb/testsuite/gdb.python/python.exp | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
> index d9fd60f3dd4..bc670ce25c1 100644
> --- a/gdb/testsuite/gdb.python/python.exp
> +++ b/gdb/testsuite/gdb.python/python.exp
> @@ -496,12 +496,27 @@ if ![runto_main] then {
>  runto [gdb_get_line_number "Break at func2 call site."]
>  
>  gdb_py_test_silent_cmd "python line = gdb.selected_frame().find_sal().line" "Get line number of func2 call site" 1
> +
> +gdb_py_test_silent_cmd "python pc_call = gdb.selected_frame().pc()" \
> +    "Get pc of func2 call site" 1
> +
>  gdb_test "python print (gdb.find_pc_line(gdb.selected_frame().pc()).line == line)" "True" "test find_pc_line at func2 call site"
>  
>  gdb_py_test_silent_cmd "step" "Step into func2" 1
>  gdb_py_test_silent_cmd "up" "Step out of func2" 1
>  
> -gdb_test "python print (gdb.find_pc_line(gdb.selected_frame().pc()).line > line)" "True" "test find_pc_line with resume address"
> +# The point of the following test is to see if gdb has advanced past the
> +# location where the branch to a function was made.
> +set test_name "test find_pc_line with resume address"
> +
> +gdb_py_test_silent_cmd "python pc_rtn = gdb.selected_frame().pc()" \
> +    "Get pc at func2 return site" 1
> +
> +gdb_test "python print (pc_rtn > pc_call)" "True" \
> +    "test resume address greater then call address"
> +
> +gdb_test "python print (gdb.find_pc_line(pc_rtn).line >= line)" "True" \
> +    "test find_pc_line with resume address"
>  
>  gdb_test_no_output "set variable \$cvar1 = 23" "set convenience variable"
>  gdb_test "python print(gdb.convenience_variable('cvar1'))" "23"
> -- 
> 2.17.1
> 
> 
>