This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[PATCH] Eliminate dwarf2_frame_cache recursion (move dwarf2_tailcall_sniffer_first elsewhere)
- From: Pedro Alves <palves at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Wed, 20 Nov 2013 18:03:15 +0000
- Subject: [PATCH] Eliminate dwarf2_frame_cache recursion (move dwarf2_tailcall_sniffer_first elsewhere)
- Authentication-results: sourceware.org; auth=none
TL;DR:
dwarf2_frame_cache recursion is evil. dwarf2_frame_cache calls
dwarf2_tailcall_sniffer_first which then recurses into
dwarf2_frame_cache. Avoid that by deferring
dwarf2_tailcall_sniffer_first until it's really necessary.
Rationale:
As an experiment, I tried making dwarf2-frame.c:read_addr_from_reg use
address_from_register. That caused a bunch of regressions, but it
actually took me a long while to figure out what was going on. Turns
out dwarf2-frame.c:read_addr_from_reg is called while computing the
frame's CFA, from within dwarf2_frame_cache. address_from_register
wants to create a register with frame_id set to the frame being
constructed. To create the frame id, we again call dwarf2_frame_cache,
which given:
static struct dwarf2_frame_cache *
dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
{
...
if (*this_cache)
return *this_cache;
returns an incomplete object to the caller:
static void
dwarf2_frame_this_id (struct frame_info *this_frame, void **this_cache,
struct frame_id *this_id)
{
struct dwarf2_frame_cache *cache =
dwarf2_frame_cache (this_frame, this_cache);
...
(*this_id) = frame_id_build (cache->cfa, get_frame_func (this_frame));
}
As cache->cfa is still 0 (we were trying to compute it!), and
get_frame_id recalls this id from here on, we end up with a broken
frame id in recorded for this frame. Later, when inspecting locals,
the dwarf machinery needs to know the selected frame's base, which
calls get_frame_base:
CORE_ADDR
get_frame_base (struct frame_info *fi)
{
return get_frame_id (fi).stack_addr;
}
which as seen above then returns 0 ...
So I gave up using address_from_register.
But, the pain of investigating this made me want to have GDB itself
assert that recursion never happens here. So I wrote a patch to do
that. But, it triggers on current mainline, because
dwarf2_tailcall_sniffer_first, called from dwarf2_frame_cache, unwinds
the this_frame.
Now, I think that as design principle, a sniffer shouldn't be trying
to unwind, exactly because of this sort of tricky issue. So the patch
defers calling dwarf2_tailcall_sniffer_first until it's really
necessary, in dwarf2_frame_prev_register (thus actually outside the
sniffer path). As this makes the call to dwarf2_frame_sniffer in
dwarf2_frame_cache unnecessary again, the patch removes that too.
Tested on x86_64 Fedora 17.
gdb/
2013-11-20 Pedro Alves <palves@redhat.com>
* dwarf2-frame.c (struct dwarf2_frame_cache)
<checked_tailcall_bottom, entry_cfa_sp_offset,
entry_cfa_sp_offset_p>: New fields.
(dwarf2_frame_cache): Adjust to use the new cache fields instead
of locals. Don't call dwarf2_tailcall_sniffer_first here.
(dwarf2_frame_prev_register): Call it here, but only once.
---
gdb/dwarf2-frame.c | 37 ++++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 91d8802..c4f8771 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -993,12 +993,22 @@ struct dwarf2_frame_cache
/* The .text offset. */
CORE_ADDR text_offset;
+ /* True if we already checked whether this frame is the bottom frame
+ of a virtual tail call frame chain. */
+ int checked_tailcall_bottom;
+
/* If not NULL then this frame is the bottom frame of a TAILCALL_FRAME
sequence. If NULL then it is a normal case with no TAILCALL_FRAME
involved. Non-bottom frames of a virtual tail call frames chain use
dwarf2_tailcall_frame_unwind unwinder so this field does not apply for
them. */
void *tailcall_cache;
+
+ /* The number of bytes to subtract from TAILCALL_FRAME frames frame
+ base to get the SP, to simulate the return address pushed on the
+ stack. */
+ LONGEST entry_cfa_sp_offset;
+ int entry_cfa_sp_offset_p;
};
/* A cleanup that sets a pointer to NULL. */
@@ -1023,8 +1033,6 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
struct dwarf2_fde *fde;
volatile struct gdb_exception ex;
CORE_ADDR entry_pc;
- LONGEST entry_cfa_sp_offset;
- int entry_cfa_sp_offset_p = 0;
const gdb_byte *instr;
if (*this_cache)
@@ -1089,8 +1097,8 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
&& (gdbarch_dwarf2_reg_to_regnum (gdbarch, fs->regs.cfa_reg)
== gdbarch_sp_regnum (gdbarch)))
{
- entry_cfa_sp_offset = fs->regs.cfa_offset;
- entry_cfa_sp_offset_p = 1;
+ cache->entry_cfa_sp_offset = fs->regs.cfa_offset;
+ cache->entry_cfa_sp_offset_p = 1;
}
}
else
@@ -1239,13 +1247,6 @@ incomplete CFI data; unspecified registers (e.g., %s) at %s"),
cache->undefined_retaddr = 1;
do_cleanups (old_chain);
-
- /* Try to find a virtual tail call frames chain with bottom (callee) frame
- starting at THIS_FRAME. */
- dwarf2_tailcall_sniffer_first (this_frame, &cache->tailcall_cache,
- (entry_cfa_sp_offset_p
- ? &entry_cfa_sp_offset : NULL));
-
discard_cleanups (reset_cache_cleanup);
return cache;
}
@@ -1292,6 +1293,16 @@ dwarf2_frame_prev_register (struct frame_info *this_frame, void **this_cache,
CORE_ADDR addr;
int realnum;
+ /* Check whether THIS_FRAME is the bottom frame of a virtual tail
+ call frame chain. */
+ if (!cache->checked_tailcall_bottom)
+ {
+ cache->checked_tailcall_bottom = 1;
+ dwarf2_tailcall_sniffer_first (this_frame, &cache->tailcall_cache,
+ (cache->entry_cfa_sp_offset_p
+ ? &cache->entry_cfa_sp_offset : NULL));
+ }
+
/* Non-bottom frames of a virtual tail call frames chain use
dwarf2_tailcall_frame_unwind unwinder so this code does not apply for
them. If dwarf2_tailcall_prev_register_first does not have specific value
@@ -1418,10 +1429,6 @@ dwarf2_frame_sniffer (const struct frame_unwind *self,
if (self->type != NORMAL_FRAME)
return 0;
- /* Preinitializa the cache so that TAILCALL_FRAME can find the record by
- dwarf2_tailcall_sniffer_first. */
- dwarf2_frame_cache (this_frame, this_cache);
-
return 1;
}
--
1.7.11.7