Re: [PATCH ver3] gdb: Add PowerPC support to gdb.dwarf2/frame-inlined-in-outer-frame.exp

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2021-12-06 11:37 a.m., Carl Love wrote:
> Simon:
>
> On Fri, 2021-12-03 at 21:13 -0500, Simon Marchi wrote:
>> <snip>
>>
>> Format like this:
>>
>> #if defined (__powerpc64__)
>> #  if _CALL_ELF == 2
>> ...
>> #  else
>> ...
>> #  endif
>> #endif
>>
>> Put the assembly statements at column 0, as they would be without the
>> preprocessor conditionals.
>
> I made the requested changes, above, to the patch.
>
>> I guess I'll have to trust you because I know nothing about PowerPC
>> assembly but... this is all really necessary for such a simple
>> program?
>
> Per Will Schmidt's commnets
>
>    Yes..  Details as I paraphrase them are that
>    ElfV1 (Powerpc Big Endian) requires an opd (Official Procedure
>    Descriptor) section that contains the function descriptors.
>    So, the assembly here is defining the abiversion, and the .opd
>    section
>    (with required alignment), and adding an entry for the _start
>    function
>    in that section.
>
>    This requirement was eliminated in the ELF V2 ABI, as
>    seen in the simpler #ifdef branch above.
>
>    Thanks
>    -Will
>
> Yes unfortunately.  I would also add to what Will said that we need tp
> explicitly specify which abi version ( directive .abiversion ) for the
> assembler to use. Otherwise it will assume version 1 by default.

Thanks to both for the explanations.

>
> Please let me know if the patch is acceptable for mainline.  Thanks.
>
>                     Carl
>
> ---------------------------------------------------------------------
> gdb: Add PowerPC support to gdb.dwarf2/frame-inlined-in-outer-frame
>
> This patch adds an #elif defined for PowerPC to setup the exit_0 macro.
> This patch addes the needed macro definitionald logic to handle both elfV1
> and elfV2.
>
> The patch has been successfully tested on both PowerPC BE, Powerpc LE and
> X86_64 with no regressions.
> ---
>  .../gdb.dwarf2/frame-inlined-in-outer-frame.S | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.dwarf2/frame-inlined-in-outer-frame.S b/gdb/testsuite/gdb.dwarf2/frame-inlined-in-outer-frame.S
> index 9fb6e7b7164..09fe75d8132 100644
> --- a/gdb/testsuite/gdb.dwarf2/frame-inlined-in-outer-frame.S
> +++ b/gdb/testsuite/gdb.dwarf2/frame-inlined-in-outer-frame.S
> @@ -65,6 +65,16 @@
>  	swi 0x0
>  .endm
>
> +#elif defined __powerpc64__
> +
> +# define RETURN_ADDRESS_REGNO 65
> +
> +.macro exit_0
> +	li 0, __NR_exit  /* r0 - contains system call number */
> +	li 3, 0          /* r3 - contains first argument for sys call */
> +	sc
> +.endm
> +
>  #else
>  # error "Unsupported architecture"
>  #endif
> @@ -90,8 +100,28 @@
>     16 }
>  */
>
> +#if defined __powerpc64__
> +#  if _CALL_ELF == 2
> +.abiversion 2   /* Tell gdb what ELF version to use. */
> +.global _start
> +_start:
> +#  else
> +.abiversion 1   /* Tell gdb what ELF version to use. */
> +.align 2
> +.global _start
> +.section ".opd", "aw"
> +.align 3
> +_start:
> +.quad ._start,.TOC.@tocbase,0
> +.previous
> +.type ._start,@function
> +._start:
> +#  endif
> +#else
>  .global _start
>  _start:
> +#endif

My only question is whether these two lines:

  .global _start
  _start:

could be kept unconditional, and just not put in the powerpc-specific
sections of the code at all.

Simon