Re: [PATCH 3/4] [gdb/tdep, aarch64] Fix frame address of last insn in leaf function

[Luis Machado <luis.machado@arm.com>]

On 1/19/23 10:46, Tom de Vries wrote:
> Consider the test-case test.c, compiled without debug info:
> ...
> void
> foo (const char *s)
> {
> }
> 
> int
> main (void)
> {
>    foo ("foo");
>    return 0;
> }
> ...
> 
> Disassembly of foo:
> ...
> 0000000000400564 <foo>:
>    400564:       d10043ff        sub     sp, sp, #0x10
>    400568:       f90007e0        str     x0, [sp, #8]
>    40056c:       d503201f        nop
>    400570:       910043ff        add     sp, sp, #0x10
>    400574:       d65f03c0        ret
> ...
> 
> Now, let's do "info frame" at each insn in foo, as well as printing $sp
> and $x29 (and strip the output of info frame to the first line, for brevity):
> ...
> $ gdb -q a.out
> Reading symbols from a.out...
> (gdb) b *foo
> Breakpoint 1 at 0x400564
> (gdb) r
> Starting program: a.out
> 
> Breakpoint 1, 0x0000000000400564 in foo ()
> (gdb) display /x $sp
> 1: /x $sp = 0xfffffffff3a0
> (gdb) display /x $x29
> 2: /x $x29 = 0xfffffffff3a0
> (gdb) info frame
> Stack level 0, frame at 0xfffffffff3a0:
> (gdb) si
> 0x0000000000400568 in foo ()
> 1: /x $sp = 0xfffffffff390
> 2: /x $x29 = 0xfffffffff3a0
> (gdb) info frame
> Stack level 0, frame at 0xfffffffff3a0:
> (gdb) si
> 0x000000000040056c in foo ()
> 1: /x $sp = 0xfffffffff390
> 2: /x $x29 = 0xfffffffff3a0
> (gdb) info frame
> Stack level 0, frame at 0xfffffffff3a0:
> (gdb) si
> 0x0000000000400570 in foo ()
> 1: /x $sp = 0xfffffffff390
> 2: /x $x29 = 0xfffffffff3a0
> (gdb) info frame
> Stack level 0, frame at 0xfffffffff3a0:
> (gdb) si
> 0x0000000000400574 in foo ()
> 1: /x $sp = 0xfffffffff3a0
> 2: /x $x29 = 0xfffffffff3a0
> (gdb) info frame
> Stack level 0, frame at 0xfffffffff3b0:
>   pc = 0x400574 in foo; saved pc = 0x40058c
> (gdb) si
> 0x000000000040058c in main ()
> 1: /x $sp = 0xfffffffff3a0
> 2: /x $x29 = 0xfffffffff3a0
> ...
> 
> The "frame at" bit lists 0xfffffffff3a0 except at the last insn, where it
> lists 0xfffffffff3b0.
> 
> The frame address is calculated here in aarch64_make_prologue_cache_1:
> ...
>    unwound_fp = get_frame_register_unsigned (this_frame, cache->framereg);
>    if (unwound_fp == 0)
>      return;
> 
>    cache->prev_sp = unwound_fp + cache->framesize;
> ...
> 
> For insns after the prologue, we have cache->framereg == sp and
> cache->framesize == 16, so unwound_fp + cache->framesize gives the wrong
> answer once sp has been restored to entry value by the before-last insn.
> 
> Fix this by detecting the situation that the sp has been restored.
> 
> This fixes PR tdep/30011.
> 
> This also fixes the aarch64 FAILs in gdb.reverse/solib-precsave.exp and
> gdb.reverse/solib-reverse.exp I reported in PR gdb/PR29721.

I still see failures for gdb.reverse/solib-precsave.exp and gdb.reverse/solib-reverse.exp for both Ubuntu 22.04 and 20.04
on aarch64-linux.

Running /work/luimac01/work/builds/binutils-gdb-arm64-jammy/gdb/testsuite/../../../../repos/binutils-gdb/gdb/testsuite/gdb.reverse/solib-prec
save.exp ...
FAIL: gdb.reverse/solib-precsave.exp: reverse-step into solib function one
FAIL: gdb.reverse/solib-precsave.exp: reverse-step within solib function one
FAIL: gdb.reverse/solib-precsave.exp: reverse-step back to main one
FAIL: gdb.reverse/solib-precsave.exp: reverse-step into solib function two
FAIL: gdb.reverse/solib-precsave.exp: reverse-step within solib function two

Running /work/luimac01/work/builds/binutils-gdb-arm64-jammy/gdb/testsuite/../../../../repos/binutils-gdb/gdb/testsuite/gdb.reverse/solib-reve
rse.exp ...
FAIL: gdb.reverse/solib-reverse.exp: reverse-step into solib function one
FAIL: gdb.reverse/solib-reverse.exp: reverse-step within solib function one
FAIL: gdb.reverse/solib-reverse.exp: reverse-step back to main one
FAIL: gdb.reverse/solib-reverse.exp: reverse-step into solib function two
FAIL: gdb.reverse/solib-reverse.exp: reverse-step within solib function two

Maybe it addresses a different issue, but what I'm seeing is possibly something else (the linetable issue? I vaguely recall the situation for that).
> 
> Tested on aarch64-linux.
> PR tdep/30011
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30011
> ---
>   gdb/aarch64-tdep.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index b576d3b9d99..06349353716 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -996,7 +996,11 @@ aarch64_make_prologue_cache_1 (frame_info_ptr this_frame,
>     if (unwound_fp == 0)
>       return;
>   
> -  cache->prev_sp = unwound_fp + cache->framesize;
> +  if (cache->framereg == AARCH64_SP_REGNUM
> +      && get_frame_register_unsigned (this_frame, AARCH64_FP_REGNUM) == unwound_fp)
> +    cache->prev_sp = unwound_fp;
> +  else
> +    cache->prev_sp = unwound_fp + cache->framesize;
>   
>     /* Calculate actual addresses of saved registers using offsets
>        determined by aarch64_analyze_prologue.  */