Add new overload of gdbarch_return_value

The gdbarch "return_value" can't correctly handle variably-sized
types.  The problem here is that the TYPE_LENGTH of such a type is 0,
until the type is resolved, which requires reading memory.  However,
gdbarch_return_value only accepts a buffer as an out parameter.

Fixing this requires letting the implementation of the gdbarch method
resolve the type and return a value -- that is, both the contents and
the new type.

After an attempt at this, I realized I wouldn't be able to correctly
update all implementations (there are ~80) of this method.  So,
instead, this patch adds a new method that falls back to the current
method, and it updates gdb to only call the new method.  This way it's
possible to incrementally convert the architectures that I am able to
test.

gdb/arch-utils.c

@@ -1098,6 +1098,24 @@ default_get_return_buf_addr (struct type *val_type, frame_info_ptr cur_frame)
   return 0;
 }
 
+enum return_value_convention
+default_gdbarch_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 ();
+    }
+
+  return gdbarch_return_value (gdbarch, function, valtype, regcache,
+			       readbuf, writebuf);
+}
+
 /* Non-zero if we want to trace architecture code.  */
 
 #ifndef GDBARCH_DEBUG

gdb/arch-utils.h

@@ -308,4 +308,10 @@ extern void default_read_core_file_mappings
 /* Default implementation of gdbarch default_get_return_buf_addr method.  */
 extern CORE_ADDR default_get_return_buf_addr (struct type *val_typegdbarch,
 					      frame_info_ptr cur_frame);
+
+extern enum return_value_convention default_gdbarch_return_value
+     (struct gdbarch *gdbarch, struct value *function, struct type *valtype,
+      struct regcache *regcache, struct value **read_value,
+      const gdb_byte *writebuf);
+
 #endif /* ARCH_UTILS_H */

gdb/elfread.c

@@ -1038,8 +1038,8 @@ elf_gnu_ifunc_resolver_return_stop (code_breakpoint *b)
   set_value_address (func_func, b->loc->related_address);
 
   value = allocate_value (value_type);
-  gdbarch_return_value (gdbarch, func_func, value_type, regcache,
-			value_contents_raw (value).data (), NULL);
+  gdbarch_return_value_as_value (gdbarch, func_func, value_type, regcache,
+				 &value, NULL);
   resolved_address = value_as_address (value);
   resolved_pc = gdbarch_convert_from_func_ptr_addr
     (gdbarch, resolved_address, current_inferior ()->top_target ());

gdb/gdbarch-components.py

@@ -854,6 +854,9 @@ If WRITEBUF is not NULL, it contains a return value which will be
 stored into the appropriate register.  This can be used when we want
 to force the value returned by a function (see the "return" command
 for instance).
+
+NOTE: it is better to implement return_value_as_value instead, as that
+method can properly handle variably-sized types.
 """,
     type="enum return_value_convention",
     name="return_value",
@@ -864,8 +867,37 @@ for instance).
         ("gdb_byte *", "readbuf"),
         ("const gdb_byte *", "writebuf"),
     ],
-    predicate=True,
-    invalid=True,
+    invalid=False,
+)
+
+Method(
+    comment="""
+Return the return-value convention that will be used by FUNCTION
+to return a value of type VALTYPE.  FUNCTION may be NULL in which
+case the return convention is computed based only on VALTYPE.
+
+If READ_VALUE is not NULL, extract the return value and save it in
+this pointer.
+
+If WRITEBUF is not NULL, it contains a return value which will be
+stored into the appropriate register.  This can be used when we want
+to force the value returned by a function (see the "return" command
+for instance).
+""",
+    type="enum return_value_convention",
+    name="return_value_as_value",
+    params=[
+        ("struct value *", "function"),
+        ("struct type *", "valtype"),
+        ("struct regcache *", "regcache"),
+        ("struct value **", "read_value"),
+        ("const gdb_byte *", "writebuf"),
+    ],
+    predefault="default_gdbarch_return_value",
+    # If we're using the default, then the other method must be set;
+    # but if we aren't using the default here then the other method
+    # must not be set.
+    invalid="(gdbarch->return_value_as_value == default_gdbarch_return_value) == (gdbarch->return_value == nullptr)",
 )
 
 Function(

gdb/gdbarch-gen.h

@@ -433,14 +433,31 @@ extern void set_gdbarch_integer_to_address (struct gdbarch *gdbarch, gdbarch_int
    If WRITEBUF is not NULL, it contains a return value which will be
    stored into the appropriate register.  This can be used when we want
    to force the value returned by a function (see the "return" command
-   for instance). */
+   for instance).
 
-extern bool gdbarch_return_value_p (struct gdbarch *gdbarch);
+   NOTE: it is better to implement return_value_as_value instead, as that
+   method can properly handle variably-sized types. */
 
 typedef enum return_value_convention (gdbarch_return_value_ftype) (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf);
 extern enum return_value_convention gdbarch_return_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf);
 extern void set_gdbarch_return_value (struct gdbarch *gdbarch, gdbarch_return_value_ftype *return_value);
 
+/* Return the return-value convention that will be used by FUNCTION
+   to return a value of type VALTYPE.  FUNCTION may be NULL in which
+   case the return convention is computed based only on VALTYPE.
+
+   If READ_VALUE is not NULL, extract the return value and save it in
+   this pointer.
+
+   If WRITEBUF is not NULL, it contains a return value which will be
+   stored into the appropriate register.  This can be used when we want
+   to force the value returned by a function (see the "return" command
+   for instance). */
+
+typedef enum return_value_convention (gdbarch_return_value_as_value_ftype) (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, struct value **read_value, const gdb_byte *writebuf);
+extern enum return_value_convention gdbarch_return_value_as_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, struct value **read_value, const gdb_byte *writebuf);
+extern void set_gdbarch_return_value_as_value (struct gdbarch *gdbarch, gdbarch_return_value_as_value_ftype *return_value_as_value);
+
 /* Return the address at which the value being returned from
    the current function will be stored.  This routine is only
    called if the current function uses the the "struct return

gdb/gdbarch.c

@@ -116,6 +116,7 @@ struct gdbarch
   gdbarch_address_to_pointer_ftype *address_to_pointer = unsigned_address_to_pointer;
   gdbarch_integer_to_address_ftype *integer_to_address = nullptr;
   gdbarch_return_value_ftype *return_value = nullptr;
+  gdbarch_return_value_as_value_ftype *return_value_as_value = default_gdbarch_return_value;
   gdbarch_get_return_buf_addr_ftype *get_return_buf_addr = default_get_return_buf_addr;
   gdbarch_return_in_first_hidden_param_p_ftype *return_in_first_hidden_param_p = default_return_in_first_hidden_param_p;
   gdbarch_skip_prologue_ftype *skip_prologue = 0;
@@ -369,7 +370,9 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of pointer_to_address, invalid_p == 0 */
   /* Skip verify of address_to_pointer, invalid_p == 0 */
   /* Skip verify of integer_to_address, has predicate.  */
-  /* Skip verify of return_value, has predicate.  */
+  /* Skip verify of return_value, invalid_p == 0 */
+  if ((gdbarch->return_value_as_value == default_gdbarch_return_value) == (gdbarch->return_value == nullptr))
+    log.puts ("\n\treturn_value_as_value");
   /* Skip verify of get_return_buf_addr, invalid_p == 0 */
   /* Skip verify of return_in_first_hidden_param_p, invalid_p == 0 */
   if (gdbarch->skip_prologue == 0)
@@ -776,12 +779,12 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
   gdb_printf (file,
 	      "gdbarch_dump: integer_to_address = <%s>\n",
 	      host_address_to_string (gdbarch->integer_to_address));
-  gdb_printf (file,
-	      "gdbarch_dump: gdbarch_return_value_p() = %d\n",
-	      gdbarch_return_value_p (gdbarch));
   gdb_printf (file,
 	      "gdbarch_dump: return_value = <%s>\n",
 	      host_address_to_string (gdbarch->return_value));
+  gdb_printf (file,
+	      "gdbarch_dump: return_value_as_value = <%s>\n",
+	      host_address_to_string (gdbarch->return_value_as_value));
   gdb_printf (file,
 	      "gdbarch_dump: get_return_buf_addr = <%s>\n",
 	      host_address_to_string (gdbarch->get_return_buf_addr));
@@ -2568,13 +2571,6 @@ set_gdbarch_integer_to_address (struct gdbarch *gdbarch,
   gdbarch->integer_to_address = integer_to_address;
 }
 
-bool
-gdbarch_return_value_p (struct gdbarch *gdbarch)
-{
-  gdb_assert (gdbarch != NULL);
-  return gdbarch->return_value != NULL;
-}
-
 enum return_value_convention
 gdbarch_return_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf)
 {
@@ -2592,6 +2588,23 @@ set_gdbarch_return_value (struct gdbarch *gdbarch,
   gdbarch->return_value = return_value;
 }
 
+enum return_value_convention
+gdbarch_return_value_as_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, struct value **read_value, const gdb_byte *writebuf)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->return_value_as_value != NULL);
+  if (gdbarch_debug >= 2)
+    gdb_printf (gdb_stdlog, "gdbarch_return_value_as_value called\n");
+  return gdbarch->return_value_as_value (gdbarch, function, valtype, regcache, read_value, writebuf);
+}
+
+void
+set_gdbarch_return_value_as_value (struct gdbarch *gdbarch,
+				   gdbarch_return_value_as_value_ftype return_value_as_value)
+{
+  gdbarch->return_value_as_value = return_value_as_value;
+}
+
 CORE_ADDR
 gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info_ptr cur_frame)
 {

gdb/infcall.c

@@ -485,10 +485,9 @@ get_call_return_value (struct call_return_meta_info *ri)
     }
   else
     {
-      retval = allocate_value (ri->value_type);
-      gdbarch_return_value (ri->gdbarch, ri->function, ri->value_type,
-			    get_current_regcache (),
-			    value_contents_raw (retval).data (), NULL);
+      gdbarch_return_value_as_value (ri->gdbarch, ri->function, ri->value_type,
+				     get_current_regcache (),
+				     &retval, NULL);
       if (stack_temporaries && class_or_union_p (ri->value_type))
 	{
 	  /* Values of class type returned in registers are copied onto

gdb/infcmd.c

@@ -1497,15 +1497,14 @@ get_return_value (struct symbol *func_symbol, struct value *function)
      inferior function call code.  In fact, when inferior function
      calls are made async, this will likely be made the norm.  */
 
-  switch (gdbarch_return_value (gdbarch, function, value_type,
-				nullptr, nullptr, nullptr))
+  switch (gdbarch_return_value_as_value (gdbarch, function, value_type,
+					 nullptr, nullptr, nullptr))
     {
     case RETURN_VALUE_REGISTER_CONVENTION:
     case RETURN_VALUE_ABI_RETURNS_ADDRESS:
     case RETURN_VALUE_ABI_PRESERVES_ADDRESS:
-      value = allocate_value (value_type);
-      gdbarch_return_value (gdbarch, function, value_type, stop_regs,
-			    value_contents_raw (value).data (), nullptr);
+      gdbarch_return_value_as_value (gdbarch, function, value_type, stop_regs,
+				     &value, nullptr);
       break;
     case RETURN_VALUE_STRUCT_CONVENTION:
       value = nullptr;
@@ -1889,10 +1888,11 @@ finish_command (const char *arg, int from_tty)
       struct type * val_type
 	= check_typedef (sm->function->type ()->target_type ());
 
-      return_value = gdbarch_return_value (gdbarch,
-					   read_var_value (sm->function, nullptr,
-							   callee_frame),
-					   val_type, nullptr, nullptr, nullptr);
+      return_value
+	= gdbarch_return_value_as_value (gdbarch,
+					 read_var_value (sm->function, nullptr,
+							 callee_frame),
+					 val_type, nullptr, nullptr, nullptr);
 
       if (return_value == RETURN_VALUE_STRUCT_CONVENTION
 	  && val_type->code () != TYPE_CODE_VOID)

gdb/stack.c

@@ -2838,9 +2838,10 @@ return_command (const char *retval_exp, int from_tty)
 
       gdb_assert (rv_conv != RETURN_VALUE_STRUCT_CONVENTION
 		  && rv_conv != RETURN_VALUE_ABI_RETURNS_ADDRESS);
-      gdbarch_return_value (cache_arch, function, return_type,
-			    get_current_regcache (), NULL /*read*/,
-			    value_contents (return_value).data () /*write*/);
+      gdbarch_return_value_as_value
+	(cache_arch, function, return_type,
+	 get_current_regcache (), NULL /*read*/,
+	 value_contents (return_value).data () /*write*/);
     }
 
   /* If we are at the end of a call dummy now, pop the dummy frame

gdb/value.c

@@ -3957,8 +3957,8 @@ struct_return_convention (struct gdbarch *gdbarch,
     error (_("Function return type unknown."));
 
   /* Probe the architecture for the return-value convention.  */
-  return gdbarch_return_value (gdbarch, function, value_type,
-			       NULL, NULL, NULL);
+  return gdbarch_return_value_as_value (gdbarch, function, value_type,
+					NULL, NULL, NULL);
 }
 
 /* Return true if the function returning the specified type is using