This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
[patch rfc] per-architecture unwind_pc()
- From: Andrew Cagney <ac131313 at redhat dot com>
- To: gdb-patches at sources dot redhat dot com
- Date: Thu, 06 Mar 2003 14:00:13 -0500
- Subject: [patch rfc] per-architecture unwind_pc()
Hello,
The original code was using FRAME_SAVED_PC() to obtain the caller's PC
value. A typical implementation looked something like:
if (frame is dummy)
return pc from dummy frame;
else if (frame is sigtramp)
return pc saved in sigtram;
else if (frame is normal)
return pc saved on stack;
In changing to per-frame unwinders, it was restructured in the `obvious'
way, each frame type gained a specalized pc_unwind method:
frame->pc_unwind (frame);
where the dummy unwinder is:
dummy_frame_pc_unwind(frame)
{ return pc from dummy frame; }
The change missed something even more telling:
- a frame's PC can be computed solely from that frame's register values
For instance, say an ISA has a CIA register (current instruction
address) that contains (among other things) the address of the next
instruction to be executed, then the value of the unwound `PC' is simply
the adjusted value of the unwound CIA register vis:
isa_unwind_pc(f)
ULONGEST cia;
frame_unwind_unsigned_register (f, CIA_REGNUM, &cia);
return cia & ~3; // discard low bits
Significantly, this works regardless of the frame F's type (dummy,
sigtramp, normal, sentinel, ...). The only assumption is that F's
register unwind method works(1).
The attached patch replaces the per-frame pc_unwind() method with the
per-architecture, frame agnostic, unwind_pc() method:
+ at item CORE_ADDR unwind_pc (struct frame_info * at var{next_frame})
+ at findex unwind_pc
+ at anchor{unwind_pc} Return the instruction address in @var{next_frame}'s
+caller at which execution will resume after @var{next_frame} returns.
+This is commonly refered to as the return address.
+
+The implementation, which must be frame agnostic (work with any frame),
+is typically no more than:
+
+ at smallexample
+ULONGEST pc;
+frame_unwind_unsigned_register (next_frame, D10V_PC_REGNUM, &pc);
+return d10v_make_iaddr (pc);
+ at end smallexample
+
+ at noindent
+ at xref{FRAME_SAVED_PC}, which this method replaces.
It then modifies frame_pc_unwind() to strongly prefer this method over
all others.
When the new gdbarch unwind_pc method isn't available, frame_pc_unwind()
falls back to the more traditional FRAME_SAVED_PC() and read_pc()
functions. That way existing code continues to work unmodified.
Thoughts. I'm going to leave this until next week before committing.
Andrew
(1) It turns out that this is a slightly bigger ask then one might
expect. The d10v register unwind code, for instance, wasn't correctly
handling a request for an unwound PC.
Index: ChangeLog
2003-03-06 Andrew Cagney <cagney at redhat dot com>
* gdbarch.sh (gdbarch_unwind_pc): New method.
* gdbarch.h, gdbarch.c: Regenerate.
* frame.c (frame_pc_unwind): Rewrite. Prefer gdbarch_unwind_pc,
but use read_pc and FRAME_SAVED_PC as fall backs.
(frame_saved_regs_pc_unwind): Delete function.
(trad_frame_unwinder): Update.
* frame-unwind.h (frame_unwind_pc_ftype): Delete declaration.
(struct frame_unwind): Update.
* dummy-frame.c (dummy_frame_pc_unwind): Delete function.
(dummy_frame_unwind): Update.
* sentinel-frame.c (sentinel_frame_pc_unwind): Delete function.
(sentinel_frame_unwinder): Update.
Index: doc/ChangeLog
2003-03-06 Andrew Cagney <cagney at redhat dot com>
* gdbint.texinfo (Target Architecture Definition): Cross reference
FRAME_SAVED_PC to unwind_pc. Document unwind_pc.
Index: dummy-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dummy-frame.c,v
retrieving revision 1.10
diff -u -r1.10 dummy-frame.c
--- dummy-frame.c 19 Jan 2003 17:39:16 -0000 1.10
+++ dummy-frame.c 6 Mar 2003 18:05:41 -0000
@@ -370,23 +370,6 @@
}
}
-/* Assuming that FRAME is a dummy, return the resume address for the
- previous frame. */
-
-static CORE_ADDR
-dummy_frame_pc_unwind (struct frame_info *frame,
- void **cache)
-{
- struct dummy_frame *dummy = cached_find_dummy_frame (frame, cache);
- /* Oops! In a dummy-frame but can't find the stack dummy. Pretend
- that the frame doesn't unwind. Should this function instead
- return a has-no-caller indication? */
- if (dummy == NULL)
- return 0;
- return dummy->pc;
-}
-
-
/* Assuming that FRAME is a dummy, return the ID of the calling frame
(the frame that the dummy has the saved state of). */
@@ -408,7 +391,6 @@
static struct frame_unwind dummy_frame_unwind =
{
dummy_frame_pop,
- dummy_frame_pc_unwind,
dummy_frame_id_unwind,
dummy_frame_register_unwind
};
Index: frame-unwind.h
===================================================================
RCS file: /cvs/src/src/gdb/frame-unwind.h,v
retrieving revision 1.3
diff -u -r1.3 frame-unwind.h
--- frame-unwind.h 19 Jan 2003 17:39:16 -0000 1.3
+++ frame-unwind.h 6 Mar 2003 18:05:41 -0000
@@ -69,12 +69,6 @@
CORE_ADDR *addrp,
int *realnump, void *valuep);
-/* Same as for registers above, but return the address at which the
- calling frame would resume. */
-
-typedef CORE_ADDR (frame_unwind_pc_ftype) (struct frame_info * frame,
- void **unwind_cache);
-
/* Same as for registers above, but return the ID of the frame that
called this one. */
@@ -103,7 +97,6 @@
/* Should an attribute indicating the frame's address-in-block go
here? */
frame_unwind_pop_ftype *pop;
- frame_unwind_pc_ftype *pc;
frame_unwind_id_ftype *id;
frame_unwind_reg_ftype *reg;
};
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.74
diff -u -r1.74 frame.c
--- frame.c 5 Mar 2003 23:14:17 -0000 1.74
+++ frame.c 6 Mar 2003 18:05:41 -0000
@@ -135,14 +135,54 @@
}
CORE_ADDR
-frame_pc_unwind (struct frame_info *frame)
+frame_pc_unwind (struct frame_info *next_frame)
{
- if (!frame->pc_unwind_cache_p)
+ if (!next_frame->pc_unwind_cache_p)
{
- frame->pc_unwind_cache = frame->unwind->pc (frame, &frame->unwind_cache);
- frame->pc_unwind_cache_p = 1;
+ CORE_ADDR pc;
+ if (gdbarch_unwind_pc_p (current_gdbarch))
+ {
+ /* The right way. The `pure' way. The one true way. This
+ method depends solely on the register-unwind code to
+ determine the value of registers in THIS frame, and hence
+ the value of this frame's PC (resume address). A typical
+ implementation is no more than:
+
+ frame_unwind_register (next_frame, ISA_PC_REGNUM, buf);
+ return extract_address (buf, size of ISA_PC_REGNUM);
+
+ Note: this method is very heavily dependent on a correct
+ register-unwind implementation, it pays to fix that
+ method first; this method is frame type agnostic, since
+ it only deals with register values, it works with any
+ frame. This is all in stark contrast to the old
+ FRAME_SAVED_PC which would try to directly handle all the
+ different ways that a PC could be unwound. */
+ pc = gdbarch_unwind_pc (current_gdbarch, next_frame);
+ }
+ else if (next_frame->level < 0)
+ {
+ /* FIXME: cagney/2003-03-06: Old code and and a sentinel
+ frame. Do like was always done. Fetch the PC's value
+ direct from regcache. This assumes that this frame belongs to
+ the current global register cache. The assumption is
+ dumb. */
+ pc = read_pc ();
+ }
+ else if (FRAME_SAVED_PC_P ())
+ {
+ /* FIXME: cagney/2003-03-06: Old code, but not a sentinel
+ frame. Do like was always done. Note that this method,
+ unlike unwind_pc(), tries to handle all the different
+ frame cases directly. It fails. */
+ pc = FRAME_SAVED_PC (next_frame);
+ }
+ else
+ internal_error (__FILE__, __LINE__, "No gdbarch_unwind_pc method");
+ next_frame->pc_unwind_cache = pc;
+ next_frame->pc_unwind_cache_p = 1;
}
- return frame->pc_unwind_cache;
+ return next_frame->pc_unwind_cache;
}
void
@@ -667,13 +707,6 @@
bufferp);
}
-static CORE_ADDR
-frame_saved_regs_pc_unwind (struct frame_info *frame, void **cache)
-{
- gdb_assert (FRAME_SAVED_PC_P ());
- return FRAME_SAVED_PC (frame);
-}
-
static void
frame_saved_regs_id_unwind (struct frame_info *next_frame, void **cache,
struct frame_id *id)
@@ -745,7 +778,6 @@
const struct frame_unwind trad_frame_unwinder = {
frame_saved_regs_pop,
- frame_saved_regs_pc_unwind,
frame_saved_regs_id_unwind,
frame_saved_regs_register_unwind
};
Index: gdbarch.sh
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.200
diff -u -r1.200 gdbarch.sh
--- gdbarch.sh 5 Mar 2003 23:14:17 -0000 1.200
+++ gdbarch.sh 6 Mar 2003 18:05:41 -0000
@@ -591,7 +591,9 @@
f:2:FRAMELESS_FUNCTION_INVOCATION:int:frameless_function_invocation:struct frame_info *fi:fi:::generic_frameless_function_invocation_not::0
F:2:FRAME_CHAIN:CORE_ADDR:frame_chain:struct frame_info *frame:frame::0:0
F:2:FRAME_CHAIN_VALID:int:frame_chain_valid:CORE_ADDR chain, struct frame_info *thisframe:chain, thisframe::0:0
+# NOTE: FRAME_SAVED_PC is replaced by UNWIND_PC
F:2:FRAME_SAVED_PC:CORE_ADDR:frame_saved_pc:struct frame_info *fi:fi::0:0
+M::UNWIND_PC:CORE_ADDR:unwind_pc:struct frame_info *next_frame:next_frame:
f:2:FRAME_ARGS_ADDRESS:CORE_ADDR:frame_args_address:struct frame_info *fi:fi::0:get_frame_base::0
f:2:FRAME_LOCALS_ADDRESS:CORE_ADDR:frame_locals_address:struct frame_info *fi:fi::0:get_frame_base::0
f:2:SAVED_PC_AFTER_CALL:CORE_ADDR:saved_pc_after_call:struct frame_info *frame:frame::0:0
Index: sentinel-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/sentinel-frame.c,v
retrieving revision 1.2
diff -u -r1.2 sentinel-frame.c
--- sentinel-frame.c 27 Jan 2003 21:41:41 -0000 1.2
+++ sentinel-frame.c 6 Mar 2003 18:05:41 -0000
@@ -70,17 +70,6 @@
}
}
-CORE_ADDR
-sentinel_frame_pc_unwind (struct frame_info *frame,
- void **cache)
-{
- /* FIXME: cagney/2003-01-08: This should be using a per-architecture
- method that doesn't suffer from DECR_PC_AFTER_BREAK problems.
- Such a method would take unwind_cache, regcache and stop reason
- parameters. */
- return read_pc ();
-}
-
void
sentinel_frame_id_unwind (struct frame_info *frame,
void **cache,
@@ -105,7 +94,6 @@
const struct frame_unwind sentinel_frame_unwinder =
{
sentinel_frame_pop,
- sentinel_frame_pc_unwind,
sentinel_frame_id_unwind,
sentinel_frame_register_unwind
};
Index: doc/gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.127
diff -u -r1.127 gdbint.texinfo
--- doc/gdbint.texinfo 5 Mar 2003 23:14:18 -0000 1.127
+++ doc/gdbint.texinfo 6 Mar 2003 18:05:42 -0000
@@ -3270,8 +3270,28 @@
@item FRAME_SAVED_PC(@var{frame})
@findex FRAME_SAVED_PC
-Given @var{frame}, return the pc saved there. This is the return
-address.
+ at anchor{FRAME_SAVED_PC} Given @var{frame}, return the pc saved there.
+This is the return address.
+
+This method is deprecated. @xref{unwind_pc}.
+
+ at item CORE_ADDR unwind_pc (struct frame_info * at var{next_frame})
+ at findex unwind_pc
+ at anchor{unwind_pc} Return the instruction address in @var{next_frame}'s
+caller at which execution will resume after @var{next_frame} returns.
+This is commonly refered to as the return address.
+
+The implementation, which must be frame agnostic (work with any frame),
+is typically no more than:
+
+ at smallexample
+ULONGEST pc;
+frame_unwind_unsigned_register (next_frame, D10V_PC_REGNUM, &pc);
+return d10v_make_iaddr (pc);
+ at end smallexample
+
+ at noindent
+ at xref{FRAME_SAVED_PC}, which this method replaces.
@item FUNCTION_EPILOGUE_SIZE
@findex FUNCTION_EPILOGUE_SIZE