public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@adacore.com>
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tromey@adacore.com>
Subject: [PATCH 7/9] Fix inferior calls with variably-sized return type
Date: Fri,  7 Oct 2022 12:01:18 -0600	[thread overview]
Message-ID: <20221007180120.1866772-8-tromey@adacore.com> (raw)
In-Reply-To: <20221007180120.1866772-1-tromey@adacore.com>

This patch updates the gdbarch_return_value_as_value implementations
to work correctly with variably-sized return types.
---
 gdb/aarch64-tdep.c       | 22 +++++++++---------
 gdb/amd64-tdep.c         | 18 +++++++--------
 gdb/amd64-windows-tdep.c | 19 +++++++--------
 gdb/arm-tdep.c           | 30 +++++++++++++++---------
 gdb/i386-tdep.c          | 22 ++++++++----------
 gdb/riscv-tdep.c         | 50 +++++++++++++++++++++-------------------
 gdb/sparc-tdep.c         | 19 +++++++--------
 7 files changed, 91 insertions(+), 89 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 6d24f5ab495..1ba3228a037 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2335,6 +2335,9 @@ aarch64_return_in_memory (struct gdbarch *gdbarch, struct type *type)
   int elements;
   struct type *fundamental_type;
 
+  if (TYPE_HAS_DYNAMIC_LENGTH (type))
+    return 1;
+
   if (aapcs_is_vfp_call_or_return_candidate (type, &elements,
 					     &fundamental_type))
     {
@@ -2449,13 +2452,6 @@ aarch64_return_value (struct gdbarch *gdbarch, struct value *func_value,
 		      struct type *valtype, struct regcache *regcache,
 		      struct value **read_value, const gdb_byte *writebuf)
 {
-  gdb_byte *readbuf = nullptr;
-  if (read_value != nullptr)
-    {
-      *read_value = allocate_value (valtype);
-      readbuf = value_contents_raw (*read_value).data ();
-    }
-
   if (valtype->code () == TYPE_CODE_STRUCT
       || valtype->code () == TYPE_CODE_UNION
       || valtype->code () == TYPE_CODE_ARRAY)
@@ -2471,12 +2467,12 @@ aarch64_return_value (struct gdbarch *gdbarch, struct value *func_value,
 
 	  aarch64_debug_printf ("return value in memory");
 
-	  if (readbuf)
+	  if (read_value != nullptr)
 	    {
 	      CORE_ADDR addr;
 
 	      regcache->cooked_read (AARCH64_STRUCT_RETURN_REGNUM, &addr);
-	      read_memory (addr, readbuf, valtype->length ());
+	      *read_value = value_at_non_lval (valtype, addr);
 	    }
 
 	  return RETURN_VALUE_ABI_RETURNS_ADDRESS;
@@ -2486,8 +2482,12 @@ aarch64_return_value (struct gdbarch *gdbarch, struct value *func_value,
   if (writebuf)
     aarch64_store_return_value (valtype, regcache, writebuf);
 
-  if (readbuf)
-    aarch64_extract_return_value (valtype, regcache, readbuf);
+  if (read_value)
+    {
+      *read_value = allocate_value (valtype);
+      aarch64_extract_return_value (valtype, regcache,
+				    value_contents_raw (*read_value).data ());
+    }
 
   aarch64_debug_printf ("return value in registers");
 
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index c250c07e4ce..e6f9f3eca32 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -801,13 +801,6 @@ amd64_return_value (struct gdbarch *gdbarch, struct value *function,
 
   gdb_assert (!(read_value && writebuf));
 
-  gdb_byte *readbuf = nullptr;
-  if (read_value != nullptr)
-    {
-      *read_value = allocate_value (type);
-      readbuf = value_contents_raw (*read_value).data ();
-    }
-
   /* 1. Classify the return type with the classification algorithm.  */
   amd64_classify (type, theclass);
 
@@ -824,17 +817,24 @@ amd64_return_value (struct gdbarch *gdbarch, struct value *function,
 	 can always find the return value just after the function has
 	 returned.  */
 
-      if (readbuf)
+      if (read_value != nullptr)
 	{
 	  ULONGEST addr;
 
 	  regcache_raw_read_unsigned (regcache, AMD64_RAX_REGNUM, &addr);
-	  read_memory (addr, readbuf, type->length ());
+	  *read_value = value_at_non_lval (type, addr);
 	}
 
       return RETURN_VALUE_ABI_RETURNS_ADDRESS;
     }
 
+  gdb_byte *readbuf = nullptr;
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (type);
+      readbuf = value_contents_raw (*read_value).data ();
+    }
+
   /* 8. If the class is COMPLEX_X87, the real part of the value is
 	returned in %st0 and the imaginary part in %st1.  */
   if (theclass[0] == AMD64_COMPLEX_X87)
diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index 05a5990f1e0..1d366417075 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -360,13 +360,6 @@ amd64_windows_return_value (struct gdbarch *gdbarch, struct value *function,
   int len = type->length ();
   int regnum = -1;
 
-  gdb_byte *readbuf = nullptr;
-  if (read_value != nullptr)
-    {
-      *read_value = allocate_value (type);
-      readbuf = value_contents_raw (*read_value).data ();
-    }
-
   /* See if our value is returned through a register.  If it is, then
      store the associated register number in REGNUM.  */
   switch (type->code ())
@@ -401,20 +394,24 @@ amd64_windows_return_value (struct gdbarch *gdbarch, struct value *function,
   if (regnum < 0)
     {
       /* RAX contains the address where the return value has been stored.  */
-      if (readbuf)
+      if (read_value != nullptr)
 	{
 	  ULONGEST addr;
 
 	  regcache_raw_read_unsigned (regcache, AMD64_RAX_REGNUM, &addr);
-	  read_memory (addr, readbuf, type->length ());
+	  *read_value = value_at_non_lval (type, addr);
 	}
       return RETURN_VALUE_ABI_RETURNS_ADDRESS;
     }
   else
     {
       /* Extract the return value from the register where it was stored.  */
-      if (readbuf)
-	regcache->raw_read_part (regnum, 0, len, readbuf);
+      if (read_value != nullptr)
+	{
+	  *read_value = allocate_value (type);
+	  regcache->raw_read_part (regnum, 0, len,
+				   value_contents_raw (*read_value).data ());
+	}
       if (writebuf)
 	regcache->raw_write_part (regnum, 0, len, writebuf);
       return RETURN_VALUE_REGISTER_CONVENTION;
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 132c10b1ca6..edc11253627 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -8808,6 +8808,9 @@ arm_return_in_memory (struct gdbarch *gdbarch, struct type *type)
       && TYPE_CODE_ARRAY != code && TYPE_CODE_COMPLEX != code)
     return 0;
 
+  if (TYPE_HAS_DYNAMIC_LENGTH (type))
+    return 1;
+
   if (TYPE_CODE_ARRAY == code && type->is_vector ())
     {
       /* Vector values should be returned using ARM registers if they
@@ -9010,13 +9013,6 @@ arm_return_value (struct gdbarch *gdbarch, struct value *function,
 		  struct type *valtype, struct regcache *regcache,
 		  struct value **read_value, const gdb_byte *writebuf)
 {
-  gdb_byte *readbuf = nullptr;
-  if (read_value != nullptr)
-    {
-      *read_value = allocate_value (valtype);
-      readbuf = value_contents_raw (*read_value).data ();
-    }
-
   arm_gdbarch_tdep *tdep = gdbarch_tdep<arm_gdbarch_tdep> (gdbarch);
   struct type *func_type = function ? value_type (function) : NULL;
   enum arm_vfp_cprc_base_type vfp_base_type;
@@ -9028,6 +9024,14 @@ arm_return_value (struct gdbarch *gdbarch, struct value *function,
       int reg_char = arm_vfp_cprc_reg_char (vfp_base_type);
       int unit_length = arm_vfp_cprc_unit_length (vfp_base_type);
       int i;
+
+      gdb_byte *readbuf = nullptr;
+      if (read_value != nullptr)
+	{
+	  *read_value = allocate_value (valtype);
+	  readbuf = value_contents_raw (*read_value).data ();
+	}
+
       for (i = 0; i < vfp_base_count; i++)
 	{
 	  if (reg_char == 'q')
@@ -9079,12 +9083,12 @@ arm_return_value (struct gdbarch *gdbarch, struct value *function,
       if (tdep->struct_return == pcc_struct_return
 	  || arm_return_in_memory (gdbarch, valtype))
 	{
-	  if (readbuf)
+	  if (read_value != nullptr)
 	    {
 	      CORE_ADDR addr;
 
 	      regcache->cooked_read (ARM_A1_REGNUM, &addr);
-	      read_memory (addr, readbuf, valtype->length ());
+	      *read_value = value_at_non_lval (valtype, addr);
 	    }
 	  return RETURN_VALUE_ABI_RETURNS_ADDRESS;
 	}
@@ -9098,8 +9102,12 @@ arm_return_value (struct gdbarch *gdbarch, struct value *function,
   if (writebuf)
     arm_store_return_value (valtype, regcache, writebuf);
 
-  if (readbuf)
-    arm_extract_return_value (valtype, regcache, readbuf);
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (valtype);
+      gdb_byte *readbuf = value_contents_raw (*read_value).data ();
+      arm_extract_return_value (valtype, regcache, readbuf);
+    }
 
   return RETURN_VALUE_REGISTER_CONVENTION;
 }
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index e609b7c833e..ebc6ebd7d8b 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -3008,7 +3008,8 @@ i386_reg_struct_return_p (struct gdbarch *gdbarch, struct type *type)
 
   if (struct_convention == pcc_struct_convention
       || (struct_convention == default_struct_convention
-	  && tdep->struct_return == pcc_struct_return))
+	  && tdep->struct_return == pcc_struct_return)
+      || TYPE_HAS_DYNAMIC_LENGTH (type))
     return 0;
 
   /* Structures consisting of a single `float', `double' or 'long
@@ -3036,13 +3037,6 @@ i386_return_value (struct gdbarch *gdbarch, struct value *function,
 {
   enum type_code code = type->code ();
 
-  gdb_byte *readbuf = nullptr;
-  if (read_value != nullptr)
-    {
-      *read_value = allocate_value (type);
-      readbuf = value_contents_raw (*read_value).data ();
-    }
-
   if (((code == TYPE_CODE_STRUCT
 	|| code == TYPE_CODE_UNION
 	|| code == TYPE_CODE_ARRAY)
@@ -3070,12 +3064,12 @@ i386_return_value (struct gdbarch *gdbarch, struct value *function,
 	 a record, so the convention applied to records also applies
 	 to arrays.  */
 
-      if (readbuf)
+      if (read_value != nullptr)
 	{
 	  ULONGEST addr;
 
 	  regcache_raw_read_unsigned (regcache, I386_EAX_REGNUM, &addr);
-	  read_memory (addr, readbuf, type->length ());
+	  *read_value = value_at_non_lval (type, addr);
 	}
 
       return RETURN_VALUE_ABI_RETURNS_ADDRESS;
@@ -3099,8 +3093,12 @@ i386_return_value (struct gdbarch *gdbarch, struct value *function,
       return result;
     }
 
-  if (readbuf)
-    i386_extract_return_value (gdbarch, type, regcache, readbuf);
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (type);
+      i386_extract_return_value (gdbarch, type, regcache,
+				 value_contents_raw (*read_value).data ());
+    }
   if (writebuf)
     i386_store_return_value (gdbarch, type, regcache, writebuf);
 
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index ed92ff67c7b..26a5e91c74f 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -2492,7 +2492,8 @@ static void
 riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo,
 			   struct riscv_call_info *cinfo)
 {
-  if (ainfo->length > (2 * cinfo->xlen))
+  if (TYPE_HAS_DYNAMIC_LENGTH (ainfo->type)
+      || ainfo->length > (2 * cinfo->xlen))
     {
       /* Argument is going to be passed by reference.  */
       ainfo->argloc[0].loc_type
@@ -2910,8 +2911,12 @@ riscv_arg_location (struct gdbarch *gdbarch,
       break;
 
     case TYPE_CODE_STRUCT:
-      riscv_call_arg_struct (ainfo, cinfo);
-      break;
+      if (!TYPE_HAS_DYNAMIC_LENGTH (ainfo->type))
+	{
+	  riscv_call_arg_struct (ainfo, cinfo);
+	  break;
+	}
+      /* FALLTHROUGH */
 
     default:
       riscv_call_arg_scalar_int (ainfo, cinfo);
@@ -3228,13 +3233,6 @@ riscv_return_value (struct gdbarch  *gdbarch,
   struct riscv_arg_info info;
   struct type *arg_type;
 
-  gdb_byte *readbuf = nullptr;
-  if (read_value != nullptr)
-    {
-      *read_value = allocate_value (type);
-      readbuf = value_contents_raw (*read_value).data ();
-    }
-
   arg_type = check_typedef (type);
   riscv_arg_location (gdbarch, &info, &call_info, arg_type, false);
 
@@ -3246,15 +3244,15 @@ riscv_return_value (struct gdbarch  *gdbarch,
       gdb_printf (gdb_stdlog, "\n");
     }
 
-  if (readbuf != nullptr || writebuf != nullptr)
+  if (read_value != nullptr || writebuf != nullptr)
     {
       unsigned int arg_len;
       struct value *abi_val;
-      gdb_byte *old_readbuf = nullptr;
+      gdb_byte *readbuf = nullptr;
       int regnum;
 
       /* We only do one thing at a time.  */
-      gdb_assert (readbuf == nullptr || writebuf == nullptr);
+      gdb_assert (read_value == nullptr || writebuf == nullptr);
 
       /* In some cases the argument is not returned as the declared type,
 	 and we need to cast to or from the ABI type in order to
@@ -3295,7 +3293,6 @@ riscv_return_value (struct gdbarch  *gdbarch,
       else
 	{
 	  abi_val = allocate_value (info.type);
-	  old_readbuf = readbuf;
 	  readbuf = value_contents_raw (abi_val).data ();
 	}
       arg_len = info.type->length ();
@@ -3375,8 +3372,17 @@ riscv_return_value (struct gdbarch  *gdbarch,
 
 	    regcache_cooked_read_unsigned (regcache, RISCV_A0_REGNUM,
 					   &addr);
-	    if (readbuf != nullptr)
-	      read_memory (addr, readbuf, info.length);
+	    if (read_value != nullptr)
+	      {
+		abi_val = value_at_non_lval (type, addr);
+		/* Also reset the expected type, so that the cast
+		   later on is a no-op.  If the cast is not a no-op,
+		   and if the return type is variably-sized, then the
+		   type of ABI_VAL will differ from ARG_TYPE due to
+		   dynamic type resolution, and so will most likely
+		   fail.  */
+		arg_type = value_type (abi_val);
+	      }
 	    if (writebuf != nullptr)
 	      write_memory (addr, writebuf, info.length);
 	  }
@@ -3391,10 +3397,8 @@ riscv_return_value (struct gdbarch  *gdbarch,
       /* This completes the cast from abi type back to the declared type
 	 in the case that we are reading from the machine.  See the
 	 comment at the head of this block for more details.  */
-      if (readbuf != nullptr)
+      if (read_value != nullptr)
 	{
-	  struct value *arg_val;
-
 	  if (is_fixed_point_type (arg_type))
 	    {
 	      /* Convert abi_val to the actual return type, but
@@ -3405,15 +3409,13 @@ riscv_return_value (struct gdbarch  *gdbarch,
 	      unscaled.read (value_contents (abi_val),
 			     type_byte_order (info.type),
 			     info.type->is_unsigned ());
-	      arg_val = allocate_value (arg_type);
-	      unscaled.write (value_contents_raw (arg_val),
+	      *read_value = allocate_value (arg_type);
+	      unscaled.write (value_contents_raw (*read_value),
 			      type_byte_order (arg_type),
 			      arg_type->is_unsigned ());
 	    }
 	  else
-	    arg_val = value_cast (arg_type, abi_val);
-	  memcpy (old_readbuf, value_contents_raw (arg_val).data (),
-		  arg_type->length ());
+	    *read_value = value_cast (arg_type, abi_val);
 	}
     }
 
diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index bbb98ef5ed7..c2d1ed15273 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -1501,13 +1501,6 @@ sparc32_return_value (struct gdbarch *gdbarch, struct value *function,
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
 
-  gdb_byte *readbuf = nullptr;
-  if (read_value != nullptr)
-    {
-      *read_value = allocate_value (type);
-      readbuf = value_contents_raw (*read_value).data ();
-    }
-
   /* The psABI says that "...every stack frame reserves the word at
      %fp+64.  If a function returns a structure, union, or
      quad-precision value, this word should hold the address of the
@@ -1520,11 +1513,11 @@ sparc32_return_value (struct gdbarch *gdbarch, struct value *function,
       ULONGEST sp;
       CORE_ADDR addr;
 
-      if (readbuf)
+      if (read_value != nullptr)
 	{
 	  regcache_cooked_read_unsigned (regcache, SPARC_SP_REGNUM, &sp);
 	  addr = read_memory_unsigned_integer (sp + 64, 4, byte_order);
-	  read_memory (addr, readbuf, type->length ());
+	  *read_value = value_at_non_lval (type, addr);
 	}
       if (writebuf)
 	{
@@ -1536,8 +1529,12 @@ sparc32_return_value (struct gdbarch *gdbarch, struct value *function,
       return RETURN_VALUE_ABI_PRESERVES_ADDRESS;
     }
 
-  if (readbuf)
-    sparc32_extract_return_value (type, regcache, readbuf);
+  if (read_value != nullptr)
+    {
+      *read_value = allocate_value (type);
+      gdb_byte *readbuf = value_contents_raw (*read_value).data ();
+      sparc32_extract_return_value (type, regcache, readbuf);
+    }
   if (writebuf)
     sparc32_store_return_value (type, regcache, writebuf);
 
-- 
2.34.3


  parent reply	other threads:[~2022-10-07 18:01 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-07 18:01 [PATCH 0/9] Fix "finish" with variably-sized types Tom Tromey
2022-10-07 18:01 ` [PATCH 1/9] Fix crash in amd64-tdep.c Tom Tromey
2022-10-07 18:01 ` [PATCH 2/9] Add new overload of gdbarch_return_value Tom Tromey
2022-10-07 18:01 ` [PATCH 3/9] Don't emit gdbarch_return_value Tom Tromey
2022-10-07 18:01 ` [PATCH 4/9] Introduce value_at_non_lval Tom Tromey
2022-10-07 18:01 ` [PATCH 5/9] Don't let property evaluation affect the current language Tom Tromey
2022-10-07 18:01 ` [PATCH 6/9] Convert selected architectures to gdbarch_return_value_as_value Tom Tromey
2022-10-07 18:01 ` Tom Tromey [this message]
2022-10-07 18:01 ` [PATCH 8/9] Use value_at_non_lval in get_call_return_value Tom Tromey
2022-10-07 18:01 ` [PATCH 9/9] Add test case for "finish" with variably-sized types Tom Tromey
2023-01-03 16:44 ` [PATCH 0/9] Fix " Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221007180120.1866772-8-tromey@adacore.com \
    --to=tromey@adacore.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).