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>
Date:
Thu, 31 Oct 2024 08:13:03 +0900

Download raw body.

Thread
There are no objections, so I will commit later.
--
ASOU Masato

On Fri, Oct 18, 2024 at 10:35 AM ASOU Masato <takeasou.masato@gmail.com> wrote:
>
> 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)
> >