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

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

On Wed, 2022-06-08 at 08:36 -0700, Carl Love wrote:
> Will, Pedro, Kevin, GDB maintainers:
> 
> Oops, accidentally hit send.  Didn't get the patch attached yet.
> 
> I have made the additional fixes to the comments and the setting of the
> clang options as mentioned by Will in version 4.  I do not have an
> environment where gdb can be built with clang.  But I did check that
> the clang options are properly passed to the compile command. 
> Unfortunately, extensive testing of gdb built with XLC and clang are
> beyond the scope of this patch as I do not have access to such systems.
> 
> Per the comments from Kevin, the patch was updated to check for the gcc
> and xlc compilers on PowerPC.  The patch was also tested and verified
> on AIX which uses the gcc compiler to build gdb.  The attempt to build
> gdb using the xlc compiler fails due to unrelated compiler errors.  The
> xlc options to disable the Traceback Table was verified but that was
> it.
> 
> 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 for this function is contained in the .long statements
> following the last instruction in the function.  The Traceback table
> consists of a series of bit fields.  The assembler tries to interpret
> the Traceback Table as instructions.  If the bits in the Traceback
> Table happen to match a known instruction, the assembler will print a
> bogus instruction, otherwise the assembler just prints the bits using
> the .long statement.  Unfortunately, the disassembler does not know how
> to locate the Traceback Table information at the end of a function.
> 
> With this patch, the Traceback Table is disabled, so the last
> instruction of the function is accurately found.  Previously, the break
> point was set at the last .long statement which gdb will never reach. 
> The test now passes as gdb successfully executes to the identified last
> instruction.
> 
> Note, the use of the gcc mtraceback option is not valid on other
> architectures.
> 
> I have tested the patch on Linux Power 10 with gcc, AIX with the gcc
> and Intel with gcc.
> 
> 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 for this test.  The
> Traceback Table consists of a series of bit fields to indicate things like
> the Traceback Table version, language, and specific information about the
> function.  The Traceback Table is generated following the end of the code
> for every function by default.  The Traceback Table is defined in the
> PowerPC ELF ABI and is intended to support debuggers and exception
> handlers.  The Traceback Table is displayed in the disassembly of functions
> by default and is part of the function length.  The table is typically
> interpreted by the disassembler as data represented by .long xxx entries.
> 
> Generation of the Traceback Table is disabled in this test using the
> PowerPC specific gcc compiler option -mtraceback=no, the xlc option
> additional_flags-qtable=none and the clang optons
>  -mllvm -xcoff-traceback-table=false.  Disabling the Traceback Table
> generation in this test results in the gdb_test_multiple statement
> correctly locating the address of the bclr instruction before the statement
> "End of assembler dump." in the disassembly output.
> ---
>  gdb/testsuite/gdb.base/eh_return.exp | 36 +++++++++++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.base/eh_return.exp b/gdb/testsuite/gdb.base/eh_return.exp
> index df55dbc72da..6e4b9bd0bba 100644
> --- a/gdb/testsuite/gdb.base/eh_return.exp
> +++ b/gdb/testsuite/gdb.base/eh_return.exp
> @@ -18,8 +18,42 @@
> 
>  standard_testfile
> 
> +# Set compiler flags.
> +if {[istarget "powerpc*"]} then {
> +    # PowerPC generates a Traceback Table, as defined in the PPC64 ABI,
> +    # following each function by default.  The Traceback Table information is
> +    # typically interpreted by the disassembler as data represented with
> +    # .long xxxx following the last instruction in the function.  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
> +    # compiler option, so the test gdb_test_multiple "disassemble eh2" will
> +    # locate the address of the blr instruction not the last .long statement.
> +    if { [test_compiler_info "gcc-*"] } {
> +	set compile_flags {debug nopie additional_flags=-mtraceback=no}
> +    } elseif { [test_compiler_info "xlc-*"] } {
> +	set compile_flags {debug nopie additional_flags=-qtbtable=none}
> +    } elseif { [test_compiler_info "clang-*"] } {
> +	set compile_flags [list debug nopie additional_flags=-mllvm \
> +			       additional_flags=-xcoff-traceback-table=false]
> +    } else {
> +	set compile_flags {debug nopie }
> +    }
> +} else {
> +    set compile_flags {debug nopie}
> +}
> +
>  if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
> -	 {debug nopie}]} {
> +	 $compile_flags]} {
>      return -1
>  }


This looks good to me.  Thanks for the multiple iterations and updates.
I am not an approver but I recommend approval.  :-) 

Thanks
-Will


>