diff mbox series

[1/2,ver,2] PowerPC, function ppc64_sysv_abi_return_value add missing return value convention

Message ID c82b551b5204a47746b24b12959e58e0da092b56.camel@us.ibm.com
State Committed
Commit 24b27e5e9b6bf5a37fb39dfca151722bb801cbaa
Headers
Series PowerPC, fix support for printing the function return value for non-trivial values. |

Commit Message

Carl Love Oct. 18, 2022, 6:55 p.m. UTC 
  GDB maintainers:

Version 2, updated the commit log per feedback.

The following PowerPC specific patch fixes an issue of GDB reporting a
bogus return value for functions that return a non-trivial value.  The
bogus return values result in five testcase failures for test
gdb.cp/non-trivial-retval.exp.  The issue is the function
ppc64_sysv_abi_return_value does not return the correct value when the
valtype->code() is TYPE_CODE_STRUCT and the language_pass_by_reference
is not trivially_copyable.  This patch adds the needed code to return
RETURN_VALUE_STRUCT_CONVENTION in these cases.  

The testcase gdb.cp/non-trivial-retval.exp still fails as gdb now
correctly reports "Cannot determine contents" instead of the expected
values, which is correct in this case.  The PowerPC ABI uses passes the
return buffer address in r3.  The value of r3 is valid on entry to the
function but the PowerPC ABI does not guarantee it will not be changed
in the function.  Hence the contents of r3 is not reliable on exit from
the function.  This issue will be addressed by the next patch in this
patch series.

The patch has been re-tested on PowerPC and on Intel X86-64 with no
regression failures.

Please let me know if this version of the patch is acceptable for the
GDB mainline.  Thanks.

                       Carl Love


----------------------------
PowerPC, function ppc64_sysv_abi_return_value add missing  return value convention

This patch address five testcase failures in gdb.cp/non-trivial-retval.exp.
The following commit resulted in the five testcases failures on PowerPC.  The
value returned by the function is being reported incorrectly.

  commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608
  Author: Andrew Burgess <aburgess@redhat.com>
  Date:   Mon Dec 13 16:56:16 2021 +0000

      gdb: on x86-64 non-trivial C++ objects are returned in memory

      Fixes PR gdb/28681.  It was observed that after using the `finish`
      command an incorrect value was displayed in some cases.  Specifically,
      this behaviour was observed on an x86-64 target.

The function:

  enum return_value_convention
  ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function,
                               struct type *valtype, struct regcache *regcache,
                               gdb_byte *readbuf, const gdb_byte *writebuf)

should return RETURN_VALUE_STRUCT_CONVENTION if the valtype->code() is
TYPE_CODE_STRUCT and if the language_pass_by_reference is not
trivially_copyable.

This patch adds the need code to return the value
RETURN_VALUE_STRUCT_CONVENTION in the case of this case.

With this patch, the five test cases still fail but with the message "Value
returned has type: A. Cannot determine contents".  The PowerPC ABI stores the
address of the buffer containing the function return value in register r3 on
entry to the function.  However, the PowerPC ABI does not guarentee that r3
will not be modified in the function.  So when the function returns, the return
buffer address cannot be reliably obtained from register r3.  Thus the message
"Cannot determine contents" is appropriate in this case.
---
 gdb/ppc-sysv-tdep.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Carl Love Nov. 7, 2022, 8:04 p.m. UTC | #1 
Ping on this patch.  I have not received any additional comments on the
patch.  I have rebased and retested both patches in the series onto the
latest gdb source tree. This patch applied cleanly on the latest tree.

GDB maintainers:

Version 3, rebased patch on the latest tree.  No additional changes
made to the patch.  Patch has been retested with no regressions.

Version 2, updated the commit log per feedback.

The following PowerPC specific patch fixes an issue of GDB reporting a
bogus return value for functions that return a non-trivial value.  The
bogus return values result in five testcase failures for test
gdb.cp/non-trivial-retval.exp.  The issue is the function
ppc64_sysv_abi_return_value does not return the correct value when the
valtype->code() is TYPE_CODE_STRUCT and the language_pass_by_reference
is not trivially_copyable.  This patch adds the needed code to return
RETURN_VALUE_STRUCT_CONVENTION in these cases.  

The testcase gdb.cp/non-trivial-retval.exp still fails as gdb now
correctly reports "Cannot determine contents" instead of the expected
values, which is correct in this case.  The PowerPC ABI uses passes the
return buffer address in r3.  The value of r3 is valid on entry to the
function but the PowerPC ABI does not guarantee it will not be changed
in the function.  Hence the contents of r3 is not reliable on exit from
the function.  This issue will be addressed by the next patch in this
patch series.

The patch has been re-tested on PowerPC and on Intel X86-64 with no
regression failures.

Please let me know if this version of the patch is acceptable for the
GDB mainline.  Thanks.

                       Carl Love

-------------------------------
PowerPC, function ppc64_sysv_abi_return_value add missing return value convention

This patch address five testcase failures in gdb.cp/non-trivial-retval.exp.
The following commit resulted in the five testcases failures on PowerPC.  The
value returned by the function is being reported incorrectly.

  commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608
  Author: Andrew Burgess <aburgess@redhat.com>
  Date:   Mon Dec 13 16:56:16 2021 +0000

      gdb: on x86-64 non-trivial C++ objects are returned in memory

      Fixes PR gdb/28681.  It was observed that after using the `finish`
      command an incorrect value was displayed in some cases.  Specifically,
      this behaviour was observed on an x86-64 target.

The function:

  enum return_value_convention
  ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function,
                               struct type *valtype, struct regcache *regcache,
                               gdb_byte *readbuf, const gdb_byte *writebuf)

should return RETURN_VALUE_STRUCT_CONVENTION if the valtype->code() is
TYPE_CODE_STRUCT and if the language_pass_by_reference is not
trivially_copyable.

This patch adds the need code to return the value
RETURN_VALUE_STRUCT_CONVENTION in the case of this case.

With this patch, the five test cases still fail but with the message "Value
returned has type: A. Cannot determine contents".  The PowerPC ABI stores the
address of the buffer containing the function return value in register r3 on
entry to the function.  However, the PowerPC ABI does not guarentee that r3
will not be modified in the function.  So when the function returns, the return
buffer address cannot be reliably obtained from register r3.  Thus the message
"Cannot determine contents" is appropriate in this case.
---
 gdb/ppc-sysv-tdep.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c
index d7f05ddc6b4..450162dd46e 100644
--- a/gdb/ppc-sysv-tdep.c
+++ b/gdb/ppc-sysv-tdep.c
@@ -2099,6 +2099,10 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function,
       return RETURN_VALUE_REGISTER_CONVENTION;
     }
 
+  if (!language_pass_by_reference (valtype).trivially_copyable
+      && valtype->code () == TYPE_CODE_STRUCT)
+    return RETURN_VALUE_STRUCT_CONVENTION;
+
   /* In the ELFv2 ABI, aggregate types of up to 16 bytes are
      returned in registers r3:r4.  */
   if (tdep->elf_abi == POWERPC_ELF_V2
Ulrich Weigand Nov. 14, 2022, 4:45 p.m. UTC | #2 
Carl Love <cel@us.ibm.com> wrote:

>-------------------------------
>PowerPC, function ppc64_sysv_abi_return_value add missing return value convention
>
>This patch address five testcase failures in gdb.cp/non-trivial-retval.exp.
>The following commit resulted in the five testcases failures on PowerPC.  The
>value returned by the function is being reported incorrectly.
>
>  commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608
>  Author: Andrew Burgess <aburgess@redhat.com>
>  Date:   Mon Dec 13 16:56:16 2021 +0000
>
>      gdb: on x86-64 non-trivial C++ objects are returned in memory
>
>      Fixes PR gdb/28681.  It was observed that after using the `finish`
>      command an incorrect value was displayed in some cases.  Specifically,
>      this behaviour was observed on an x86-64 target.
>
>The function:
>
>  enum return_value_convention
>  ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function,
>                               struct type *valtype, struct regcache *regcache,
>                               gdb_byte *readbuf, const gdb_byte *writebuf)
>
>should return RETURN_VALUE_STRUCT_CONVENTION if the valtype->code() is
>TYPE_CODE_STRUCT and if the language_pass_by_reference is not
>trivially_copyable.
>
>This patch adds the need code to return the value
>RETURN_VALUE_STRUCT_CONVENTION in the case of this case.

This still has the typos Kevin pointed out.

>With this patch, the five test cases still fail but with the message "Value
>returned has type: A. Cannot determine contents".  The PowerPC ABI stores the
>address of the buffer containing the function return value in register r3 on
>entry to the function.  However, the PowerPC ABI does not guarentee that r3
>will not be modified in the function.  So when the function returns, the return
>buffer address cannot be reliably obtained from register r3.  Thus the message
>"Cannot determine contents" is appropriate in this case.

OK with the above typos fixed.

Bye,
Ulrich
Carl Love Nov. 14, 2022, 7:38 p.m. UTC | #3 
On Mon, 2022-11-14 at 16:45 +0000, Ulrich Weigand wrote:
> Carl Love <cel@us.ibm.com> wrote:
> 
> > -------------------------------
> > PowerPC, function ppc64_sysv_abi_return_value add missing return
> > value convention
> > 
> > This patch address five testcase failures in gdb.cp/non-trivial-
> > retval.exp.
> > The following commit resulted in the five testcases failures on
> > PowerPC.  The
> > value returned by the function is being reported incorrectly.
> > 
> >  commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608
> >  Author: Andrew Burgess <aburgess@redhat.com>
> >  Date:   Mon Dec 13 16:56:16 2021 +0000
> > 
> >      gdb: on x86-64 non-trivial C++ objects are returned in memory
> > 
> >      Fixes PR gdb/28681.  It was observed that after using the
> > `finish`
> >      command an incorrect value was displayed in some
> > cases.  Specifically,
> >      this behaviour was observed on an x86-64 target.
> > 
> > The function:
> > 
> >  enum return_value_convention
> >  ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value
> > *function,
> >                               struct type *valtype, struct regcache
> > *regcache,
> >                               gdb_byte *readbuf, const gdb_byte
> > *writebuf)
> > 
> > should return RETURN_VALUE_STRUCT_CONVENTION if the valtype->code() 
> > is
> > TYPE_CODE_STRUCT and if the language_pass_by_reference is not
> > trivially_copyable.
> > 
> > This patch adds the need code to return the value
> > RETURN_VALUE_STRUCT_CONVENTION in the case of this case.
> 
> This still has the typos Kevin pointed out.
> 
> > With this patch, the five test cases still fail but with the
> > message "Value
> > returned has type: A. Cannot determine contents".  The PowerPC ABI
> > stores the
> > address of the buffer containing the function return value in
> > register r3 on
> > entry to the function.  However, the PowerPC ABI does not guarentee
> > that r3
> > will not be modified in the function.  So when the function
> > returns, the return
> > buffer address cannot be reliably obtained from register r3.  Thus
> > the message
> > "Cannot determine contents" is appropriate in this case.
> 
> OK with the above typos fixed.
> 

Yes, I had fixed the typos once.  But it seems I didn't get the patch
updated on all of the systems where I was testing/developing the patch
and inadvertently lost the updates to this patch. 

I have fixed the typos again and updated the patch on the various
test/development systems.

Sorry about that.

                                  Carl
diff mbox series

Patch

diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c
index f57c261d9dc..14effb93210 100644
--- a/gdb/ppc-sysv-tdep.c
+++ b/gdb/ppc-sysv-tdep.c
@@ -2099,6 +2099,10 @@  ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function,
       return RETURN_VALUE_REGISTER_CONVENTION;
     }
 
+  if (!language_pass_by_reference (valtype).trivially_copyable
+      && valtype->code () == TYPE_CODE_STRUCT)
+    return RETURN_VALUE_STRUCT_CONVENTION;
+
   /* In the ELFv2 ABI, aggregate types of up to 16 bytes are
      returned in registers r3:r4.  */
   if (tdep->elf_abi == POWERPC_ELF_V2