[2/9] Add new overload of gdbarch_return_value

Message ID 20221007180120.1866772-3-tromey@adacore.com
State Committed
Headers
Series Fix "finish" with variably-sized types |

Commit Message

Tom Tromey Oct. 7, 2022, 6:01 p.m. UTC
  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          | 18 ++++++++++++++++++
 gdb/arch-utils.h          |  6 ++++++
 gdb/elfread.c             |  4 ++--
 gdb/gdbarch-components.py | 36 ++++++++++++++++++++++++++++++++++--
 gdb/gdbarch-gen.h         | 21 +++++++++++++++++++--
 gdb/gdbarch.c             | 36 +++++++++++++++++++++++++-----------
 gdb/infcall.c             |  7 +++----
 gdb/infcmd.c              |  9 ++++-----
 gdb/stack.c               |  7 ++++---
 gdb/value.c               |  4 ++--
 10 files changed, 117 insertions(+), 31 deletions(-)
  

Patch

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 77269425ddc..677e405fd6c 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -1092,6 +1092,24 @@  default_read_core_file_mappings
 {
 }
 
+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
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index f850e5fd6e7..2e1f5a561f6 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -300,4 +300,10 @@  extern void default_read_core_file_mappings
    struct bfd *cbfd,
    read_core_file_mappings_pre_loop_ftype pre_loop_cb,
    read_core_file_mappings_loop_ftype loop_cb);
+
+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 */
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 8aee634b44b..a6c7ead9558 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1018,8 +1018,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 ());
diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py
index b8d7648d5ec..844eb4f161d 100644
--- a/gdb/gdbarch-components.py
+++ b/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)",
 )
 
 Method(
diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
index 383d84f8142..4ec11fd4eee 100644
--- a/gdb/gdbarch-gen.h
+++ b/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 true if the return value of function is stored in the first hidden
    parameter.  In theory, this feature should be language-dependent, specified
    by language and its ABI, such as C++.  Unfortunately, compiler may
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 4b0c10be402..911aa0ec277 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -116,6 +116,7 @@  struct gdbarch
   gdbarch_address_to_pointer_ftype *address_to_pointer = nullptr;
   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 = nullptr;
   gdbarch_return_in_first_hidden_param_p_ftype *return_in_first_hidden_param_p = nullptr;
   gdbarch_skip_prologue_ftype *skip_prologue = nullptr;
   gdbarch_skip_main_prologue_ftype *skip_main_prologue = nullptr;
@@ -313,6 +314,7 @@  gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->value_from_register = default_value_from_register;
   gdbarch->pointer_to_address = unsigned_pointer_to_address;
   gdbarch->address_to_pointer = unsigned_address_to_pointer;
+  gdbarch->return_value_as_value = default_gdbarch_return_value;
   gdbarch->return_in_first_hidden_param_p = default_return_in_first_hidden_param_p;
   gdbarch->breakpoint_from_pc = default_breakpoint_from_pc;
   gdbarch->sw_breakpoint_from_kind = NULL;
@@ -464,7 +466,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 return_in_first_hidden_param_p, invalid_p == 0 */
   if (gdbarch->skip_prologue == 0)
     log.puts ("\n\tskip_prologue");
@@ -871,12 +875,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: return_in_first_hidden_param_p = <%s>\n",
                       host_address_to_string (gdbarch->return_in_first_hidden_param_p));
@@ -2660,13 +2664,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)
 {
@@ -2684,6 +2681,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;
+}
+
 int
 gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch, struct type *type)
 {
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 34852191043..21270ef48d3 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -457,10 +457,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
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index decd61111b7..b66d5e73784 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1485,15 +1485,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,
-				NULL, NULL, NULL))
+  switch (gdbarch_return_value_as_value (gdbarch, function, value_type,
+					 NULL, NULL, NULL))
     {
     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 (), NULL);
+      gdbarch_return_value_as_value (gdbarch, function, value_type, stop_regs,
+				     &value, NULL);
       break;
     case RETURN_VALUE_STRUCT_CONVENTION:
       value = NULL;
diff --git a/gdb/stack.c b/gdb/stack.c
index 379635e409e..f2b70d20f4d 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2835,9 +2835,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
diff --git a/gdb/value.c b/gdb/value.c
index 8ed941f3749..e79022e740c 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3878,8 +3878,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