* [PATCH v2 0/2] gdbstub: Unbreak i386 with gdb remote
@ 2020-04-09 18:25 Peter Xu
2020-04-09 18:25 ` [PATCH v2 1/2] gdbstub: Assert len read from each register Peter Xu
2020-04-09 18:25 ` [PATCH v2 2/2] gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb Peter Xu
0 siblings, 2 replies; 4+ messages in thread
From: Peter Xu @ 2020-04-09 18:25 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Bennée, Philippe Mathieu-Daudé, peterx
v2:
- use "goto" in patch 1 [Phil]
- pick some tags
Thanks,
Peter Xu (2):
gdbstub: Assert len read from each register
gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb
gdbstub.c | 12 +++++++++---
target/i386/gdbstub.c | 2 +-
2 files changed, 10 insertions(+), 4 deletions(-)
--
2.24.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/2] gdbstub: Assert len read from each register
2020-04-09 18:25 [PATCH v2 0/2] gdbstub: Unbreak i386 with gdb remote Peter Xu
@ 2020-04-09 18:25 ` Peter Xu
2020-04-09 18:25 ` [PATCH v2 2/2] gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb Peter Xu
1 sibling, 0 replies; 4+ messages in thread
From: Peter Xu @ 2020-04-09 18:25 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Bennée, Philippe Mathieu-Daudé, peterx
This can expose the issue earlier on which register returned the wrong
result.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
gdbstub.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 171e150950..69656e1052 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -911,17 +911,23 @@ static int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
CPUClass *cc = CPU_GET_CLASS(cpu);
CPUArchState *env = cpu->env_ptr;
GDBRegisterState *r;
+ int len = 0, orig_len = buf->len;
if (reg < cc->gdb_num_core_regs) {
- return cc->gdb_read_register(cpu, buf, reg);
+ len = cc->gdb_read_register(cpu, buf, reg);
+ goto out;
}
for (r = cpu->gdb_regs; r; r = r->next) {
if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) {
- return r->get_reg(env, buf, reg - r->base_reg);
+ len = r->get_reg(env, buf, reg - r->base_reg);
+ break;
}
}
- return 0;
+
+out:
+ assert(len == buf->len - orig_len);
+ return len;
}
static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
--
2.24.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb
2020-04-09 18:25 [PATCH v2 0/2] gdbstub: Unbreak i386 with gdb remote Peter Xu
2020-04-09 18:25 ` [PATCH v2 1/2] gdbstub: Assert len read from each register Peter Xu
@ 2020-04-09 18:25 ` Peter Xu
2020-04-09 18:56 ` Peter Xu
1 sibling, 1 reply; 4+ messages in thread
From: Peter Xu @ 2020-04-09 18:25 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Bennée, Philippe Mathieu-Daudé, peterx
We should only pass in gdb_get_reg16() with the GByteArray* object
itself, no need to shift. Without this patch, gdb remote attach will
crash QEMU.
Cc: Alex Bennée <alex.bennee@linaro.org>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Fixes: b7b8756a9c ("target/i386: use gdb_get_reg helpers", 2020-03-17)
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
target/i386/gdbstub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
index f3d23b614e..b98a99500a 100644
--- a/target/i386/gdbstub.c
+++ b/target/i386/gdbstub.c
@@ -106,7 +106,7 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
} else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
floatx80 *fp = (floatx80 *) &env->fpregs[n - IDX_FP_REGS];
int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low));
- len += gdb_get_reg16(mem_buf + len, cpu_to_le16(fp->high));
+ len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high));
return len;
} else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
n -= IDX_XMM_REGS;
--
2.24.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb
2020-04-09 18:25 ` [PATCH v2 2/2] gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb Peter Xu
@ 2020-04-09 18:56 ` Peter Xu
0 siblings, 0 replies; 4+ messages in thread
From: Peter Xu @ 2020-04-09 18:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Bennée, Philippe Mathieu-Daudé
On Thu, Apr 09, 2020 at 02:25:38PM -0400, Peter Xu wrote:
> We should only pass in gdb_get_reg16() with the GByteArray* object
> itself, no need to shift. Without this patch, gdb remote attach will
> crash QEMU.
>
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Fixes: b7b8756a9c ("target/i386: use gdb_get_reg helpers", 2020-03-17)
So I think this should still be:
Fixes: a010bdbe71 ("gdbstub: extend GByteArray to read register helpers")
As Phil & Laurent pointed out.
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> target/i386/gdbstub.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
> index f3d23b614e..b98a99500a 100644
> --- a/target/i386/gdbstub.c
> +++ b/target/i386/gdbstub.c
> @@ -106,7 +106,7 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
> } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
> floatx80 *fp = (floatx80 *) &env->fpregs[n - IDX_FP_REGS];
> int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low));
> - len += gdb_get_reg16(mem_buf + len, cpu_to_le16(fp->high));
> + len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high));
> return len;
> } else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
> n -= IDX_XMM_REGS;
> --
> 2.24.1
>
--
Peter Xu
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-04-09 18:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-09 18:25 [PATCH v2 0/2] gdbstub: Unbreak i386 with gdb remote Peter Xu
2020-04-09 18:25 ` [PATCH v2 1/2] gdbstub: Assert len read from each register Peter Xu
2020-04-09 18:25 ` [PATCH v2 2/2] gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb Peter Xu
2020-04-09 18:56 ` Peter Xu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.