From: Tom Tromey Date: Wed, 7 Sep 2022 14:39:52 +0000 (-0600) Subject: Add new overload of gdbarch_return_value X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=4e1d2f5814b2;p=binutils-gdb.git 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. --- diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c index a6d0035f973..3ca193085cb 100644 --- a/gdb/arch-utils.c +++ b/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 diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h index 9671e50c0dc..56690f0fd43 100644 --- a/gdb/arch-utils.h +++ b/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 */ diff --git a/gdb/elfread.c b/gdb/elfread.c index ceaf81f0fca..ccee37406c8 100644 --- a/gdb/elfread.c +++ b/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 ()); diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py index ca9c3bceb3d..0e1ca40102d 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)", ) Function( diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index c2299dcf6e6..128efa10bef 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 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 diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index 5469b1355ce..353040cf020 100644 --- a/gdb/gdbarch.c +++ b/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) { diff --git a/gdb/infcall.c b/gdb/infcall.c index ec2aa180daa..acaaf0c029d 100644 --- a/gdb/infcall.c +++ b/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 diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 0c1765d418d..0497ad05091 100644 --- a/gdb/infcmd.c +++ b/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) diff --git a/gdb/stack.c b/gdb/stack.c index 7f7478ea323..0fd797836dc 100644 --- a/gdb/stack.c +++ b/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 diff --git a/gdb/value.c b/gdb/value.c index 7d14dc4a5d8..e3f60e7e989 100644 --- a/gdb/value.c +++ b/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