Add new overload of gdbarch_return_value
authorTom Tromey <tromey@adacore.com>
Wed, 7 Sep 2022 14:39:52 +0000 (08:39 -0600)
committerTom Tromey <tromey@adacore.com>
Tue, 3 Jan 2023 15:45:00 +0000 (08:45 -0700)
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
gdb/arch-utils.h
gdb/elfread.c
gdb/gdbarch-components.py
gdb/gdbarch-gen.h
gdb/gdbarch.c
gdb/infcall.c
gdb/infcmd.c
gdb/stack.c
gdb/value.c

index a6d0035f97381e760af895db541f7d7e18128d84..3ca193085cb4df94ff4e3787b6a7f4da79d9eb0e 100644 (file)
@@ -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
index 9671e50c0dcdd6033e7b9912c95b7c533939ea50..56690f0fd435ec9670edace5a1c6c02abf865521 100644 (file)
@@ -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 */
index ceaf81f0fca179761e487733e3f9c3eb9175da53..ccee37406c877e0bd4db7d1320068e8f464c30b5 100644 (file)
@@ -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 ());
index ca9c3bceb3dea728c1f8c7664c541bd55ba97388..0e1ca40102d1ae453803af1d0611083055d2c603 100644 (file)
@@ -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(
index c2299dcf6e6e9deb1ce3b9c467103859d0224a32..128efa10bef8d8106ee6fa46ace25a9db37ca080 100644 (file)
@@ -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
index 5469b1355ce7b40b39507bc8fff1f8d3538cd3ab..353040cf020a095dead9fb2a3a32ea3583c475c6 100644 (file)
@@ -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)
 {
index ec2aa180daa564ca5ff91e86e3890bebda8e20f8..acaaf0c029d086a4ed017fe6629b7e11551e756f 100644 (file)
@@ -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
index 0c1765d418d01647ad6a55d7bd79868da6ceceb0..0497ad0509111ed03ae653f43fcc4f0266784a2b 100644 (file)
@@ -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)
index 7f7478ea32390762b18a55767028b4998eb2aae1..0fd797836dcfed0f2975d607f8af6cce39f31cb0 100644 (file)
@@ -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
index 7d14dc4a5d8312d8a3555d5cb930b6592cd05944..e3f60e7e9891219cb908c7557ef1445a6886c30b 100644 (file)
@@ -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