gdbstub: x86-64: reintroduce dynamic register sets

PATCH In review Jan Kiszka 3 participants 5 messages Nov 4, 2008 – Nov 5, 2008

Share & tools

Jan Kiszka

author

2008-11-04 17:23 UTC #0 lore raw

PATCH 1/1 gdbstub: x86-64: reintroduce dynamic register sets

Commit 5459 broke the dynamic register set switching of qemu's gdbstub for x86-64. This prevents setting the correct architecture in gdb when debugging 32 or 16-bit code in a 64-bit emulator. Fix this.

Signed-off-by Jan Kiszka <[email protected]>

---
 gdbstub.c         |   54 +++++++++++++++++++++++++++++++++++++++++++-----------
 target-i386/cpu.h |    7 +++++--
 2 files changed, 48 insertions(+), 13 deletions(-)

gdbstub.c +43 ~~-11~~

Index: b/gdbstub.c
===================================================================
@@ -302,15 +302,22 @@ static const int gpr_map[16] = {
     8, 9, 10, 11, 12, 13, 14, 15
 };
 #else
-static const int gpr_map[8] = {0, 1, 2, 3, 4, 5, 6, 7};
+#define gpr_map gpr_map32
 #endif
+static const int gpr_map32[8] = { 0, 1, 2, 3, 4, 5, 6, 7 };
 
 #define NUM_CORE_REGS (CPU_NB_REGS * 2 + 25)
 
 static int cpu_gdb_read_register(CPUState *env, uint8_t *mem_buf, int n)
 {
     if (n < CPU_NB_REGS) {
-        GET_REGL(env->regs[gpr_map[n]]);
+        if (TARGET_LONG_BITS == 64 && (env->hflags & HF_CS64_MASK)) {
+            GET_REGL(env->regs[gpr_map[n]]);
+        } else if (n < CPU_NB_REGS32) {
+            GET_REG32(env->regs[gpr_map32[n]]);
+        } else {
+            return 0;
+        }
     } else if (n >= CPU_NB_REGS + 8 && n < CPU_NB_REGS + 16) {
         /* FIXME: byteswap float values.  */
 #ifdef USE_X86LDOUBLE

@@ -321,17 +328,25 @@ static int cpu_gdb_read_register(CPUStat
         return 10;
     } else if (n >= CPU_NB_REGS + 24) {
         n -= CPU_NB_REGS + 24;
-        if (n < CPU_NB_REGS) {
+        if (n < CPU_NB_REGS32
+            || (TARGET_LONG_BITS == 64 && (env->hflags & HF_CS64_MASK)
+                && n < CPU_NB_REGS64)) {
             stq_p(mem_buf, env->xmm_regs[n].XMM_Q(0));
             stq_p(mem_buf + 8, env->xmm_regs[n].XMM_Q(1));
             return 16;
-        } else if (n == CPU_NB_REGS) {
+        } else if ((TARGET_LONG_BITS == 64 && (env->hflags & HF_CS64_MASK)
+                    && n == CPU_NB_REGS64) || n == CPU_NB_REGS32) {
             GET_REG32(env->mxcsr);
-        } 
+        }
     } else {
         n -= CPU_NB_REGS;
         switch (n) {
-        case 0: GET_REGL(env->eip);
+        case 0:
+            if (TARGET_LONG_BITS == 64 && (env->hflags & HF_CS64_MASK)) {
+                GET_REG64(env->eip);
+            } else {
+                GET_REG32(env->eip);
+            }
         case 1: GET_REG32(env->eflags);
         case 2: GET_REG32(env->segs[R_CS].selector);
         case 3: GET_REG32(env->segs[R_SS].selector);

@@ -359,8 +374,15 @@ static int cpu_gdb_write_register(CPUSta
     uint32_t tmp;
 
     if (i < CPU_NB_REGS) {
-        env->regs[gpr_map[i]] = ldtul_p(mem_buf);
-        return sizeof(target_ulong);
+        if (TARGET_LONG_BITS == 64 && (env->hflags & HF_CS64_MASK)) {
+            env->regs[gpr_map[i]] = ldtul_p(mem_buf);
+            return sizeof(target_ulong);
+        } else if (i < CPU_NB_REGS32) {
+            env->regs[gpr_map32[i]] = ldl_p(mem_buf);
+            return 4;
+        } else {
+            return 0;
+        }
     } else if (i >= CPU_NB_REGS + 8 && i < CPU_NB_REGS + 16) {
         i -= CPU_NB_REGS + 8;
 #ifdef USE_X86LDOUBLE

@@ -369,18 +391,28 @@ static int cpu_gdb_write_register(CPUSta
         return 10;
     } else if (i >= CPU_NB_REGS + 24) {
         i -= CPU_NB_REGS + 24;
-        if (i < CPU_NB_REGS) {
+        if (i < CPU_NB_REGS32
+            || (TARGET_LONG_BITS == 64 && (env->hflags & HF_CS64_MASK)
+                && i < CPU_NB_REGS64)) {
             env->xmm_regs[i].XMM_Q(0) = ldq_p(mem_buf);
             env->xmm_regs[i].XMM_Q(1) = ldq_p(mem_buf + 8);
             return 16;
-        } else if (i == CPU_NB_REGS) {
+        } else if ((TARGET_LONG_BITS == 64 && (env->hflags & HF_CS64_MASK)
+                    && i == CPU_NB_REGS64) || i == CPU_NB_REGS32) {
             env->mxcsr = ldl_p(mem_buf);
             return 4;
         }
     } else {
         i -= CPU_NB_REGS;
         switch (i) {
-        case 0: env->eip = ldtul_p(mem_buf); return sizeof(target_ulong);
+        case 0:
+            if (TARGET_LONG_BITS == 64 && (env->hflags & HF_CS64_MASK)) {
+                env->eip = ldq_p(mem_buf);
+                return 8;
+            } else {
+                env->eip = ldl_p(mem_buf);
+                return 4;
+            }
         case 1: env->eflags = ldl_p(mem_buf); return 4;
 #if defined(CONFIG_USER_ONLY)
 #define LOAD_SEG(index, sreg)\

target-i386/cpu.h +5 -2

Index: b/target-i386/cpu.h
===================================================================
@@ -494,10 +494,13 @@ typedef union {
 #endif
 #define MMX_Q(n) q
 
+#define CPU_NB_REGS64 16
+#define CPU_NB_REGS32 8
+
 #ifdef TARGET_X86_64
-#define CPU_NB_REGS 16
+#define CPU_NB_REGS CPU_NB_REGS64
 #else
-#define CPU_NB_REGS 8
+#define CPU_NB_REGS CPU_NB_REGS32
 #endif
 
 #define NB_MMU_MODES 2

Signature · 2 lines

Siemens AG, Corporate Technology, CT SE 2 ES-OS
Corporate Competence Center Embedded Linux

Paul Brook

2008-11-04 17:43 UTC #1 lore raw

Quoted 3 lines · Jan Kiszka

Commit 5459 broke the dynamic register set switching of qemu's gdbstub for x86-64. This prevents setting the correct architecture in gdb when debugging 32 or 16-bit code in a 64-bit emulator. Fix this.

Is this really a feature? Surely any attached gdb is going to break horribly when we transition from a 64-bit to a 32-bit code segment.

Paul

Jan Kiszka

author

2008-11-04 17:59 UTC #2 lore raw

Quoted 7 lines · Paul Brook

Quoted 3 lines · Jan Kiszka
Commit 5459 broke the dynamic register set switching of qemu's gdbstub for x86-64. This prevents setting the correct architecture in gdb when debugging 32 or 16-bit code in a 64-bit emulator. Fix this.
Is this really a feature? Surely any attached gdb is going to break horribly when we transition from a 64-bit to a 32-bit code segment.

Well, it would be real feature if gdb was smart enough to track those switches automatically...

However, you can (and obviously have to) call "set arch ..." after that switch in order to get the proper disassembly. Or you happen to use qemu-system-x86_64 with a 32-bit guest and fire up gdb with the appropriate 32-bit binary directly. Both used to work fine.

But if there is a way to tell gdb to switch x86 archs without switching the remote gdb packet length for register queries, I would happily use that and drop my patch!

Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT SE 2 ES-OS
Corporate Competence Center Embedded Linux

Jamie Lokier

2008-11-05 12:43 UTC #3 lore raw

Quoted 9 lines · Jan Kiszka

Quoted 7 lines · Paul Brook
Quoted 3 lines · Jan Kiszka
Commit 5459 broke the dynamic register set switching of qemu's gdbstub for x86-64. This prevents setting the correct architecture in gdb when debugging 32 or 16-bit code in a 64-bit emulator. Fix this.
Is this really a feature? Surely any attached gdb is going to break horribly when we transition from a 64-bit to a 32-bit code segment.

Quoted 7 lines

Well, it would be real feature if gdb was smart enough to track those switches automatically...
However, you can (and obviously have to) call "set arch ..." after that switch in order to get the proper disassembly. Or you happen to use qemu-system-x86_64 with a 32-bit guest and fire up gdb with the appropriate 32-bit binary directly. Both used to work fine.

How does it handle mixed 32-bit and 16-bit code? (Since you mentioned it supports 16-bit code).

Does that require manual intervention too?

-- Jamie

Jan Kiszka

author

2008-11-05 14:44 UTC #4 lore raw

Quoted 16 lines · Jamie Lokier

Quoted 7 lines · Jan Kiszka
Quoted 6 lines · Paul Brook
Quoted 3 lines · Jan Kiszka
Commit 5459 broke the dynamic register set switching of qemu's gdbstub for x86-64. This prevents setting the correct architecture in gdb when debugging 32 or 16-bit code in a 64-bit emulator. Fix this.
Is this really a feature? Surely any attached gdb is going to break horribly when we transition from a 64-bit to a 32-bit code segment.
Quoted 7 lines
Well, it would be real feature if gdb was smart enough to track those switches automatically...
However, you can (and obviously have to) call "set arch ..." after that switch in order to get the proper disassembly. Or you happen to use qemu-system-x86_64 with a 32-bit guest and fire up gdb with the appropriate 32-bit binary directly. Both used to work fine.

Quoted 4 lines

How does it handle mixed 32-bit and 16-bit code? (Since you mentioned it supports 16-bit code).
Does that require manual intervention too?

Same here: 'set arch i8086' will make gdb switch to 16-bit disassembly mode. But the remote protocol's register set format does not change then, compared to arch i386.

BTW, more glue (including gdb patches) is required for full support of x86 segmentation. I'm currently trying to get out changes in shape again that were once based on older versions of qemu and gdb.

Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT SE 2 ES-OS
Corporate Competence Center Embedded Linux