[PATCH 2/9] Add new overload of gdbarch_return_value
Tom Tromey
tromey@adacore.com
Fri Oct 7 18:01:13 GMT 2022
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(-)
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
--
2.34.3
More information about the Gdb-patches
mailing list