Index | Thread | Search

From:
ASOU Masato <takeasou.masato@gmail.com>
Subject:
Re: lldb: Show consistent stack pointer value with GDB
To:
tech@openbsd.org, "robert@openbsd.org" <robert@openbsd.org>
Cc:
Yuichiro NAITO <naito.yuichiro@gmail.com>, Masato ASOU <takeasou.masato@gmail.com>
Date:
Fri, 18 Oct 2024 10:35:43 +0900

Download raw body.

Thread
Hi robert and list.

I was tested this patch on amd64 box and work fine for me.
ok asou@

Can I commit this patch ok?

Before apply patch:
    $ cd /var/crash
    $ doas gdb bsd.0
    GNU gdb 6.3
    Copyright 2004 Free Software Foundation, Inc.
    GDB is free software, covered by the GNU General Public License, and you are
    welcome to change it and/or distribute copies of it under certain
conditions.
    Type "show copying" to see the conditions.
    There is absolutely no warranty for GDB.  Type "show warranty" for details.
    This GDB was configured as "amd64-unknown-openbsd7.6"...(no
debugging symbols found)

    (gdb) target kvm bsd.0.core
    #0  0xffffffff8188fda3 in dumpsys ()
    (gdb) info register

    </snip>

    rbp            0xffff80001cb01ca0       0xffff80001cb01ca0
    rsp            0xffff80001cb01a50       0xffff80001cb01a50

    </snip>

    (gdb) quit
    $ doas lldb bsd.0 -c bsd.0.core
    (lldb) target create "bsd.0" --core "bsd.0.core"
    Core file '/var/crash/bsd.0.core' (x86_64) was loaded.
    (lldb) bt
    * thread #1, name = 'Crashed Thread'
      * frame #0: 0xffffffff8188f51c bsd.0`boot + 268
        frame #1: 0xffffffff8188f51c bsd.0`boot + 268
        frame #2: 0xffffffff81750548 bsd.0`reboot + 104
        frame #3: 0xffffffff817504da bsd.0`sys_reboot + 154
        frame #4: 0xffffffff81597807 bsd.0`syscall + 1511
        frame #5: 0xffffffff82312134 bsd.0`Xsyscall_meltdown + 308
        frame #6: 0xffffffff82312055 bsd.0`Xsyscall_meltdown + 85
        frame #7: 0xffff80001cb01e00
        frame #8: 0x0000083fba322fab
    (lldb) register read
    General Purpose Registers:
           rbp = 0xffff80001cb01ca0
           rsp = 0xffff80001cb01a48
           rip = 0xffffffff8188f51c  bsd.0`boot + 268
    21 registers were unavailable.

    (lldb) quit

After applay patch:
    $ doas lldb bsd.0 -c bsd.0.core
    doas (asou@asou-lldb-kernel.soum.co.jp) password:
    (lldb) target create "bsd.0" --core "bsd.0.core"
    Core file '/var/crash/bsd.0.core' (x86_64) was loaded.
    (lldb) bt
    * thread #1, name = 'Crashed Thread'
      * frame #0: 0xffff80001cb01cd0
        frame #1: 0xffffffff8188f51c bsd.0`boot + 268
        frame #2: 0xffffffff81750548 bsd.0`reboot + 104
        frame #3: 0xffffffff817504da bsd.0`sys_reboot + 154
        frame #4: 0xffffffff81597807 bsd.0`syscall + 1511
        frame #5: 0xffffffff82312134 bsd.0`Xsyscall_meltdown + 308
        frame #6: 0xffffffff82312055 bsd.0`Xsyscall_meltdown + 85
        frame #7: 0xffff80001cb01e00
        frame #8: 0x0000083fba322fab
    (lldb) register read
    General Purpose Registers:
           rbp = 0xffff80001cb01ca0
           rsp = 0xffff80001cb01a50
           rip = 0xffff80001cb01cd0
    21 registers were unavailable.

    (lldb)
--
ASOU Masato

On Fri, Aug 30, 2024 at 6:22 PM Yuichiro NAITO <naito.yuichiro@gmail.com> wrote:
>
> While I see the top of the backtrace frame in the kernel core file,
> the stack register value is differs from what GDB shows.
>
> For example, the kernel core file that is written by ddb, the stack pointer
> of the crashed thread is saved by the 'savectx' function. GDB rollbacks
> the pointer to the position when the beginning of the 'savectx' function is
> called. The lldb in my code won't.
>
> The lldb shows as follows.
>
> ```
> (lldb) register read
> General Purpose Registers:
>        rbp = 0xffff80001c9e2460
>        rsp = 0xffff80001c9e2208
>        rip = 0xffffffff81adbc0c  bsd.0`db_fncall + 1052
> 21 registers were unavailable.
> ```
>
> Gdb shows as follows.
>
> ```
> (gdb) info registers
> rax            0x0      0
> rbx            0x0      0
> rcx            0x0      0
> rdx            0x0      0
> rsi            0x0      0
> rdi            0x0      0
> rbp            0xffff80001c9e2460       0xffff80001c9e2460
> rsp            0xffff80001c9e2210       0xffff80001c9e2210
> r8             0x0      0
> r9             0x0      0
> r10            0x0      0
> r11            0x0      0
> r12            0x0      0
> r13            0x0      0
> r14            0x0      0
> r15            0x0      0
> rip            0xffffffff819f8723       0xffffffff819f8723 <dumpsys+1795>
> eflags         0x0      0
> cs             0x0      0
> ss             0x0      0
> ds             0x0      0
> es             0x0      0
> fs             0x0      0
> gs             0x0      0
> ```
>
> In this case, the lldb `rsp` value is 8 bytes smaller than what of gdb.
> These 8 bytes are the return address from the 'savectx' function.
> And `rip` is also different. It is my fault way to read it from `struct pcb`.
> The 'savectx' function is called from the 'dumpsys' function so the GDB shows
> the right pointer.
>
> In the other case, `struct pcb` is written by the 'cpu_switchto' function and
> the same thing happens. I fixed these cases to be consistent with GDB
> on amd64, i386, and arm64 architectures.
> OK?
>
> diff --git a/gnu/llvm/lldb/source/Plugins/Process/OpenBSDKernel/RegisterContextOpenBSDKernel_arm64.cpp b/gnu/llvm/lldb/source/Plugins/Process/OpenBSDKernel/RegisterContextOpenBSDKernel_arm64.cpp
> index 1ebfc6a799f..34fc8067203 100644
> --- a/gnu/llvm/lldb/source/Plugins/Process/OpenBSDKernel/RegisterContextOpenBSDKernel_arm64.cpp
> +++ b/gnu/llvm/lldb/source/Plugins/Process/OpenBSDKernel/RegisterContextOpenBSDKernel_arm64.cpp
> @@ -91,7 +91,7 @@ bool RegisterContextOpenBSDKernel_arm64::ReadRegister(
>      value = (u_int64_t)sf.sf_x29;
>      return true;
>    case gpr_sp_arm64:
> -    value = (u_int64_t)pcb.pcb_sp;
> +    value = (u_int64_t)(pcb.pcb_sp + sizeof(sf));
>      return true;
>    case gpr_pc_arm64:
>      value = (u_int64_t)sf.sf_lr;
> diff --git a/gnu/llvm/lldb/source/Plugins/Process/OpenBSDKernel/RegisterContextOpenBSDKernel_i386.cpp b/gnu/llvm/lldb/source/Plugins/Process/OpenBSDKernel/RegisterContextOpenBSDKernel_i386.cpp
> index 9a909a4e07d..fcbe6d9000a 100644
> --- a/gnu/llvm/lldb/source/Plugins/Process/OpenBSDKernel/RegisterContextOpenBSDKernel_i386.cpp
> +++ b/gnu/llvm/lldb/source/Plugins/Process/OpenBSDKernel/RegisterContextOpenBSDKernel_i386.cpp
> @@ -62,12 +62,13 @@ bool RegisterContextOpenBSDKernel_i386::ReadRegister(
>    if ((pcb.pcb_flags & PCB_SAVECTX) != 0) {
>      uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
>      switch (reg) {
> -#define PCBREG(x)                                                      \
> -    case lldb_##x##_i386:                                              \
> -      value = pcb.pcb_##x;                                             \
> +    case lldb_esp_i386:
> +      value = (u_int32_t)pcb.pcb_ebp;
> +      return true;
> +    case lldb_ebp_i386:
> +      value = m_thread.GetProcess()->ReadPointerFromMemory(pcb.pcb_ebp,
> +                                                          error);
>        return true;
> -    PCBREG(ebp);
> -    PCBREG(esp);
>      case lldb_eip_i386:
>        value = m_thread.GetProcess()->ReadPointerFromMemory(pcb.pcb_ebp + 4,
>                                                            error);
> @@ -88,17 +89,21 @@ bool RegisterContextOpenBSDKernel_i386::ReadRegister(
>
>    uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
>    switch (reg) {
> +#define PCBREG(x, offset)                                              \
> +    case lldb_##x##_i386:                                              \
> +      value = (u_int32_t)(pcb.pcb_##x + (offset));                     \
> +      return true;
>  #define SFREG(x)                                                       \
>      case lldb_##x##_i386:                                              \
> -      value = sf.sf_##x;                                               \
> +      value = (u_int32_t)sf.sf_##x;                                    \
>        return true;
>
>      SFREG(edi);
>      SFREG(esi);
>      SFREG(ebx);
>      SFREG(eip);
> -    PCBREG(ebp);
> -    PCBREG(esp);
> +    PCBREG(ebp, 0);
> +    PCBREG(esp, sizeof(sf));
>    }
>  #endif
>    return false;
> diff --git a/gnu/llvm/lldb/source/Plugins/Process/OpenBSDKernel/RegisterContextOpenBSDKernel_x86_64.cpp b/gnu/llvm/lldb/source/Plugins/Process/OpenBSDKernel/RegisterContextOpenBSDKernel_x86_64.cpp
> index 501fa858a92..a5eff4f42cf 100644
> --- a/gnu/llvm/lldb/source/Plugins/Process/OpenBSDKernel/RegisterContextOpenBSDKernel_x86_64.cpp
> +++ b/gnu/llvm/lldb/source/Plugins/Process/OpenBSDKernel/RegisterContextOpenBSDKernel_x86_64.cpp
> @@ -77,9 +77,9 @@ bool RegisterContextOpenBSDKernel_x86_64::ReadRegister(
>      case lldb_##x##_x86_64:                    \
>        value = (u_int64_t)sf.sf_##x;            \
>        return true;
> -#define PCBREG(x)                              \
> +#define PCBREG(x, offset)                      \
>      case lldb_##x##_x86_64:                    \
> -      value = pcb.pcb_##x;                     \
> +      value = pcb.pcb_##x + (offset);          \
>        return true;
>      switch (reg) {
>        SFREG(r15);
> @@ -89,14 +89,14 @@ bool RegisterContextOpenBSDKernel_x86_64::ReadRegister(
>        SFREG(rbp);
>        SFREG(rbx);
>        SFREG(rip);
> -      PCBREG(rsp);
> +      PCBREG(rsp, sizeof(sf));
>      }
>    } else {
>      switch (reg) {
> -      PCBREG(rbp);
> -      PCBREG(rsp);
> +      PCBREG(rbp, 0);
> +      PCBREG(rsp, 8);
>      case lldb_rip_x86_64:
> -      value = m_thread.GetProcess()->ReadPointerFromMemory(pcb.pcb_rbp + 8,
> +      value = m_thread.GetProcess()->ReadPointerFromMemory(pcb.pcb_rsp,
>                                                            error);
>        return true;
>      }
>
> --
> Yuichiro NAITO (naito.yuichiro@gmail.com)
>