This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
[patch rfc] push_dummy_call return a frame ID; eliminate need forread_fp
- From: Andrew Cagney <ac131313 at redhat dot com>
- To: gdb-patches at sources dot redhat dot com,Mark Kettenis <kettenis at chello dot nl>
- Date: Tue, 22 Apr 2003 18:24:19 -0400
- Subject: [patch rfc] push_dummy_call return a frame ID; eliminate need forread_fp
Hello,
Now that the inferior function call code is all in a single function,
more, and significant, cleanups become possible.
This patch modifies the architecture method push_dummy_call() so that it
returns a frame ID, and not just a stack pointer. That frame ID is then
used:
- to create the breakpoint (the ID identifies the breakpoint's frame)
- to identify the dummy (somewhat indirectly at present as it still goes
via SAVE_DUMMY_FRAME_TOS() and generic_save_call_dummy_addr())
This means that:
- push_dummy_call(regcache, ...) creates the dummy frame's ID
- unwind_dummy_id(frame, ...) is then expected to return that same value
so the need for one to match the other is hopefully both clear and easy ...
Future changes will involve:
- deprecating read_fp / TARGET_READ_FP, as core GDB no longer contains
any legitimate calls
- merging SAVE_DUMMY_FRAME_TOS and generic_save_call_dummy_addr() into a
single method that receives the full frame ID, and can not be overridden
by the architecture.
Someone studying this carefully will probably realise that instead of
having push_dummy_call() return the ID, the code could simply use:
flush cached frames;
id = get_frame_id (get_current_frame ());
but that would incure additional overhead.
Mark,
I think this patch resolves the i386's FP/SP frame ID problem. It makes
push_dummy_call responsible for computing the ID, and then just passes
it through.
Richard,
For the ARM, I've pulled a quick hack. I'm not sure what would happen
if the ARM started returning a full frame ID.
I'll leave this one sit for a week.
Andrew
2003-04-22 Andrew Cagney <cagney at redhat dot com>
* d10v-tdep.c (d10v_push_dummy_call): Return a frame ID.
(d10v_gdbarch_init): Do not set read_fp.
(d10v_read_fp): Delete function.
* arm-tdep.c (arm_push_dummy_call): Return the SP in a frame ID.
* infcall.c (call_function_by_hand): New variable "id".
Initialize using gdbarch_push_dummy_call when possible. For
legacy code, use the the frame ID's "stack_addr" instead of "sp"
to track the stack's inner most address.
* gdbarch.sh (PUSH_DUMMY_CALL): Return the frame ID instead of the
stack's inner most address.
* gdbarch.h, gdbarch.c: Re-generate.
Index: doc/ChangeLog
2003-04-22 Andrew Cagney <cagney at redhat dot com>
* gdbint.texinfo (Target Architecture Definition): Update
"push_dummy_call", returns a frame ID. Cross reference
"push_dummy_call" and "unwind_dummy_id". Drop reference to
SAVE_DUMMY_FRAME_TOS.
Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.130
diff -u -r1.130 arm-tdep.c
--- arm-tdep.c 21 Apr 2003 16:48:37 -0000 1.130
+++ arm-tdep.c 22 Apr 2003 22:03:03 -0000
@@ -1404,7 +1404,7 @@
conforms with GCC's default model. Several other variants exist and
we should probably support some of them based on the selected ABI. */
-static CORE_ADDR
+static struct frame_id
arm_push_dummy_call (struct gdbarch *gdbarch, struct regcache *regcache,
CORE_ADDR dummy_addr, int nargs, struct value **args,
CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr)
@@ -1520,7 +1520,12 @@
/* Finally, update teh SP register. */
regcache_cooked_write_unsigned (regcache, ARM_SP_REGNUM, sp);
- return sp;
+ /* FIXME: cagney/2003-04-22: Return just the SP via the frame ID.
+ Yes, this is a hack. It should instead return the complete frame
+ ID of the dummy frame, but that will only work when
+ call_function_by_hand starts saving the ID in the dummy frame
+ info. */
+ return frame_id_build (sp, 0);
}
static void
Index: d10v-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/d10v-tdep.c,v
retrieving revision 1.106
diff -u -r1.106 d10v-tdep.c
--- d10v-tdep.c 11 Apr 2003 03:12:58 -0000 1.106
+++ d10v-tdep.c 22 Apr 2003 22:03:03 -0000
@@ -93,8 +93,6 @@
static CORE_ADDR d10v_read_sp (void);
-static CORE_ADDR d10v_read_fp (void);
-
static void d10v_eva_prepare_to_trace (void);
static void d10v_eva_get_trace_data (void);
@@ -941,12 +939,6 @@
return (d10v_make_daddr (read_register (SP_REGNUM)));
}
-static CORE_ADDR
-d10v_read_fp (void)
-{
- return (d10v_make_daddr (read_register (D10V_FP_REGNUM)));
-}
-
/* When arguments must be pushed onto the stack, they go on in reverse
order. The below implements a FILO (stack) to do this. */
@@ -983,7 +975,7 @@
}
-static CORE_ADDR
+static struct frame_id
d10v_push_dummy_call (struct gdbarch *gdbarch, struct regcache *regcache,
CORE_ADDR dummy_addr, int nargs, struct value **args,
CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr)
@@ -1059,7 +1051,12 @@
regcache_cooked_write_unsigned (regcache, SP_REGNUM,
d10v_convert_daddr_to_raw (sp));
- return sp;
+ /* FIXME: cagney/2003-04-22: Return just the SP via the frame ID.
+ Yes, this is a hack. It should instead return the complete frame
+ ID of the dummy frame, but that will only work when
+ call_function_by_hand starts saving the ID in the dummy frame
+ info. */
+ return frame_id_build (sp, dummy_addr);
}
@@ -1629,7 +1626,6 @@
set_gdbarch_read_pc (gdbarch, d10v_read_pc);
set_gdbarch_write_pc (gdbarch, d10v_write_pc);
- set_gdbarch_read_fp (gdbarch, d10v_read_fp);
set_gdbarch_read_sp (gdbarch, d10v_read_sp);
set_gdbarch_num_regs (gdbarch, d10v_num_regs);
Index: gdbarch.sh
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.225
diff -u -r1.225 gdbarch.sh
--- gdbarch.sh 12 Apr 2003 17:41:25 -0000 1.225
+++ gdbarch.sh 22 Apr 2003 22:03:05 -0000
@@ -556,7 +556,7 @@
f:2:RETURN_VALUE_ON_STACK:int:return_value_on_stack:struct type *type:type:::generic_return_value_on_stack_not::0
# Replaced by PUSH_DUMMY_CALL
F:2:DEPRECATED_PUSH_ARGUMENTS:CORE_ADDR:deprecated_push_arguments:int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr:nargs, args, sp, struct_return, struct_addr
-M::PUSH_DUMMY_CALL:CORE_ADDR:push_dummy_call:struct regcache *regcache, CORE_ADDR dummy_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr:regcache, dummy_addr, nargs, args, sp, struct_return, struct_addr
+M::PUSH_DUMMY_CALL:struct frame_id:push_dummy_call:struct regcache *regcache, CORE_ADDR dummy_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr:regcache, dummy_addr, nargs, args, sp, struct_return, struct_addr
F:2:DEPRECATED_PUSH_DUMMY_FRAME:void:deprecated_push_dummy_frame:void:-:::0
# NOTE: This can be handled directly in push_dummy_call.
F:2:DEPRECATED_PUSH_RETURN_ADDRESS:CORE_ADDR:deprecated_push_return_address:CORE_ADDR pc, CORE_ADDR sp:pc, sp:::0
Index: infcall.c
===================================================================
RCS file: /cvs/src/src/gdb/infcall.c,v
retrieving revision 1.1
diff -u -r1.1 infcall.c
--- infcall.c 21 Apr 2003 16:48:39 -0000 1.1
+++ infcall.c 22 Apr 2003 22:03:06 -0000
@@ -272,6 +272,8 @@
struct type *param_type = NULL;
struct type *ftype = check_typedef (SYMBOL_TYPE (function));
int n_method_args = 0;
+ struct frame_id id;
+ CORE_ADDR bp_addr;
dummy = alloca (SIZEOF_CALL_DUMMY_WORDS);
sizeof_dummy1 = REGISTER_SIZE * SIZEOF_CALL_DUMMY_WORDS / sizeof (ULONGEST);
@@ -628,15 +630,24 @@
/* When there is no push_dummy_call method, should this code
simply error out. That would the implementation of this method
for all ABIs (which is probably a good thing). */
- sp = gdbarch_push_dummy_call (current_gdbarch, current_regcache,
- dummy_addr, nargs, args, sp, struct_return,
- struct_addr);
+ id = gdbarch_push_dummy_call (current_gdbarch, current_regcache,
+ dummy_addr, nargs, args, sp,
+ struct_return, struct_addr);
else if (DEPRECATED_PUSH_ARGUMENTS_P ())
/* Keep old targets working. */
- sp = DEPRECATED_PUSH_ARGUMENTS (nargs, args, sp, struct_return,
- struct_addr);
+ id = frame_id_build (DEPRECATED_PUSH_ARGUMENTS (nargs, args, sp,
+ struct_return,
+ struct_addr), 0);
else
- sp = legacy_push_arguments (nargs, args, sp, struct_return, struct_addr);
+ id = frame_id_build (legacy_push_arguments (nargs, args, sp,
+ struct_return,
+ struct_addr), 0);
+
+ /* From now on, the frame, and not the SP is used when tracking the
+ inner-most stack address. Note that since all the below updates
+ involve deprecated methods, they will all eventually just go away
+ leaving "frame" with the true dummy frame's ID. */
+ sp = 0;
if (DEPRECATED_PUSH_RETURN_ADDRESS_P ())
/* for targets that use no CALL_DUMMY */
@@ -649,7 +660,7 @@
return-address register as appropriate. Formerly this has been
done in PUSH_ARGUMENTS, but that's overloading its
functionality a bit, so I'm making it explicit to do it here. */
- sp = DEPRECATED_PUSH_RETURN_ADDRESS (real_pc, sp);
+ id.stack_addr = DEPRECATED_PUSH_RETURN_ADDRESS (real_pc, id.stack_addr);
/* NOTE: cagney/2003-03-23: Diable this code when there is a
push_dummy_call() method. Since that method will have already
@@ -661,8 +672,8 @@
/* If stack grows up, we must leave a hole at the bottom, note
that sp already has been advanced for the arguments! */
if (DEPRECATED_CALL_DUMMY_STACK_ADJUST_P ())
- sp += DEPRECATED_CALL_DUMMY_STACK_ADJUST;
- sp = STACK_ALIGN (sp);
+ id.stack_addr += DEPRECATED_CALL_DUMMY_STACK_ADJUST;
+ id.stack_addr = STACK_ALIGN (id.stack_addr);
}
/* XXX This seems wrong. For stacks that grow down we shouldn't do
@@ -674,7 +685,7 @@
if (INNER_THAN (1, 2))
{
/* stack grows downward */
- sp -= DEPRECATED_CALL_DUMMY_STACK_ADJUST;
+ id.stack_addr -= DEPRECATED_CALL_DUMMY_STACK_ADJUST;
}
/* Store the address at which the structure is supposed to be
@@ -682,7 +693,7 @@
/* NOTE: 2003-03-24: Since PUSH_ARGUMENTS can (and typically does)
store the struct return address, this call is entirely redundant. */
if (struct_return && DEPRECATED_STORE_STRUCT_RETURN_P ())
- DEPRECATED_STORE_STRUCT_RETURN (struct_addr, sp);
+ DEPRECATED_STORE_STRUCT_RETURN (struct_addr, id.stack_addr);
/* Write the stack pointer. This is here because the statements above
might fool with it. On SPARC, this write also stores the register
@@ -694,10 +705,10 @@
frame), and none of the code following that code adjusts the
stack-pointer value, the below call is entirely redundant. */
if (DEPRECATED_DUMMY_WRITE_SP_P ())
- DEPRECATED_DUMMY_WRITE_SP (sp);
+ DEPRECATED_DUMMY_WRITE_SP (id.stack_addr);
if (SAVE_DUMMY_FRAME_TOS_P ())
- SAVE_DUMMY_FRAME_TOS (sp);
+ SAVE_DUMMY_FRAME_TOS (id.stack_addr);
{
char *name;
@@ -774,17 +785,21 @@
}
sal.section = find_pc_overlay (sal.pc);
- {
+ if (id.code_addr == 0)
/* Set up a frame ID for the dummy frame so we can pass it to
set_momentary_breakpoint. We need to give the breakpoint a
frame ID so that the breakpoint code can correctly
- re-identify the dummy breakpoint. */
- struct frame_id frame = frame_id_build (read_fp (), sal.pc);
- /* Create a momentary breakpoint at the return address of the
- inferior. That way it breaks when it returns. */
- bpt = set_momentary_breakpoint (sal, frame, bp_call_dummy);
- bpt->disposition = disp_del;
- }
+ re-identify the dummy breakpoint. Code using
+ push_dummy_call() will have previously computed the dummy
+ frame's ID (and hence set "code_addr" to something
+ non-zero. */
+ id = frame_id_build (read_fp (), sal.pc);
+ /* else ... is the sal.pc the frame ID's code_addr? */
+
+ /* Create a momentary breakpoint at the return address of the
+ inferior. That way it breaks when it returns. */
+ bpt = set_momentary_breakpoint (sal, id, bp_call_dummy);
+ bpt->disposition = disp_del;
/* If all error()s out of proceed ended up calling normal_stop
(and perhaps they should; it already does in the special case
Index: doc/gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.140
diff -u -r1.140 gdbint.texinfo
--- doc/gdbint.texinfo 8 Apr 2003 21:25:33 -0000 1.140
+++ doc/gdbint.texinfo 22 Apr 2003 22:03:09 -0000
@@ -3696,13 +3696,16 @@
@item push_dummy_call (@var{gdbarch}, @var{regcache}, @var{dummy_addr}, @var{nargs}, @var{args}, @var{sp}, @var{struct_return}, @var{struct_addr})
@findex push_dummy_call
@findex DEPRECATED_PUSH_ARGUMENTS.
- at anchor{push_dummy_call}
-Define this to push the dummy frame's call to the inferior function onto
-the stack. In addition to pushing @var{nargs}, the code should push
- at var{struct_addr} (when @var{struct_return}), and the return value (in
-the call dummy at @var{dummy_addr}).
-
-Returns the updated top-of-stack pointer.
+ at anchor{push_dummy_call} Define this to push the dummy frame's call to
+the inferior function onto the stack. In addition to pushing
+ at var{nargs}, the code should push @var{struct_addr} (when
+ at var{struct_return}), and the address of the breakpoint that the
+function should return to (located in the call dummy at
+ at var{dummy_addr}).
+
+Returns a @code{struct frame_id} that uniquely identifies an inferior
+function call's dummy frame. That same value must also be returned by
+ at code{unwind_dummy_id} (@pxref{unwind_dummy_id}).
This method replaces @code{DEPRECATED_PUSH_ARGUMENTS}.
@@ -3918,9 +3921,8 @@
@findex unwind_dummy_id
@anchor{unwind_dummy_id} Given @var{frame} return a @code{struct
frame_id} that uniquely identifies an inferior function call's dummy
-frame. The value returned must match the dummy frame stack value
-previously saved using @code{SAVE_DUMMY_FRAME_TOS}.
- at xref{SAVE_DUMMY_FRAME_TOS} dot
+frame. The value must match that previously returned by
+ at code{push_dummy_call} (@pxref{push_dummy_call}).
@item USE_STRUCT_CONVENTION (@var{gcc_p}, @var{type})
@findex USE_STRUCT_CONVENTION