From 5d1ab242067cdff30a8fc875933b14e21bb53308 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Mon, 9 Oct 2023 17:40:48 +0100 Subject: gdbstub: Fix target.xml response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was failing to return target.xml after the first request. Fixes: 56e534bd11 ("gdbstub: refactor get_feature_xml") Signed-off-by: Akihiko Odaki Message-Id: <20230912224107.29669-3-akihiko.odaki@daynix.com> Signed-off-by: Alex Bennée Message-Id: <20231009164104.369749-10-alex.bennee@linaro.org> --- gdbstub/gdbstub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'gdbstub/gdbstub.c') diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index 8eea214..4f3762f 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -396,8 +396,8 @@ static const char *get_feature_xml(const char *p, const char **newp, g_string_append(xml, ""); process->target_xml = g_string_free(xml, false); - return process->target_xml; } + return process->target_xml; } /* Is it dynamically generated by the target? */ if (cc->gdb_get_dynamic_xml) { -- cgit v1.1 From 956af7daad5a97ca20f8e24d0f4d91a8fca650ad Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Mon, 9 Oct 2023 17:40:51 +0100 Subject: gdbstub: Introduce GDBFeature structure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this change, the information from a XML file was stored in an array that is not descriptive. Introduce a dedicated structure type to make it easier to understand and to extend with more fields. Signed-off-by: Akihiko Odaki Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20230912224107.29669-6-akihiko.odaki@daynix.com> Signed-off-by: Alex Bennée Message-Id: <20231009164104.369749-13-alex.bennee@linaro.org> --- gdbstub/gdbstub.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'gdbstub/gdbstub.c') diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index 4f3762f..bba2640 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -408,11 +408,11 @@ static const char *get_feature_xml(const char *p, const char **newp, } } /* Is it one of the encoded gdb-xml/ files? */ - for (int i = 0; xml_builtin[i][0]; i++) { - const char *name = xml_builtin[i][0]; + for (int i = 0; gdb_static_features[i].xmlname; i++) { + const char *name = gdb_static_features[i].xmlname; if ((strncmp(name, p, len) == 0) && strlen(name) == len) { - return xml_builtin[i][1]; + return gdb_static_features[i].xml; } } -- cgit v1.1 From a650683871ba728ebce3d320941f48bac91f0f6a Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Mon, 9 Oct 2023 17:40:53 +0100 Subject: hw/core/cpu: Return static value with gdb_arch_name() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All implementations of gdb_arch_name() returns dynamic duplicates of static strings. It's also unlikely that there will be an implementation of gdb_arch_name() that returns a truly dynamic value due to the nature of the function returning a well-known identifiers. Qualify the value gdb_arch_name() with const and make all of its implementations return static strings. Signed-off-by: Akihiko Odaki Reviewed-by: Alex Bennée Reviewed-by: Alistair Francis Message-Id: <20230912224107.29669-8-akihiko.odaki@daynix.com> Signed-off-by: Alex Bennée Message-Id: <20231009164104.369749-15-alex.bennee@linaro.org> --- gdbstub/gdbstub.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'gdbstub/gdbstub.c') diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index bba2640..a20169c 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -380,10 +380,9 @@ static const char *get_feature_xml(const char *p, const char **newp, ""); if (cc->gdb_arch_name) { - g_autofree gchar *arch = cc->gdb_arch_name(cpu); g_string_append_printf(xml, "%s", - arch); + cc->gdb_arch_name(cpu)); } g_string_append(xml, "gdb_core_xml_file); -- cgit v1.1 From 6d8f77a6a152c1c64c5ee79e31fe3510b020cd0e Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Mon, 9 Oct 2023 17:40:54 +0100 Subject: gdbstub: Use g_markup_printf_escaped() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit g_markup_printf_escaped() is a safer alternative to simple printf() as it automatically escapes values. Signed-off-by: Akihiko Odaki Message-Id: <20230912224107.29669-9-akihiko.odaki@daynix.com> Signed-off-by: Alex Bennée Message-Id: <20231009164104.369749-16-alex.bennee@linaro.org> --- gdbstub/gdbstub.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) (limited to 'gdbstub/gdbstub.c') diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index a20169c..3dc847f 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -373,28 +373,34 @@ static const char *get_feature_xml(const char *p, const char **newp, if (strncmp(p, "target.xml", len) == 0) { if (!process->target_xml) { GDBRegisterState *r; - GString *xml = g_string_new(""); + g_autoptr(GPtrArray) xml = g_ptr_array_new_with_free_func(g_free); - g_string_append(xml, - "" - ""); + g_ptr_array_add( + xml, + g_strdup("" + "" + "")); if (cc->gdb_arch_name) { - g_string_append_printf(xml, - "%s", - cc->gdb_arch_name(cpu)); + g_ptr_array_add( + xml, + g_markup_printf_escaped("%s", + cc->gdb_arch_name(cpu))); } - g_string_append(xml, "gdb_core_xml_file); - g_string_append(xml, "\"/>"); + g_ptr_array_add( + xml, + g_markup_printf_escaped("", + cc->gdb_core_xml_file)); for (r = cpu->gdb_regs; r; r = r->next) { - g_string_append(xml, "xml); - g_string_append(xml, "\"/>"); + g_ptr_array_add( + xml, + g_markup_printf_escaped("", + r->xml)); } - g_string_append(xml, ""); + g_ptr_array_add(xml, g_strdup("")); + g_ptr_array_add(xml, NULL); - process->target_xml = g_string_free(xml, false); + process->target_xml = g_strjoinv(NULL, (void *)xml->pdata); } return process->target_xml; } -- cgit v1.1 From 213316d401e72b218d8edd9b88d72df6ecf0cc49 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Mon, 9 Oct 2023 17:40:57 +0100 Subject: gdbstub: Remove gdb_has_xml variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GDB has XML support since 6.7 which was released in 2007. It's time to remove support for old GDB versions without XML support. Signed-off-by: Akihiko Odaki Reviewed-by: Alistair Francis Message-Id: <20230912224107.29669-12-akihiko.odaki@daynix.com> Signed-off-by: Alex Bennée Message-Id: <20231009164104.369749-19-alex.bennee@linaro.org> --- gdbstub/gdbstub.c | 15 --------------- 1 file changed, 15 deletions(-) (limited to 'gdbstub/gdbstub.c') diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index 3dc847f..62608a5 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -349,11 +349,6 @@ static CPUState *gdb_get_cpu(uint32_t pid, uint32_t tid) } } -bool gdb_has_xml(void) -{ - return !!gdb_get_cpu_process(gdbserver_state.g_cpu)->target_xml; -} - static const char *get_feature_xml(const char *p, const char **newp, GDBProcess *process) { @@ -1086,11 +1081,6 @@ static void handle_set_reg(GArray *params, void *user_ctx) { int reg_size; - if (!gdb_get_cpu_process(gdbserver_state.g_cpu)->target_xml) { - gdb_put_packet(""); - return; - } - if (params->len != 2) { gdb_put_packet("E22"); return; @@ -1107,11 +1097,6 @@ static void handle_get_reg(GArray *params, void *user_ctx) { int reg_size; - if (!gdb_get_cpu_process(gdbserver_state.g_cpu)->target_xml) { - gdb_put_packet(""); - return; - } - if (!params->len) { gdb_put_packet("E14"); return; -- cgit v1.1 From 73c392c26b0c96eb07272ea21cc0062ce534b6a3 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Mon, 9 Oct 2023 17:40:58 +0100 Subject: gdbstub: Replace gdb_regs with an array MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An array is a more appropriate data structure than a list for gdb_regs since it is initialized only with append operation and read-only after initialization. Signed-off-by: Akihiko Odaki Reviewed-by: Alistair Francis Message-Id: <20230912224107.29669-13-akihiko.odaki@daynix.com> [AJB: fixed a checkpatch violation] Signed-off-by: Alex Bennée Message-Id: <20231009164104.369749-20-alex.bennee@linaro.org> --- gdbstub/gdbstub.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) (limited to 'gdbstub/gdbstub.c') diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index 62608a5..b153211 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -51,7 +51,6 @@ typedef struct GDBRegisterState { gdb_get_reg_cb get_reg; gdb_set_reg_cb set_reg; const char *xml; - struct GDBRegisterState *next; } GDBRegisterState; GDBState gdbserver_state; @@ -386,7 +385,8 @@ static const char *get_feature_xml(const char *p, const char **newp, xml, g_markup_printf_escaped("", cc->gdb_core_xml_file)); - for (r = cpu->gdb_regs; r; r = r->next) { + for (guint i = 0; i < cpu->gdb_regs->len; i++) { + r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); g_ptr_array_add( xml, g_markup_printf_escaped("", @@ -430,7 +430,8 @@ static int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg) return cc->gdb_read_register(cpu, buf, reg); } - for (r = cpu->gdb_regs; r; r = r->next) { + for (guint i = 0; i < cpu->gdb_regs->len; i++) { + r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) { return r->get_reg(env, buf, reg - r->base_reg); } @@ -448,7 +449,8 @@ static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg) return cc->gdb_write_register(cpu, mem_buf, reg); } - for (r = cpu->gdb_regs; r; r = r->next) { + for (guint i = 0; i < cpu->gdb_regs->len; i++) { + r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) { return r->set_reg(env, mem_buf, reg - r->base_reg); } @@ -461,17 +463,23 @@ void gdb_register_coprocessor(CPUState *cpu, int num_regs, const char *xml, int g_pos) { GDBRegisterState *s; - GDBRegisterState **p; - - p = &cpu->gdb_regs; - while (*p) { - /* Check for duplicates. */ - if (strcmp((*p)->xml, xml) == 0) - return; - p = &(*p)->next; + guint i; + + if (cpu->gdb_regs) { + for (i = 0; i < cpu->gdb_regs->len; i++) { + /* Check for duplicates. */ + s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); + if (strcmp(s->xml, xml) == 0) { + return; + } + } + } else { + cpu->gdb_regs = g_array_new(false, false, sizeof(GDBRegisterState)); + i = 0; } - s = g_new0(GDBRegisterState, 1); + g_array_set_size(cpu->gdb_regs, i + 1); + s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); s->base_reg = cpu->gdb_num_regs; s->num_regs = num_regs; s->get_reg = get_reg; @@ -480,7 +488,6 @@ void gdb_register_coprocessor(CPUState *cpu, /* Add to end of list. */ cpu->gdb_num_regs += num_regs; - *p = s; if (g_pos) { if (g_pos != s->base_reg) { error_report("Error: Bad gdb register numbering for '%s', " -- cgit v1.1