Re: [PATCH V2] PowerPC: fix for gdb.base/eh_return.exp

[will schmidt <will_schmidt@vnet.ibm.com>]

On Mon, 2022-05-09 at 12:17 -0700, Carl Love wrote:
> Will, Pedro, Kevin, GDB maintainers:
> 
> Sorry, resend, the first attempt was missing the mailing list.
> 
> I have updated the patch per the comments from Will.  The new version
> of the patch uses a PowerPC specific gcc option to suppress the
> generation of the Traceback Table information.  The Traceback
> information is contained in the .long statements following the last
> instruction in the function.  Now the existing test to locate the last
> instruction in the function works without any changes.


In this cases the disassembly is presenting the traceback table
contents as .long values, but with an (un)lucky combination of bits, it
may also represent parts of that table as instructions.  Thats all
chance on which bits/fields are set.  That can show up readily with the
"-mtraceback=full" option, probably unlikely with the default trace
table contents.   (tldr; it won't always be represented as .long
0xfoo...).



> 
> Note, the use of the gcc mtraceback option is not valid on other
> architectures.
> 
> I have tested the patch on Power 10 and Intel.
> 
> Please let me know if this patch is acceptable.  Thanks for the input
> and help with the patch.
> 
>                           Carl Love
> -----------------------------------------------------------------
> 
> PowerPC: fix for gdb.base/eh_return.exp
> 


> Disable the Traceback Table generation on PowerPC.  The Traceback Table

  Disable ... for this test.

> consists of multiple .long statement following the final instruction in

The traceback table is being interpreted by the assembler as data and
is represented as .long by the disassembler that is looking for
instructions, but the table itself is a series of mostly bitfields.   


I'll suggest rephrasing, something like 


s/consists ... statements /is defined by the PowerPC ELF ABI and is
intended to support debuggers and exception handlers.  The Traceback
Table is located following the end of the code for every function. 

> the function.  Generation of the .long statements needs to be disabled with
> the PowerPC specific gcc compiler option -mtraceback=no so the test will
> correctly locate the address of the bclr instruction before the statement
> "End of assembler dump."



> ---
>  gdb/testsuite/gdb.base/eh_return.exp | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.base/eh_return.exp b/gdb/testsuite/gdb.base/eh_return.exp
> index df55dbc72da..3a49464af63 100644
> --- a/gdb/testsuite/gdb.base/eh_return.exp
> +++ b/gdb/testsuite/gdb.base/eh_return.exp
> @@ -18,8 +18,30 @@
> 
>  standard_testfile
> 
> +if {[istarget "powerpc*"]} then {
> +    # PowerPC generates a Traceback Table following each function by default.


Consider adding "as defined by the PPC64 ABIs" betwen Table and
Following. 


> +    # The .long statements following the function contain the Traceback Table
> +    # information.  For example:
> +    #   Dump of assembler code for function eh2:
> +    #   0x00000000100009e0 <+0>:     lis     r2,4098
> +    #   ...
> +    #   0x0000000010000b04 <+292>:   add     r1,r1,r10
> +    #   0x0000000010000b08 <+296>:   blr
> +    #   0x0000000010000b0c <+300>:   .long 0x0
> +    #   0x0000000010000b10 <+304>:   .long 0x1000000
> +    #   0x0000000010000b14 <+308>:   .long 0x1000180
> +    #   End of assembler dump.
> +    #
> +    # Disable the Traceback Table generation, using the PowerPC specific
> +    # gcc option, so the test gdb_test_multiple "disassemble eh2" will match
> +    # the blr instruction not the .long statement.
> +    set compile_flags {debug nopie additional_flags=-mtraceback=no}
> +} else {
> +    set compile_flags {debug nopie}
> +}

This seems OK to me, assuming this test is exercising a simple
scenario, versus representative of function elsewhere in the GDB code
base.  If there is a deeper issue that the traceback tables are causing
elsewhere in the GDB code base, some deeper solution may need to be
investigated. 


> +
>  if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
> -	 {debug nopie}]} {
> +	 $compile_flags]} {
>      return -1
>  }
>