When a feature XML file provides a "regnum=" tag to indicate
the registers base index, respect it, as it might not be the
same as our current number of registered entries, in particular
when there are gaps.

This fixes a bug with the "power-fpu.xml" file [*] which was
loaded at index 70 while the base register is 71. This latent
bug was exposed by commit 1ec0fbe2dda ("target/ppc: Fix
CPUClass::gdb_num_core_regs value").

[*] https://lore.kernel.org/qemu-devel/e44df309-d40d-46f0-88a8-7ac55f9a3634@fhofhammer.de/

Reported-by: Florian Hofhammer <florian.hofhammer@fhofhammer.de>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 gdbstub/gdbstub.c    | 16 ++++++++++------
 gdbstub/trace-events |  1 +
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index ad4bdc0623d..6976f31933f 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -556,19 +556,19 @@ int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
     return 0;
 }
 
-static void gdb_register_feature(CPUState *cpu, int base_reg,
+static void gdb_register_feature(CPUState *cpu,
                                  gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg,
                                  const GDBFeature *feature)
 {
     GDBRegisterState s = {
-        .base_reg = base_reg,
+        .base_reg = feature->base_reg,
         .get_reg = get_reg,
         .set_reg = set_reg,
         .feature = feature
     };
 
     trace_gdbxml_register_feature(feature->name, feature->xmlname,
-                                  base_reg, feature->num_regs);
+                                  feature->base_reg, feature->num_regs);
     g_array_append_val(cpu->gdb_regs, s);
 }
 
@@ -597,7 +597,8 @@ void gdb_init_cpu(CPUState *cpu)
     if (xmlfile) {
         assert(!cc->gdb_num_core_regs);
         feature = gdb_find_static_feature(xmlfile);
-        gdb_register_feature(cpu, 0,
+        assert(feature->base_reg == 0);
+        gdb_register_feature(cpu,
                              cc->gdb_read_register, cc->gdb_write_register,
                              feature);
         cpu->gdb_num_regs = cpu->gdb_num_g_regs = feature->num_regs;
@@ -617,7 +618,6 @@ void gdb_register_coprocessor(CPUState *cpu,
 {
     GDBRegisterState *s;
     guint i;
-    int base_reg = cpu->gdb_num_regs;
 
     for (i = 0; i < cpu->gdb_regs->len; i++) {
         /* Check for duplicates.  */
@@ -627,7 +627,11 @@ void gdb_register_coprocessor(CPUState *cpu,
         }
     }
 
-    gdb_register_feature(cpu, base_reg, get_reg, set_reg, feature);
+    if (cpu->gdb_num_regs < feature->base_reg) {
+        trace_gdbxml_register_coprocessor_gap(cpu->gdb_num_regs,
+                                              feature->base_reg);
+    }
+    gdb_register_feature(cpu, get_reg, set_reg, feature);
 
     /* Add to end of list.  */
     cpu->gdb_num_regs += feature->num_regs;
diff --git a/gdbstub/trace-events b/gdbstub/trace-events
index 44ef3339934..7036818a387 100644
--- a/gdbstub/trace-events
+++ b/gdbstub/trace-events
@@ -29,6 +29,7 @@ gdbstub_err_checksum_incorrect(uint8_t expected, uint8_t got) "got command packe
 gdbstub_err_unexpected_runpkt(uint8_t ch) "unexpected packet (0x%02x) while target running"
 
 gdbxml_init_cpu(const char *typename, unsigned id, unsigned gdb_num_regs, unsigned gdb_num_g_regs, unsigned gdb_num_core_regs) "%s:%d regs:%u g_regs:%u core_regs:%u"
+gdbxml_register_coprocessor_gap(unsigned gdb_num_regs, unsigned base_reg) "regs %u -> %u"
 gdbxml_register_feature(const char *featname, const char *xmlname, unsigned base_reg, unsigned num_regs) "%s (%s) @%u +%u"
 gdbxml_feature_builder_header(const char *name, const char *xmlname, int num_regs) "%s (%s) regs:%d"
 gdbxml_feature_builder_content(const char *xml) "%s"
-- 
2.53.0

On 10/3/26 16:53, Philippe Mathieu-Daudé wrote:
> When a feature XML file provides a "regnum=" tag to indicate
> the registers base index, respect it, as it might not be the
> same as our current number of registered entries, in particular
> when there are gaps.
> 
> This fixes a bug with the "power-fpu.xml" file [*] which was
> loaded at index 70 while the base register is 71. This latent
> bug was exposed by commit 1ec0fbe2dda ("target/ppc: Fix
> CPUClass::gdb_num_core_regs value").
> 
> [*] https://lore.kernel.org/qemu-devel/e44df309-d40d-46f0-88a8-7ac55f9a3634@fhofhammer.de/
> 
> Reported-by: Florian Hofhammer <florian.hofhammer@fhofhammer.de>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   gdbstub/gdbstub.c    | 16 ++++++++++------
>   gdbstub/trace-events |  1 +
>   2 files changed, 11 insertions(+), 6 deletions(-)


> @@ -617,7 +618,6 @@ void gdb_register_coprocessor(CPUState *cpu,
>   {
>       GDBRegisterState *s;
>       guint i;
> -    int base_reg = cpu->gdb_num_regs;
>   
>       for (i = 0; i < cpu->gdb_regs->len; i++) {
>           /* Check for duplicates.  */
> @@ -627,7 +627,11 @@ void gdb_register_coprocessor(CPUState *cpu,
>           }
>       }
>   
> -    gdb_register_feature(cpu, base_reg, get_reg, set_reg, feature);
> +    if (cpu->gdb_num_regs < feature->base_reg) {
> +        trace_gdbxml_register_coprocessor_gap(cpu->gdb_num_regs,
> +                                              feature->base_reg);
> +    }
> +    gdb_register_feature(cpu, get_reg, set_reg, feature);
Broken patch, please disregard.