This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch 04/12] entryval: Virtual tail call frames
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Cc: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Date: Tue, 19 Jul 2011 19:17:05 +0100
- Subject: Re: [patch 04/12] entryval: Virtual tail call frames
- References: <20110718201658.GE30496@host1.jankratochvil.net>
This (and gcc/dwarf's side of the) work is really impressive. Kudos!
I'll shoot a couple of comments below on a couple of hunks
I noticed on a first skim of this patch.
Sorry for the quick comments. This definitely deserves a thorougher read.
On Monday 18 July 2011 21:16:58, Jan Kratochvil wrote:
> +static void
> +tailcall_frame_this_id (struct frame_info *this_frame, void **this_cache,
> + struct frame_id *this_id)
> +{
> + struct tailcall_cache *cache = *this_cache;
> + struct frame_info *next_frame;
> + CORE_ADDR this_frame_base;
> +
> + /* Tail call does not make sense for a sentinel frame. */
> + next_frame = get_next_frame (this_frame);
> + gdb_assert (next_frame != NULL);
> +
> + /* SP does not change during tail calls. */
> + this_frame_base = get_frame_base (next_frame);
Should probably preserve frame_id->special_addr as well, for ia64.
> +
> + (*this_id) = frame_id_build (this_frame_base, get_frame_pc (this_frame));
> + (*this_id).inline_depth = (cache->chain_levels
> + - existing_next_levels (this_frame, cache));
> + gdb_assert ((*this_id).inline_depth > 0);
> +}
> @@ -1285,6 +1297,15 @@ dwarf2_frame_prev_register (struct frame_info *this_frame, void **this_cache,
> CORE_ADDR addr;
> int realnum;
>
> + /* Virtual tail call frames report different values only for PC. 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 (cache->tailcall_cache && regnum == gdbarch_pc_regnum (gdbarch))
> + return dwarf2_tailcall_frame_unwind.prev_register (this_frame,
> + &cache->tailcall_cache,
> + regnum);
> +
This looked strange. Isn't it breaking some abstraction?
I would have expected dwarf2_tailcall_frame_unwind.prev_register
to be called _first_ (automatically by frame_prev_register) and then
have that defer to dwarf2_frame_prev_register when necessary.
--
Pedro Alves