riscvarchive / riscv-binutils-gdb

RISC-V backports for binutils-gdb. Development is done upstream at the FSF.
GNU General Public License v2.0
147 stars 233 forks source link

GDB displays an "fp" register #152

Closed sebhub closed 6 years ago

sebhub commented 6 years ago

The psABI doesn't mention an "fp" register.

https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md

It is also not understood by the assembler:

mv  sp, sp
mv  fp, fp

This results in: test.s: Assembler messages: test.s:2: Error: illegal operands `mv fp,fp'

GDB prints register s0 as fp:

(gdb) info registers
ra             0x80017f10       0x80017f10 <_Thread_Entry_adaptor_numeric+56>                                                                                                                                                                                                  
sp             0x80053d40       0x80053d40                                                                                                                                                                                                                                     
gp             0x8004a800       0x8004a800 <_Configuration_Scheduler_priority_dflt+1744>
tp             0x800540b8       0x800540b8
t0             0x8000fb60       2147548000
t1             0xf      15
t2             0xffffffffffffffff       18446744073709551615
fp             0x80053d60       0x80053d60
s1             0x0      0
a0             0x8004c2a0       2147795616
a1             0x0      0
a2             0x3      3
a3             0x10     16
a4             0x800003e0       2147484640
a5             0x8004c2a0       2147795616
a6             0x80024e1c       2147634716
a7             0x800540b8       2147827896
s2             0x0      0
s3             0x0      0
s4             0x0      0
s5             0x0      0
s6             0x0      0
s7             0x0      0
s8             0x0      0
s9             0x0      0
s10            0x0      0
s11            0x0      0
t3             0x0      0
t4             0x0      0
t5             0x0      0
t6             0x0      0
pc             0x800003f4       0x800003f4 <Init+20>

Maybe this patch should be applied to GDB:

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 154567136e..4b8149054d 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -128,7 +128,6 @@ static const struct register_alias riscv_register_aliases[] =
   { "t0", 5 },
   { "t1", 6 },
   { "t2", 7 },
-  { "fp", 8 },
   { "s0", 8 },
   { "s1", 9 },
   { "a0", 10 },
jim-wilson commented 6 years ago

​s0 is the frame pointer (fp) register. This is documented in the ISA manual, chapter 20, RISC-V Asssembly Programmer's Handbook. The function FP is important enough that it is reasonable for gdb to prefer that name over s0. The lack of mention in the psABI looks like an oversight. I'd suggest a bug report or patch against that document.

Interesting that the assembler doesn't accept it. Maybe it should.

sebhub commented 6 years ago

I am not sure if the role of s0 as a frame point is of practical relevance. The following code

void g(void);

int f(int i)
{
        g();
        return i;
}

compiles to using -Og or -O2 with GCC

f:
        addi    sp,sp,-16
        sw      ra,12(sp)
        sw      s0,8(sp)
        mv      s0,a0
        call    g
        mv      a0,s0
        lw      ra,12(sp)
        lw      s0,8(sp)
        addi    sp,sp,16
        jr      ra

So, s0 is just the first callee-saved register picked up by GCC. Do you see s0 as a frame pointer in real code? I think the use of the more specific name "fp" instead of the general "s0" is just confusing in GDB.

aswaterman commented 6 years ago

The GCC default is to omit the frame pointer. If you use -fno-omit-frame-pointer you'll see s0 used as such.

jim-wilson commented 6 years ago

Just looking at MIPS toolchain sources for comparison, the assembler supports $30, $s8, and $fp as names for the same register. The compiler supports only $30, and $fp. And gdb apparently only supports s8. So the RISC-V port isn't any more confused than the MIPS port.

I think dropping fp from gdb is wrong, as some people will expect "print $fp" to work, and that should work. But we could perhaps put s0 before fp which presumably makes that the preferred reg for gdb disassembly.

Note that gdb is upstream, and the upstream gdb port is not the same as the one in riscv-binutils-gdb, so adding patches here isn't very useful. Patches added here may be lost when we switch to upstream gdb.

aswaterman commented 6 years ago

@jim-wilson I agree dropping fp seems wrong, but that it would make sense to disassemble it as s0 instead. That would make it consistent with gas/objdump.

sebhub commented 6 years ago

I sent a patch to the GDB patches list:

https://sourceware.org/ml/gdb-patches/2018-06/msg00705.html