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

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

[Carl Love]

GDB maintainers:

The following patch adds the needed Powerpc support to the
gdb.dwarf2/frame-inlined-in-outer-frame test.  The patch adds the
needed Power instructions to the exit_0 macro.  It also adds the needed
support for PowerPC Elfv1 or Elfv2 if the architecture is powerpc64.

The patch has been tested on PowerPC LE and BE as well as X86_64 with
no failures.

Please let me know if this patch is acceptable for mainline.  Thanks.

              Carl 
--------------------------------------------
gdb: Add PowerPC support to gdb.dwarf2/frame-inlined-in-outer-frame.exp

The test does not run on PowerPC BE or LE as it is missing an #elif
defined for PowerPC to setup the exit_0 macro.  This patch addes the needed
macro definition.  It also setup the assembly code to use the elfv1 if
running on a PowerPC BE system and elfv2 if running on a PowerPC LE system.

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 | 34 +++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

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..ecb344f4426 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 }
 */
 
-.global _start
-_start:
+# 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
+
 .cfi_startproc
 
 /* State that the return address for this frame is undefined. */
-- 
2.30.2

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

[will schmidt]

On Mon, 2021-11-29 at 13:43 -0800, Carl Love wrote:
> GDB maintainers:
> 

Hi,

> The following patch adds the needed Powerpc support to the
> gdb.dwarf2/frame-inlined-in-outer-frame test.  The patch adds the
> needed Power instructions to the exit_0 macro.  It also adds the needed
> support for PowerPC Elfv1 or Elfv2 if the architecture is powerpc64.
> 

Part of this could be combined with the patch description below.


> The patch has been tested on PowerPC LE and BE as well as X86_64 with
> no failures.
> 
> Please let me know if this patch is acceptable for mainline.  Thanks.
> 


>               Carl 
> --------------------------------------------
> gdb: Add PowerPC support to gdb.dwarf2/frame-inlined-in-outer-frame.exp
> 
> The test does not run on PowerPC BE or LE as it is missing an #elif

It does after this patch, so I'd suggest stating this as "This patch adds an #elif clause for powerpc ..."


> defined for PowerPC to setup the exit_0 macro.  This patch addes the needed
> macro definition.  It also setup the assembly code to use the elfv1 if

Not an also, the macro definition includes logic to handle both elfV1 and elfV2 ABIs. 

> running on a PowerPC BE system and elfv2 if running on a PowerPC LE system.
> 
> 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 | 34 +++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> 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..ecb344f4426 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 }
>  */
> 
> -.global _start
> -_start:
> +# if defined __powerpc64__
> +	#if _CALL_ELF == 2
> +		.abiversion 2   /* Tell gdb what ELF version to use. */
> +		.global _start

GCC currently emits .globl (without 'a').    GAS documentation suggests

that .global is compatible.   I would lean towards that, but no strong
feelings, I just mention it as it caught my eye.

> +		_start:
> +	#else
> +		.abiversion 1     /* Tell gdb what ELF version to use. */

Nit, There is a whitespace variation between the two .abiversion lines above. 


> +		.align 2
> +		.global _start
> +		.section ".opd", "aw"
> +		.align 3

I see .align 3 is also emitted by gcc, so presumably this is necessary
and appropriate.    

> +		_start:
> +		.quad ._start,.TOC.@tocbase,0
> +		.previous
> +		.type ._start,@function
> +		._start:
> +	#endif
> +#else
> +	.global _start
> +	_start:
> +#endif
> +

Cosmetic nits aside, this looks reasonable to me, I defer to
maintainers, of course. 
Thanks
-Will



>  .cfi_startproc
> 
>  /* State that the return address for this frame is undefined. */

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

[Carl Love]

GDB maintainers:

The following is an updated version of the patch based on the comments
from Will Schmidt.  Also added Ulrigh to the cc list.

The following patch adds the needed Powerpc support to the
gdb.dwarf2/frame-inlined-in-outer-frame test as described below.

The patch has been tested on PowerPC LE and BE as well as X86_64 with
no failures.

Please let me know if this 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 | 34 +++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

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..ad332cfc671 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 }
 */
 
-.global _start
-_start:
+# 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
+
 .cfi_startproc
 
 /* State that the return address for this frame is undefined. */
-- 
2.30.2

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

[Simon Marchi]

On 2021-12-03 19:38, Carl Love via Gdb-patches wrote:
> 
> GDB maintainers:
> 
> The following is an updated version of the patch based on the comments
> from Will Schmidt.  Also added Ulrigh to the cc list.
> 
> The following patch adds the needed Powerpc support to the
> gdb.dwarf2/frame-inlined-in-outer-frame test as described below.
> 
> The patch has been tested on PowerPC LE and BE as well as X86_64 with
> no failures.
> 
> Please let me know if this 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.

Thanks for doing this.  More formatting comments below.

> ---
>  .../gdb.dwarf2/frame-inlined-in-outer-frame.S | 34 +++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> 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..ad332cfc671 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 }
>  */
>  
> -.global _start
> -_start:
> +# if defined __powerpc64__
> +	#if _CALL_ELF == 2

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.


> +		.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

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?

Simon

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

[will schmidt]

On Fri, 2021-12-03 at 21:13 -0500, Simon Marchi via Gdb-patches wrote:
> 
> On 2021-12-03 19:38, Carl Love via Gdb-patches wrote:
> > GDB maintainers:
> > 
> > The following is an updated version of the patch based on the
> > comments
> > from Will Schmidt.  Also added Ulrigh to the cc list.
> > 
> > The following patch adds the needed Powerpc support to the
> > gdb.dwarf2/frame-inlined-in-outer-frame test as described below.
> > 
> > The patch has been tested on PowerPC LE and BE as well as X86_64
> > with
> > no failures.
> > 
> > Please let me know if this 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.
> 
> Thanks for doing this.  More formatting comments below.
> 
> > ---
> >  .../gdb.dwarf2/frame-inlined-in-outer-frame.S | 34
> > +++++++++++++++++--
> >  1 file changed, 32 insertions(+), 2 deletions(-)
> > 
> > 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..ad332cfc671 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 }
> >  */
> >  
> > -.global _start
> > -_start:
> > +# if defined __powerpc64__
> > +	#if _CALL_ELF == 2
> 
> 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.
> 
> 
> > +		.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
> 
> 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?

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


> 
> Simon

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

[Carl Love]

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.

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
+
 .cfi_startproc
 
 /* State that the return address for this frame is undefined. */
-- 
2.30.2

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

[Simon Marchi]

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

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

[Carl Love]

Simon:

On Mon, 2021-12-06 at 11:43 -0500, Simon Marchi wrote:
> > +#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.

I chose to have them in the Power #if case with the .abiversion 2
statement and in the #else for all other architectures. On Power, we
must include .abiversion 2.  I just don't know if on other platforms
.abiversion 2 is a valid statement or not?  So, I chose not to include
it, i.e. don't change anything for other architectures to be sure I
don't break them.  Basically, I took the most conservative approach as
I can't test all the other architectures.

                         Carl 

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

[Simon Marchi]

On 2021-12-06 1:06 p.m., Carl Love wrote:
> Simon:
>
> On Mon, 2021-12-06 at 11:43 -0500, Simon Marchi wrote:
>>> +#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.
>
> I chose to have them in the Power #if case with the .abiversion 2
> statement and in the #else for all other architectures. On Power, we
> must include .abiversion 2.  I just don't know if on other platforms
> .abiversion 2 is a valid statement or not?  So, I chose not to include
> it, i.e. don't change anything for other architectures to be sure I
> don't break them.  Basically, I took the most conservative approach as
> I can't test all the other architectures.
>
>                          Carl

I wasn't clear.  I was wondering if this would work (not duplicate
`.global _start ` and `._start:`:

#if defined __powerpc64__
#  if _CALL_ELF == 2
.abiversion 2   /* Tell gdb what ELF version to use. */
#  else
.abiversion 1   /* Tell gdb what ELF version to use. */
.align 2
.section ".opd", "aw"
.align 3
.quad ._start,.TOC.@tocbase,0
.previous
.type ._start,@function
#  endif
#endif
.global _start
_start:

Simon

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

[Carl Love]

Simon:

On Mon, 2021-12-06 at 13:50 -0500, Simon Marchi wrote:
> 
> > > 
> > > 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.
> > 

OK, I gave that a try and it does seem to work OK for Power LE and BE. 
It keeps the non-Power code the same so it shouldn't break any other
architectures.

Below is the updated and tested patch.  Please let me know what you
think.

                    Carl 
---------------------------------------------------------------
From a94f427447c8da0480a387cd364555450c785d43 Mon Sep 17 00:00:00 2001
From: Carl Love <cel@us.ibm.com>
Date: Fri, 19 Nov 2021 18:33:51 +0000
Subject: [PATCH] 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 | 24 +++++++++++++++++++
 1 file changed, 24 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..224b50b20a5 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,6 +100,20 @@
    16 }
 */
 
+#if defined __powerpc64__
+#  if _CALL_ELF == 2
+.abiversion 2   /* Tell gdb what ELF version to use. */
+#  else
+.abiversion 1   /* Tell gdb what ELF version to use. */
+.align 2
+.section ".opd", "aw"
+.align 3
+.quad ._start,.TOC.@tocbase,0
+.previous
+.type ._start,@function
+._start:
+#  endif
+#endif
 .global _start
 _start:
 .cfi_startproc
-- 
2.30.2

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

[Simon Marchi]

On 2021-12-06 2:35 p.m., Carl Love wrote:
> Simon:
> 
> On Mon, 2021-12-06 at 13:50 -0500, Simon Marchi wrote:
>>
>>>>
>>>> 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.
>>>
> 
> OK, I gave that a try and it does seem to work OK for Power LE and BE. 
> It keeps the non-Power code the same so it shouldn't break any other
> architectures.
> 
> Below is the updated and tested patch.  Please let me know what you
> think.
> 
>                     Carl 
> ---------------------------------------------------------------
> From a94f427447c8da0480a387cd364555450c785d43 Mon Sep 17 00:00:00 2001
> From: Carl Love <cel@us.ibm.com>
> Date: Fri, 19 Nov 2021 18:33:51 +0000
> Subject: [PATCH] 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 | 24 +++++++++++++++++++
>  1 file changed, 24 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..224b50b20a5 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,6 +100,20 @@
>     16 }
>  */
>  
> +#if defined __powerpc64__
> +#  if _CALL_ELF == 2
> +.abiversion 2   /* Tell gdb what ELF version to use. */
> +#  else
> +.abiversion 1   /* Tell gdb what ELF version to use. */
> +.align 2
> +.section ".opd", "aw"
> +.align 3
> +.quad ._start,.TOC.@tocbase,0
> +.previous
> +.type ._start,@function
> +._start:
> +#  endif
> +#endif
>  .global _start
>  _start:
>  .cfi_startproc
> -- 
> 2.30.2
> 
> 


Thanks, that LGTM.

Simon

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

[Ulrich Weigand]

"Simon Marchi" <simon.marchi@polymtl.ca> wrote on 06.12.2021 19:50:57:

> I wasn't clear.  I was wondering if this would work (not duplicate
> `.global _start ` and `._start:`:
>
> #if defined __powerpc64__
> #  if _CALL_ELF == 2
> .abiversion 2   /* Tell gdb what ELF version to use. */
> #  else
> .abiversion 1   /* Tell gdb what ELF version to use. */
> .align 2
> .section ".opd", "aw"
> .align 3
> .quad ._start,.TOC.@tocbase,0
> .previous
> .type ._start,@function
> #  endif
> #endif
> .global _start
> _start:

Unfortunately, this is wrong: on ELFv1, the _start symbol
must point to the *function descriptor* (in the .opd
section), not to the function code (in the .text section)
like with ELFv2 and other architectures.

Bye,
Ulrich

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

[Carl Love]

Ulrich, Simon:

It looks like I tested Simon's last patch suggestion on a Power 9LE
machine not the Power 8BE machine as intended. I verified that the
current mainline version of the frame-inlined-in-outer-frame does not
work on Power 8 BE. 

 I created the following patch to revert back to the version of the
patch with the _start symbol in the the Power LE, BE and default #if
clauses.  The patch was then retested on Power 10 LE and Power 8BE to
verify the updated patch is working correctly.

Sorry for messing up the previous testing.  Please let me know if you
see any additional issues with this patch or the assembly code file. 
Thanks.

                                                   Carl 

---------------------------------------------------------------------
-------------------------------
gdb fix elfv1 Powerpc gdb.dwarf2/frame-inlined-in-outer-frame.exp

On ELFv1, the _start symbol must point to the *function descriptor* (in
the .opd section), not to the function code (in the .text section) like
with ELFv2 and other architectures.

Patch retested on Power 10 LE and Power 8 BE systems.  Test passes on both
systems.
---
 gdb/testsuite/gdb.dwarf2/frame-inlined-in-outer-frame.S | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

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 224b50b20a5..112788a3440 100644
--- a/gdb/testsuite/gdb.dwarf2/frame-inlined-in-outer-frame.S
+++ b/gdb/testsuite/gdb.dwarf2/frame-inlined-in-outer-frame.S
@@ -103,19 +103,24 @@
 #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
-#endif
+#else
 .global _start
 _start:
+#endif
 .cfi_startproc
 
 /* State that the return address for this frame is undefined. */
-- 
2.25.1

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

[Ulrich Weigand]

"Carl Love" <cel@us.ibm.com> wrote on 08.12.2021 21:49:44:

> gdb fix elfv1 Powerpc gdb.dwarf2/frame-inlined-in-outer-frame.exp
>
> On ELFv1, the _start symbol must point to the *function descriptor* (in
> the .opd section), not to the function code (in the .text section) like
> with ELFv2 and other architectures.

This looks good to me.

Thanks,
Ulrich