Re: [PATCH v2 1/2] Change calculation of frame_id by amd64 epilogue unwinder

[Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>]

Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

> When GDB is stopped at a ret instruction and no debug information is
> available for unwinding, GDB defaults to the amd64 epilogue unwinder, to
> be able to generate a decent backtrace. However, when calculating the
> frame id, the epilogue unwinder generates information as if the return
> instruction was the whole frame.
>
> This was an issue especially when attempting to reverse debug, as GDB
> would place a step_resume_breakpoint from the epilogue of a function if
> we were to attempt to skip that function, and this breakpoint should
> ideally have the current function's frame_id to avoid other problems
> such as PR record/16678.
>
> This commit changes the frame_id calculation for the amd64 epilogue,
> so that it is always the same as the dwarf2 unwinder's frame_id.
> ---
>  gdb/amd64-tdep.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> index d89e06d27cb..17c82ac919c 100644
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -2943,7 +2943,7 @@ amd64_epilogue_frame_cache (struct frame_info *this_frame, void **this_cache)
>  					      byte_order) + cache->sp_offset;
>  
>        /* Cache pc will be the frame func.  */
> -      cache->pc = get_frame_pc (this_frame);
> +      cache->pc = get_frame_func (this_frame);
>  

Not for fixing in this patch, but I notice that
amd64_sigtramp_frame_this_id also uses get_frame_pc, and as a
consequence, has a non-stable frame-id.

It doesn't cause many problems because the sigtramp frame is pretty
small, and I guess folk don't usually debug in that frame.

But maybe worth fixing at some point...

Thanks,
Andrew

>        /* The saved %esp will be at cache->base plus 16.  */
>        cache->saved_sp = cache->base + 16;
> @@ -2986,7 +2986,7 @@ amd64_epilogue_frame_this_id (struct frame_info *this_frame,
>    if (!cache->base_p)
>      (*this_id) = frame_id_build_unavailable_stack (cache->pc);
>    else
> -    (*this_id) = frame_id_build (cache->base + 8, cache->pc);
> +    (*this_id) = frame_id_build (cache->saved_sp, cache->pc);
>  }
>  
>  static const struct frame_unwind amd64_epilogue_frame_unwind =
> -- 
> 2.37.2