This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] MIPS16 FP manual call/return fixes
On Thu, 26 Apr 2012, Tom Tromey wrote:
> Maciej> Therefore I had to change the return handlers' internal API to
> Maciej> take the value of the function being handled rather than its
> Maciej> lone type, if available. This has led to adjusting the whole
> Maciej> infrastructure.
>
> It seems reasonable to me.
>
> I didn't try to read the MIPS part of the patch.
>
> Maybe Jan could read the ifunc change in elfread.c.
Jan, would you please find a spare minute to look? The changes are meant
to be straightforward, we don't need resolved_pc before we have tracked
down the ifunc resolver breakpoint, at which point we can use its location
to track down the function actually called. Then we can use its address
with gdbarch_return_value for the backend to determine the actual ABI
used.
That said (and as noted before, I am not terribly familiar with the IFUNC
feature), I suspect that any such function has to be treated as a regular
indirect function call and therefore be reached via one of the MIPS16
call/return (as applicable) stubs concerned recently, so the standard ABI
can probably be assumed.
Then again, maybe not, in a pure-MIPS16 executable (or maybe really just
a mixed-mode one that happens to call the function in question from MIPS16
code only -- there's really no sense to use hard-FP with pure-MIPS16 code
as there's no access to the FPU in the MIPS16 mode) the resolver could
optimise IFUNC lookup to the proper MIPS16 entry point.
Anyway, any thunks used only copy registers and do not clobber the
originals, so fetching the return value from the original registers is
going to work as expected. Besides, the design is meant to be generic and
work for any other platforms/ABIs that may suffer from such peculiarities
(i.e. a different calling convention on an externally determined
case-by-case basis).
The extra assertion verifies that we only have a single resolver
breakpoint assigned with this call; my understanding of the concept and
code is this must always be the case.
> Maciej> # stored into the appropriate register. This can be used when we want
> Maciej> # to force the value returned by a function (see the "return" command
> Maciej> # for instance).
> Maciej> -M:enum return_value_convention:return_value:struct type *functype, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf:functype, valtype, regcache, readbuf, writebuf
> Maciej> +M:enum return_value_convention:return_value:struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf:functype, valtype, regcache, readbuf, writebuf
>
> A couple of nits here:
>
> Update the introductory comment to refer to FUNCTION, not FUNCTYPE.
>
> Also, please s/functype/function/ in the list of argument names (at the
> end of the line).
Ugh, I must have obviously messed up something here at one point as I got
gdbarch.c right but not gdbarch.h or gdbarch.sh.
> Maciej> + CORE_ADDR faddr = BLOCK_START (SYMBOL_BLOCK_VALUE (a->function));
> Maciej> + struct value *func = allocate_value (SYMBOL_TYPE (a->function));
>
> I think read_var_value would be better here.
Thanks for the pointer, I tend to agree and regression testing looks good
after these changes. From the context I think it's safe and appropriate
to call get_current_frame from finish_command_continuation (not sure if
the frame chosen should ever matter for the calculation of any function's
address -- perhaps for nested functions?), the other places already have
a frame available.
> The rest looked good to me.
Thanks for the review, here's the resulting update. Any other comments,
anyone?
Maciej
gdb-mips16-calls-ret-val-func-fix.diff
Index: gdb-fsf-trunk-quilt/gdb/gdbarch.h
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/gdbarch.h 2012-04-27 21:31:50.085560611 +0100
+++ gdb-fsf-trunk-quilt/gdb/gdbarch.h 2012-04-27 19:50:02.965563474 +0100
@@ -433,8 +433,8 @@ typedef CORE_ADDR (gdbarch_integer_to_ad
extern CORE_ADDR gdbarch_integer_to_address (struct gdbarch *gdbarch, struct type *type, const gdb_byte *buf);
extern void set_gdbarch_integer_to_address (struct gdbarch *gdbarch, gdbarch_integer_to_address_ftype *integer_to_address);
-/* Return the return-value convention that will be used by FUNCTYPE
- to return a value of type VALTYPE. FUNCTYPE may be NULL in which
+/* 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 READBUF is not NULL, extract the return value and save it in this buffer.
Index: gdb-fsf-trunk-quilt/gdb/gdbarch.sh
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/gdbarch.sh 2012-04-27 21:31:50.085560611 +0100
+++ gdb-fsf-trunk-quilt/gdb/gdbarch.sh 2012-04-27 19:42:02.335573553 +0100
@@ -503,8 +503,8 @@ m:CORE_ADDR:pointer_to_address:struct ty
m:void:address_to_pointer:struct type *type, gdb_byte *buf, CORE_ADDR addr:type, buf, addr::unsigned_address_to_pointer::0
M:CORE_ADDR:integer_to_address:struct type *type, const gdb_byte *buf:type, buf
-# Return the return-value convention that will be used by FUNCTYPE
-# to return a value of type VALTYPE. FUNCTYPE may be NULL in which
+# 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 READBUF is not NULL, extract the return value and save it in this buffer.
@@ -513,7 +513,7 @@ M:CORE_ADDR:integer_to_address:struct ty
# 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).
-M:enum return_value_convention:return_value:struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf:functype, valtype, regcache, readbuf, writebuf
+M:enum return_value_convention:return_value:struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf:function, valtype, regcache, readbuf, writebuf
m:CORE_ADDR:skip_prologue:CORE_ADDR ip:ip:0:0
M:CORE_ADDR:skip_main_prologue:CORE_ADDR ip:ip
Index: gdb-fsf-trunk-quilt/gdb/infcmd.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/infcmd.c 2012-04-27 21:31:50.085560611 +0100
+++ gdb-fsf-trunk-quilt/gdb/infcmd.c 2012-04-27 20:08:31.935596617 +0100
@@ -1547,11 +1547,10 @@ finish_command_continuation (void *arg,
if (TYPE_CODE (value_type) != TYPE_CODE_VOID)
{
- CORE_ADDR faddr = BLOCK_START (SYMBOL_BLOCK_VALUE (a->function));
- struct value *func = allocate_value (SYMBOL_TYPE (a->function));
volatile struct gdb_exception ex;
+ struct value *func;
- set_value_address (func, faddr);
+ func = read_var_value (a->function, get_current_frame ());
TRY_CATCH (ex, RETURN_MASK_ALL)
{
/* print_return_value can throw an exception in some
Index: gdb-fsf-trunk-quilt/gdb/stack.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/stack.c 2012-04-27 21:31:50.155559621 +0100
+++ gdb-fsf-trunk-quilt/gdb/stack.c 2012-04-27 20:11:06.425607247 +0100
@@ -2284,11 +2284,7 @@ return_command (char *retval_exp, int fr
value_fetch_lazy (return_value);
if (thisfun != NULL)
- {
- function = allocate_value (SYMBOL_TYPE (thisfun));
- set_value_address (function,
- BLOCK_START (SYMBOL_BLOCK_VALUE (thisfun)));
- }
+ function = read_var_value (thisfun, thisframe);
if (TYPE_CODE (return_type) == TYPE_CODE_VOID)
/* If the return-type is "void", don't try to find the
Index: gdb-fsf-trunk-quilt/gdb/python/py-finishbreakpoint.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/python/py-finishbreakpoint.c 2012-04-27 21:31:50.195565239 +0100
+++ gdb-fsf-trunk-quilt/gdb/python/py-finishbreakpoint.c 2012-04-27 20:18:47.945561048 +0100
@@ -252,14 +252,11 @@ bpfinishpy_init (PyObject *self, PyObjec
if (TYPE_CODE (ret_type) != TYPE_CODE_VOID)
{
struct value *func_value;
- CORE_ADDR func_addr;
/* Ignore Python errors at this stage. */
self_bpfinish->return_type = type_to_type_object (ret_type);
PyErr_Clear ();
- func_value = allocate_value (SYMBOL_TYPE (function));
- func_addr = BLOCK_START (SYMBOL_BLOCK_VALUE (function));
- set_value_address (func_value, func_addr);
+ func_value = read_var_value (function, frame);
self_bpfinish->function_value =
value_to_value_object (func_value);
PyErr_Clear ();