| Message ID | 001a113dca8274572005406e03c3@google.com |
|---|---|
| State | New |
| Headers | show |
On 11/03/2016 05:35 PM, Doug Evans wrote: > The remote protocol can't handle flipping back and forth > between 32-bit and 64-bit regs. To compensate, pretend "as if" > on 64-bit cpu when in 32-bit mode. > > Signed-off-by: Doug Evans <dje@google.com> > --- > target-i386/gdbstub.c | 52 ++++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 39 insertions(+), 13 deletions(-) Reviewed-by: Richard Henderson <rth@twiddle.net> r~
On 4 November 2016 at 16:29, Richard Henderson <rth@twiddle.net> wrote: > On 11/03/2016 05:35 PM, Doug Evans wrote: >> >> The remote protocol can't handle flipping back and forth >> between 32-bit and 64-bit regs. To compensate, pretend "as if" >> on 64-bit cpu when in 32-bit mode. >> >> Signed-off-by: Doug Evans <dje@google.com> >> --- >> target-i386/gdbstub.c | 52 >> ++++++++++++++++++++++++++++++++++++++------------- >> 1 file changed, 39 insertions(+), 13 deletions(-) > > > Reviewed-by: Richard Henderson <rth@twiddle.net> Has anybody ever looked at fixing this on the gdb end? (There are some similar cases for ARM CPUs, and actually being able to debug across 32/64 boundaries, big/little endian boundaries etc would be quite handy in some situations.) thanks -- PMM
On 11/04/2016 04:34 PM, Peter Maydell wrote: > On 4 November 2016 at 16:29, Richard Henderson <rth@twiddle.net> wrote: >> On 11/03/2016 05:35 PM, Doug Evans wrote: >>> >>> The remote protocol can't handle flipping back and forth >>> between 32-bit and 64-bit regs. To compensate, pretend "as if" >>> on 64-bit cpu when in 32-bit mode. >>> >>> Signed-off-by: Doug Evans <dje@google.com> >>> --- >>> target-i386/gdbstub.c | 52 >>> ++++++++++++++++++++++++++++++++++++++------------- >>> 1 file changed, 39 insertions(+), 13 deletions(-) >> >> >> Reviewed-by: Richard Henderson <rth@twiddle.net> > > Has anybody ever looked at fixing this on the gdb end? > (There are some similar cases for ARM CPUs, and actually > being able to debug across 32/64 boundaries, big/little > endian boundaries etc would be quite handy in some > situations.) > A while ago I was chatting with Andy Lutomirski about this, and he was saying that at least for x86_64, he'd prefer if gdb always thought of the register file as 64-bit, even if in 16-bit/32-bit mode. http://lists-archives.com/linux-kernel/28605147-x86-signal-rewire-the-restart_block-syscall-to-have-a-constant-nr.html http://lists-archives.com/linux-kernel/28605329-x86-signal-rewire-the-restart_block-syscall-to-have-a-constant-nr.html I never followed up with that, which is totally lame of me. Even if we did that, when debugging 32-bit mode code, I think you'd want gdb to present a 32-bit view of the program, so that printing expressions uses the correct types, calling 32-bit/64-bit functions in the inferior builds a call frame using the right ABI, etc. (and that exposes other challenges like maybe needing to switch modes while the infcall is running if you call a function of the "wrong" mode, etc.) I believe the MIPS 64-bit gdb port actually works that way - it always transfers 64-bit registers across the wire, and then gdb presents 32-bit registers to debugging an elf of a 32-bit ABI. I think it's the only port that does that. Whatever approach is taken, I suspect that there's a good amount of work needed to make things work completely seamlessly, though. Thanks, Pedro Alves
On Fri, Nov 4, 2016 at 12:01 PM, Pedro Alves <palves@redhat.com> wrote: > On 11/04/2016 04:34 PM, Peter Maydell wrote: >> On 4 November 2016 at 16:29, Richard Henderson <rth@twiddle.net> wrote: >>> On 11/03/2016 05:35 PM, Doug Evans wrote: >>>> >>>> The remote protocol can't handle flipping back and forth >>>> between 32-bit and 64-bit regs. To compensate, pretend "as if" >>>> on 64-bit cpu when in 32-bit mode. >>>> >>>> Signed-off-by: Doug Evans <dje@google.com> >>>> --- >>>> target-i386/gdbstub.c | 52 >>>> ++++++++++++++++++++++++++++++++++++++------------- >>>> 1 file changed, 39 insertions(+), 13 deletions(-) >>> >>> >>> Reviewed-by: Richard Henderson <rth@twiddle.net> >> >> Has anybody ever looked at fixing this on the gdb end? >> (There are some similar cases for ARM CPUs, and actually >> being able to debug across 32/64 boundaries, big/little >> endian boundaries etc would be quite handy in some >> situations.) >> > > A while ago I was chatting with Andy Lutomirski about this, > and he was saying that at least for x86_64, he'd prefer if gdb > always thought of the register file as 64-bit, even if > in 16-bit/32-bit mode. > > http://lists-archives.com/linux-kernel/28605147-x86-signal-rewire-the-restart_block-syscall-to-have-a-constant-nr.html > http://lists-archives.com/linux-kernel/28605329-x86-signal-rewire-the-restart_block-syscall-to-have-a-constant-nr.html > > I never followed up with that, which is totally lame of me. > > Even if we did that, when debugging 32-bit mode code, > I think you'd want gdb to present a 32-bit view of the program, > so that printing expressions uses the correct types, > calling 32-bit/64-bit functions in the inferior builds > a call frame using the right ABI, etc. (and that exposes > other challenges like maybe needing to switch modes > while the infcall is running if you call a function of > the "wrong" mode, etc.) I believe the MIPS 64-bit gdb port actually > works that way - it always transfers 64-bit registers across the > wire, and then gdb presents 32-bit registers to debugging an elf > of a 32-bit ABI. I think it's the only port that does that. > Whatever approach is taken, I suspect that there's a good amount > of work needed to make things work completely seamlessly, though. I agree that in 32-bit mode gdb should present a 32-bit view of the world to the user. This needs to work in the absence of debug info, thus there needs to be some protocol extension to provide this information (attaching it to the stop notification is one way, though that'd be insufficient I think). In the general case (i.e., to handle all arches) it'd be insufficient to rely on being able to just use the 64-bit wire format, but until gdb can support arch-switching as needed here, it allows things to work that previously caused things like "'g' packet too long" and totally broke the user's debugging session. I also agree this is a fair bit of work.
On 04/11/2016 00:35, Doug Evans wrote: > The remote protocol can't handle flipping back and forth > between 32-bit and 64-bit regs. To compensate, pretend "as if" > on 64-bit cpu when in 32-bit mode. > > Signed-off-by: Doug Evans <dje@google.com> > --- > target-i386/gdbstub.c | 52 > ++++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 39 insertions(+), 13 deletions(-) > > diff --git a/target-i386/gdbstub.c b/target-i386/gdbstub.c > index c494535..9b94ab8 100644 > --- a/target-i386/gdbstub.c > +++ b/target-i386/gdbstub.c > @@ -44,10 +44,22 @@ int x86_cpu_gdb_read_register(CPUState *cs, uint8_t > *mem_buf, int n) > X86CPU *cpu = X86_CPU(cs); > CPUX86State *env = &cpu->env; > > + /* N.B. GDB can't deal with changes in registers or sizes in the > middle > + of a session. So if we're in 32-bit mode on a 64-bit cpu, still act > + as if we're on a 64-bit cpu. */ > + > if (n < CPU_NB_REGS) { > - if (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK) { > - return gdb_get_reg64(mem_buf, env->regs[gpr_map[n]]); > - } else if (n < CPU_NB_REGS32) { > + if (TARGET_LONG_BITS == 64) { > + if (env->hflags & HF_CS64_MASK) { > + return gdb_get_reg64(mem_buf, env->regs[gpr_map[n]]); > + } else if (n < CPU_NB_REGS32) { > + return gdb_get_reg64(mem_buf, > + env->regs[gpr_map[n]] & > 0xffffffffUL); > + } else { > + memset(mem_buf, 0, sizeof(target_ulong)); > + return sizeof(target_ulong); > + } > + } else { > return gdb_get_reg32(mem_buf, env->regs[gpr_map32[n]]); > } > } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) { > @@ -60,8 +72,7 @@ int x86_cpu_gdb_read_register(CPUState *cs, uint8_t > *mem_buf, int n) > return 10; > } else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) { > n -= IDX_XMM_REGS; > - if (n < CPU_NB_REGS32 || > - (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK)) { > + if (n < CPU_NB_REGS32 || TARGET_LONG_BITS == 64) { > stq_p(mem_buf, env->xmm_regs[n].ZMM_Q(0)); > stq_p(mem_buf + 8, env->xmm_regs[n].ZMM_Q(1)); > return 16; > @@ -69,8 +80,12 @@ int x86_cpu_gdb_read_register(CPUState *cs, uint8_t > *mem_buf, int n) > } else { > switch (n) { > case IDX_IP_REG: > - if (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK) { > - return gdb_get_reg64(mem_buf, env->eip); > + if (TARGET_LONG_BITS == 64) { > + if (env->hflags & HF_CS64_MASK) { > + return gdb_get_reg64(mem_buf, env->eip); > + } else { > + return gdb_get_reg64(mem_buf, env->eip & > 0xffffffffUL); > + } > } else { > return gdb_get_reg32(mem_buf, env->eip); > } > @@ -151,9 +166,17 @@ int x86_cpu_gdb_write_register(CPUState *cs, > uint8_t *mem_buf, int n) > CPUX86State *env = &cpu->env; > uint32_t tmp; > > + /* N.B. GDB can't deal with changes in registers or sizes in the > middle > + of a session. So if we're in 32-bit mode on a 64-bit cpu, still act > + as if we're on a 64-bit cpu. */ > + > if (n < CPU_NB_REGS) { > - if (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK) { > - env->regs[gpr_map[n]] = ldtul_p(mem_buf); > + if (TARGET_LONG_BITS == 64) { > + if (env->hflags & HF_CS64_MASK) { > + env->regs[gpr_map[n]] = ldtul_p(mem_buf); > + } else if (n < CPU_NB_REGS32) { > + env->regs[gpr_map[n]] = ldtul_p(mem_buf) & 0xffffffffUL; > + } > return sizeof(target_ulong); > } else if (n < CPU_NB_REGS32) { > n = gpr_map32[n]; > @@ -169,8 +192,7 @@ int x86_cpu_gdb_write_register(CPUState *cs, uint8_t > *mem_buf, int n) > return 10; > } else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) { > n -= IDX_XMM_REGS; > - if (n < CPU_NB_REGS32 || > - (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK)) { > + if (n < CPU_NB_REGS32 || TARGET_LONG_BITS == 64) { > env->xmm_regs[n].ZMM_Q(0) = ldq_p(mem_buf); > env->xmm_regs[n].ZMM_Q(1) = ldq_p(mem_buf + 8); > return 16; > @@ -178,8 +200,12 @@ int x86_cpu_gdb_write_register(CPUState *cs, > uint8_t *mem_buf, int n) > } else { > switch (n) { > case IDX_IP_REG: > - if (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK) { > - env->eip = ldq_p(mem_buf); > + if (TARGET_LONG_BITS == 64) { > + if (env->hflags & HF_CS64_MASK) { > + env->eip = ldq_p(mem_buf); > + } else { > + env->eip = ldq_p(mem_buf) & 0xffffffffUL; > + } > return 8; > } else { > env->eip &= ~0xffffffffUL; Queued for 2.9, thanks. Paolo
On Wed, Dec 14, 2016 at 9:14 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 04/11/2016 00:35, Doug Evans wrote: >> The remote protocol can't handle flipping back and forth >> between 32-bit and 64-bit regs. To compensate, pretend "as if" >> on 64-bit cpu when in 32-bit mode. >> >> Signed-off-by: Doug Evans <dje@google.com> >> --- >> target-i386/gdbstub.c | 52 >> ++++++++++++++++++++++++++++++++++++++------------- >> 1 file changed, 39 insertions(+), 13 deletions(-) >> ... > > Queued for 2.9, thanks. Awesome! Thanks very much!
diff --git a/target-i386/gdbstub.c b/target-i386/gdbstub.c index c494535..9b94ab8 100644 --- a/target-i386/gdbstub.c +++ b/target-i386/gdbstub.c @@ -44,10 +44,22 @@ int x86_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n) X86CPU *cpu = X86_CPU(cs); CPUX86State *env = &cpu->env; + /* N.B. GDB can't deal with changes in registers or sizes in the middle + of a session. So if we're in 32-bit mode on a 64-bit cpu, still act + as if we're on a 64-bit cpu. */ + if (n < CPU_NB_REGS) { - if (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK) { - return gdb_get_reg64(mem_buf, env->regs[gpr_map[n]]); - } else if (n < CPU_NB_REGS32) { + if (TARGET_LONG_BITS == 64) { + if (env->hflags & HF_CS64_MASK) { + return gdb_get_reg64(mem_buf, env->regs[gpr_map[n]]); + } else if (n < CPU_NB_REGS32) { + return gdb_get_reg64(mem_buf, + env->regs[gpr_map[n]] & 0xffffffffUL); + } else { + memset(mem_buf, 0, sizeof(target_ulong)); + return sizeof(target_ulong); + } + } else { return gdb_get_reg32(mem_buf, env->regs[gpr_map32[n]]); }
The remote protocol can't handle flipping back and forth between 32-bit and 64-bit regs. To compensate, pretend "as if" on 64-bit cpu when in 32-bit mode. Signed-off-by: Doug Evans <dje@google.com> --- target-i386/gdbstub.c | 52 ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 13 deletions(-) } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) { @@ -60,8 +72,7 @@ int x86_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n) return 10; } else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) { n -= IDX_XMM_REGS; - if (n < CPU_NB_REGS32 || - (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK)) { + if (n < CPU_NB_REGS32 || TARGET_LONG_BITS == 64) { stq_p(mem_buf, env->xmm_regs[n].ZMM_Q(0)); stq_p(mem_buf + 8, env->xmm_regs[n].ZMM_Q(1)); return 16; @@ -69,8 +80,12 @@ int x86_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n) } else { switch (n) { case IDX_IP_REG: - if (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK) { - return gdb_get_reg64(mem_buf, env->eip); + if (TARGET_LONG_BITS == 64) { + if (env->hflags & HF_CS64_MASK) { + return gdb_get_reg64(mem_buf, env->eip); + } else { + return gdb_get_reg64(mem_buf, env->eip & 0xffffffffUL); + } } else { return gdb_get_reg32(mem_buf, env->eip); } @@ -151,9 +166,17 @@ int x86_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n) CPUX86State *env = &cpu->env; uint32_t tmp; + /* N.B. GDB can't deal with changes in registers or sizes in the middle + of a session. So if we're in 32-bit mode on a 64-bit cpu, still act + as if we're on a 64-bit cpu. */ + if (n < CPU_NB_REGS) { - if (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK) { - env->regs[gpr_map[n]] = ldtul_p(mem_buf); + if (TARGET_LONG_BITS == 64) { + if (env->hflags & HF_CS64_MASK) { + env->regs[gpr_map[n]] = ldtul_p(mem_buf); + } else if (n < CPU_NB_REGS32) { + env->regs[gpr_map[n]] = ldtul_p(mem_buf) & 0xffffffffUL; + } return sizeof(target_ulong); } else if (n < CPU_NB_REGS32) { n = gpr_map32[n]; @@ -169,8 +192,7 @@ int x86_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n) return 10; } else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) { n -= IDX_XMM_REGS; - if (n < CPU_NB_REGS32 || - (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK)) { + if (n < CPU_NB_REGS32 || TARGET_LONG_BITS == 64) { env->xmm_regs[n].ZMM_Q(0) = ldq_p(mem_buf); env->xmm_regs[n].ZMM_Q(1) = ldq_p(mem_buf + 8); return 16; @@ -178,8 +200,12 @@ int x86_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n) } else { switch (n) { case IDX_IP_REG: - if (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK) { - env->eip = ldq_p(mem_buf); + if (TARGET_LONG_BITS == 64) { + if (env->hflags & HF_CS64_MASK) { + env->eip = ldq_p(mem_buf); + } else { + env->eip = ldq_p(mem_buf) & 0xffffffffUL; + } return 8; } else { env->eip &= ~0xffffffffUL; --