* [PATCH v3 01/12] gdbstub: Fix target_xml initialization
2023-09-12 22:40 [PATCH v3 00/12] gdbstub and TCG plugin improvements Akihiko Odaki
@ 2023-09-12 22:40 ` Akihiko Odaki
2023-09-14 12:29 ` Philippe Mathieu-Daudé
2023-09-12 22:40 ` [PATCH v3 02/12] gdbstub: Fix target.xml response Akihiko Odaki
` (11 subsequent siblings)
12 siblings, 1 reply; 18+ messages in thread
From: Akihiko Odaki @ 2023-09-12 22:40 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki
target_xml is no longer a fixed-length array but a pointer to a
variable-length memory.
Fixes: 56e534bd11 ("gdbstub: refactor get_feature_xml")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
gdbstub/softmmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdbstub/softmmu.c b/gdbstub/softmmu.c
index 9f0b8b5497..42645d2220 100644
--- a/gdbstub/softmmu.c
+++ b/gdbstub/softmmu.c
@@ -292,7 +292,7 @@ static int find_cpu_clusters(Object *child, void *opaque)
assert(cluster->cluster_id != UINT32_MAX);
process->pid = cluster->cluster_id + 1;
process->attached = false;
- process->target_xml[0] = '\0';
+ process->target_xml = NULL;
return 0;
}
--
2.42.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 01/12] gdbstub: Fix target_xml initialization
2023-09-12 22:40 ` [PATCH v3 01/12] gdbstub: Fix target_xml initialization Akihiko Odaki
@ 2023-09-14 12:29 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-09-14 12:29 UTC (permalink / raw)
To: Alex Bennée, Akihiko Odaki, Nikita Shubin, Nikita Shubin
Cc: Mikhail Tyutin, Aleksandr Anenkov, qemu-devel
On 13/9/23 00:40, Akihiko Odaki wrote:
> target_xml is no longer a fixed-length array but a pointer to a
> variable-length memory.
>
> Fixes: 56e534bd11 ("gdbstub: refactor get_feature_xml")
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> gdbstub/softmmu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdbstub/softmmu.c b/gdbstub/softmmu.c
> index 9f0b8b5497..42645d2220 100644
> --- a/gdbstub/softmmu.c
> +++ b/gdbstub/softmmu.c
> @@ -292,7 +292,7 @@ static int find_cpu_clusters(Object *child, void *opaque)
> assert(cluster->cluster_id != UINT32_MAX);
> process->pid = cluster->cluster_id + 1;
> process->attached = false;
> - process->target_xml[0] = '\0';
> + process->target_xml = NULL;
>
> return 0;
> }
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 02/12] gdbstub: Fix target.xml response
2023-09-12 22:40 [PATCH v3 00/12] gdbstub and TCG plugin improvements Akihiko Odaki
2023-09-12 22:40 ` [PATCH v3 01/12] gdbstub: Fix target_xml initialization Akihiko Odaki
@ 2023-09-12 22:40 ` Akihiko Odaki
2023-09-12 22:40 ` [PATCH v3 03/12] plugins: Check if vCPU is realized Akihiko Odaki
` (10 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2023-09-12 22:40 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki
It was failing to return target.xml after the first request.
Fixes: 56e534bd11 ("gdbstub: refactor get_feature_xml")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
gdbstub/gdbstub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 349d348c7b..384191bcb0 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, "</target>");
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) {
--
2.42.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 03/12] plugins: Check if vCPU is realized
2023-09-12 22:40 [PATCH v3 00/12] gdbstub and TCG plugin improvements Akihiko Odaki
2023-09-12 22:40 ` [PATCH v3 01/12] gdbstub: Fix target_xml initialization Akihiko Odaki
2023-09-12 22:40 ` [PATCH v3 02/12] gdbstub: Fix target.xml response Akihiko Odaki
@ 2023-09-12 22:40 ` Akihiko Odaki
2023-09-13 6:17 ` Philippe Mathieu-Daudé
2023-09-12 22:40 ` [PATCH v3 04/12] contrib/plugins: Use GRWLock in execlog Akihiko Odaki
` (9 subsequent siblings)
12 siblings, 1 reply; 18+ messages in thread
From: Akihiko Odaki @ 2023-09-12 22:40 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Alexandre Iooss,
Mahmoud Mandour
The created member of CPUState tells if the vCPU thread is started, and
will be always false for the user space emulation that manages threads
independently. Use the realized member of DeviceState, which is valid
for both of the system and user space emulation.
Fixes: 54cb65d858 ("plugin: add core code")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
plugins/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/plugins/core.c b/plugins/core.c
index 3c4e26c7ed..fcd33a2bff 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -64,7 +64,7 @@ static void plugin_cpu_update__locked(gpointer k, gpointer v, gpointer udata)
CPUState *cpu = container_of(k, CPUState, cpu_index);
run_on_cpu_data mask = RUN_ON_CPU_HOST_ULONG(*plugin.mask);
- if (cpu->created) {
+ if (DEVICE(cpu)->realized) {
async_run_on_cpu(cpu, plugin_cpu_update__async, mask);
} else {
plugin_cpu_update__async(cpu, mask);
--
2.42.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 03/12] plugins: Check if vCPU is realized
2023-09-12 22:40 ` [PATCH v3 03/12] plugins: Check if vCPU is realized Akihiko Odaki
@ 2023-09-13 6:17 ` Philippe Mathieu-Daudé
2023-09-14 17:16 ` Akihiko Odaki
0 siblings, 1 reply; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-09-13 6:17 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Alexandre Iooss, Mahmoud Mandour, Richard Henderson
On 13/9/23 00:40, Akihiko Odaki wrote:
> The created member of CPUState tells if the vCPU thread is started, and
> will be always false for the user space emulation that manages threads
> independently.
Per the docstring:
/**
* CPUState:
* @created: Indicates whether the CPU thread has been
* successfully created.
Each CPU DeviceClass's DeviceRealize() handler which calls
qemu_init_vcpu(). Ah, what we miss is:
-- >8 --
--- a/accel/tcg/user-exec-stub.c
+++ b/accel/tcg/user-exec-stub.c
@@ -14,6 +14,7 @@ void cpu_remove_sync(CPUState *cpu)
void qemu_init_vcpu(CPUState *cpu)
{
+ cpu->created = true;
}
---
Missed in commit c7f0f3b1c8 ("qtest: add test framework").
Does that help?
> Use the realized member of DeviceState, which is valid
> for both of the system and user space emulation.
>
> Fixes: 54cb65d858 ("plugin: add core code")
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> plugins/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/plugins/core.c b/plugins/core.c
> index 3c4e26c7ed..fcd33a2bff 100644
> --- a/plugins/core.c
> +++ b/plugins/core.c
> @@ -64,7 +64,7 @@ static void plugin_cpu_update__locked(gpointer k, gpointer v, gpointer udata)
> CPUState *cpu = container_of(k, CPUState, cpu_index);
> run_on_cpu_data mask = RUN_ON_CPU_HOST_ULONG(*plugin.mask);
>
> - if (cpu->created) {
> + if (DEVICE(cpu)->realized) {
> async_run_on_cpu(cpu, plugin_cpu_update__async, mask);
> } else {
> plugin_cpu_update__async(cpu, mask);
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v3 03/12] plugins: Check if vCPU is realized
2023-09-13 6:17 ` Philippe Mathieu-Daudé
@ 2023-09-14 17:16 ` Akihiko Odaki
0 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2023-09-14 17:16 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Alexandre Iooss, Mahmoud Mandour, Richard Henderson
On 2023/09/13 15:17, Philippe Mathieu-Daudé wrote:
> On 13/9/23 00:40, Akihiko Odaki wrote:
>> The created member of CPUState tells if the vCPU thread is started, and
>> will be always false for the user space emulation that manages threads
>> independently.
>
> Per the docstring:
>
> /**
> * CPUState:
>
> * @created: Indicates whether the CPU thread has been
> * successfully created.
>
> Each CPU DeviceClass's DeviceRealize() handler which calls
> qemu_init_vcpu(). Ah, what we miss is:
>
> -- >8 --
> --- a/accel/tcg/user-exec-stub.c
> +++ b/accel/tcg/user-exec-stub.c
> @@ -14,6 +14,7 @@ void cpu_remove_sync(CPUState *cpu)
>
> void qemu_init_vcpu(CPUState *cpu)
> {
> + cpu->created = true;
> }
> ---
>
> Missed in commit c7f0f3b1c8 ("qtest: add test framework").
I think the member is never set for user space emulation since it was
introduced with commit d6dc3d424e ("qemu: introduce iothread (Marcelo
Tosatti)")
>
> Does that help?
That will work, but I don't think it makes sense to set this member
while the other members related to vCPU thread like the "thread" member
are unset.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 04/12] contrib/plugins: Use GRWLock in execlog
2023-09-12 22:40 [PATCH v3 00/12] gdbstub and TCG plugin improvements Akihiko Odaki
` (2 preceding siblings ...)
2023-09-12 22:40 ` [PATCH v3 03/12] plugins: Check if vCPU is realized Akihiko Odaki
@ 2023-09-12 22:40 ` Akihiko Odaki
2023-09-12 22:40 ` [PATCH v3 05/12] gdbstub: Introduce GDBFeature structure Akihiko Odaki
` (8 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2023-09-12 22:40 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Alexandre Iooss,
Mahmoud Mandour
execlog had the following comment:
> As we could have multiple threads trying to do this we need to
> serialise the expansion under a lock. Threads accessing already
> created entries can continue without issue even if the ptr array
> gets reallocated during resize.
However, when the ptr array gets reallocated, the other threads may have
a stale reference to the old buffer. This results in use-after-free.
Use GRWLock to properly fix this issue.
Fixes: 3d7caf145e ("contrib/plugins: add execlog to log instruction execution and memory access")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
contrib/plugins/execlog.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index 7129d526f8..82dc2f584e 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -19,7 +19,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
/* Store last executed instruction on each vCPU as a GString */
static GPtrArray *last_exec;
-static GMutex expand_array_lock;
+static GRWLock expand_array_lock;
static GPtrArray *imatches;
static GArray *amatches;
@@ -28,18 +28,16 @@ static GArray *amatches;
* Expand last_exec array.
*
* As we could have multiple threads trying to do this we need to
- * serialise the expansion under a lock. Threads accessing already
- * created entries can continue without issue even if the ptr array
- * gets reallocated during resize.
+ * serialise the expansion under a lock.
*/
static void expand_last_exec(int cpu_index)
{
- g_mutex_lock(&expand_array_lock);
+ g_rw_lock_writer_lock(&expand_array_lock);
while (cpu_index >= last_exec->len) {
GString *s = g_string_new(NULL);
g_ptr_array_add(last_exec, s);
}
- g_mutex_unlock(&expand_array_lock);
+ g_rw_lock_writer_unlock(&expand_array_lock);
}
/**
@@ -51,8 +49,10 @@ static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t info,
GString *s;
/* Find vCPU in array */
+ g_rw_lock_reader_lock(&expand_array_lock);
g_assert(cpu_index < last_exec->len);
s = g_ptr_array_index(last_exec, cpu_index);
+ g_rw_lock_reader_unlock(&expand_array_lock);
/* Indicate type of memory access */
if (qemu_plugin_mem_is_store(info)) {
@@ -80,10 +80,14 @@ static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
GString *s;
/* Find or create vCPU in array */
+ g_rw_lock_reader_lock(&expand_array_lock);
if (cpu_index >= last_exec->len) {
+ g_rw_lock_reader_unlock(&expand_array_lock);
expand_last_exec(cpu_index);
+ g_rw_lock_reader_lock(&expand_array_lock);
}
s = g_ptr_array_index(last_exec, cpu_index);
+ g_rw_lock_reader_unlock(&expand_array_lock);
/* Print previous instruction in cache */
if (s->len) {
--
2.42.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 05/12] gdbstub: Introduce GDBFeature structure
2023-09-12 22:40 [PATCH v3 00/12] gdbstub and TCG plugin improvements Akihiko Odaki
` (3 preceding siblings ...)
2023-09-12 22:40 ` [PATCH v3 04/12] contrib/plugins: Use GRWLock in execlog Akihiko Odaki
@ 2023-09-12 22:40 ` Akihiko Odaki
2023-09-12 22:40 ` [PATCH v3 06/12] target/arm: Move the reference to arm-core.xml Akihiko Odaki
` (7 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2023-09-12 22:40 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Richard Henderson,
Paolo Bonzini, Marc-André Lureau, Daniel P. Berrangé,
Thomas Huth, John Snow, Cleber Rosa
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 <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
MAINTAINERS | 2 +-
meson.build | 2 +-
include/exec/gdbstub.h | 9 ++++--
gdbstub/gdbstub.c | 6 ++--
stubs/gdbstub.c | 6 ++--
scripts/feature_to_c.py | 48 ++++++++++++++++++++++++++++
scripts/feature_to_c.sh | 69 -----------------------------------------
7 files changed, 63 insertions(+), 79 deletions(-)
create mode 100755 scripts/feature_to_c.py
delete mode 100644 scripts/feature_to_c.sh
diff --git a/MAINTAINERS b/MAINTAINERS
index 6111b6b4d9..61ff9234d3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2826,7 +2826,7 @@ F: include/exec/gdbstub.h
F: include/gdbstub/*
F: gdb-xml/
F: tests/tcg/multiarch/gdbstub/
-F: scripts/feature_to_c.sh
+F: scripts/feature_to_c.py
F: scripts/probe-gdb-support.py
Memory API
diff --git a/meson.build b/meson.build
index 98e68ef0b1..5c633f7e01 100644
--- a/meson.build
+++ b/meson.build
@@ -3683,7 +3683,7 @@ common_all = static_library('common',
dependencies: common_all.dependencies(),
name_suffix: 'fa')
-feature_to_c = find_program('scripts/feature_to_c.sh')
+feature_to_c = find_program('scripts/feature_to_c.py')
if targetos == 'darwin'
entitlement = find_program('scripts/entitlement.sh')
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 16a139043f..705be2c5d7 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -10,6 +10,11 @@
#define GDB_WATCHPOINT_READ 3
#define GDB_WATCHPOINT_ACCESS 4
+typedef struct GDBFeature {
+ const char *xmlname;
+ const char *xml;
+} GDBFeature;
+
/* Get or set a register. Returns the size of the register. */
typedef int (*gdb_get_reg_cb)(CPUArchState *env, GByteArray *buf, int reg);
@@ -48,7 +53,7 @@ void gdb_set_stop_cpu(CPUState *cpu);
*/
bool gdb_has_xml(void);
-/* in gdbstub-xml.c, generated by scripts/feature_to_c.sh */
-extern const char *const xml_builtin[][2];
+/* in gdbstub-xml.c, generated by scripts/feature_to_c.py */
+extern const GDBFeature gdb_static_features[];
#endif
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 384191bcb0..12f4d07046 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;
}
}
diff --git a/stubs/gdbstub.c b/stubs/gdbstub.c
index 2b7aee50d3..580e20702b 100644
--- a/stubs/gdbstub.c
+++ b/stubs/gdbstub.c
@@ -1,6 +1,6 @@
#include "qemu/osdep.h"
-#include "exec/gdbstub.h" /* xml_builtin */
+#include "exec/gdbstub.h" /* gdb_static_features */
-const char *const xml_builtin[][2] = {
- { NULL, NULL }
+const GDBFeature gdb_static_features[] = {
+ { NULL }
};
diff --git a/scripts/feature_to_c.py b/scripts/feature_to_c.py
new file mode 100755
index 0000000000..bcbcb83beb
--- /dev/null
+++ b/scripts/feature_to_c.py
@@ -0,0 +1,48 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+import os, sys
+
+def writeliteral(indent, bytes):
+ sys.stdout.write(' ' * indent)
+ sys.stdout.write('"')
+ quoted = True
+
+ for c in bytes:
+ if not quoted:
+ sys.stdout.write('\n')
+ sys.stdout.write(' ' * indent)
+ sys.stdout.write('"')
+ quoted = True
+
+ if c == b'"'[0]:
+ sys.stdout.write('\\"')
+ elif c == b'\\'[0]:
+ sys.stdout.write('\\\\')
+ elif c == b'\n'[0]:
+ sys.stdout.write('\\n"')
+ quoted = False
+ elif c >= 32 and c < 127:
+ sys.stdout.write(c.to_bytes(1, 'big').decode())
+ else:
+ sys.stdout.write(f'\{c:03o}')
+
+ if quoted:
+ sys.stdout.write('"')
+
+sys.stdout.write('#include "qemu/osdep.h"\n' \
+ '#include "exec/gdbstub.h"\n' \
+ '\n'
+ 'const GDBFeature gdb_static_features[] = {\n')
+
+for input in sys.argv[1:]:
+ with open(input, 'rb') as file:
+ read = file.read()
+
+ sys.stdout.write(' {\n')
+ writeliteral(8, bytes(os.path.basename(input), 'utf-8'))
+ sys.stdout.write(',\n')
+ writeliteral(8, read)
+ sys.stdout.write('\n },\n')
+
+sys.stdout.write(' { NULL }\n};\n')
diff --git a/scripts/feature_to_c.sh b/scripts/feature_to_c.sh
deleted file mode 100644
index c1f67c8f6a..0000000000
--- a/scripts/feature_to_c.sh
+++ /dev/null
@@ -1,69 +0,0 @@
-#!/bin/sh
-
-# Convert text files to compilable C arrays.
-#
-# Copyright (C) 2007 Free Software Foundation, Inc.
-#
-# This file is part of GDB.
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, see <http://www.gnu.org/licenses/>.
-
-if test -z "$1"; then
- echo "Usage: $0 INPUTFILE..."
- exit 1
-fi
-
-for input; do
- arrayname=xml_feature_$(echo $input | sed 's,.*/,,; s/[-.]/_/g')
-
- ${AWK:-awk} 'BEGIN { n = 0
- printf "#include \"qemu/osdep.h\"\n"
- print "static const char '$arrayname'[] = {"
- for (i = 0; i < 255; i++)
- _ord_[sprintf("%c", i)] = i
- } {
- split($0, line, "");
- printf " "
- for (i = 1; i <= length($0); i++) {
- c = line[i]
- if (c == "'\''") {
- printf "'\''\\'\'''\'', "
- } else if (c == "\\") {
- printf "'\''\\\\'\'', "
- } else if (_ord_[c] >= 32 && _ord_[c] < 127) {
- printf "'\''%s'\'', ", c
- } else {
- printf "'\''\\%03o'\'', ", _ord_[c]
- }
- if (i % 10 == 0)
- printf "\n "
- }
- printf "'\''\\n'\'', \n"
- } END {
- print " 0 };"
- }' < $input
-done
-
-echo
-echo '#include "exec/gdbstub.h"'
-echo "const char *const xml_builtin[][2] = {"
-
-for input; do
- basename=$(echo $input | sed 's,.*/,,')
- arrayname=xml_feature_$(echo $input | sed 's,.*/,,; s/[-.]/_/g')
- echo " { \"$basename\", $arrayname },"
-done
-
-echo " { (char *)0, (char *)0 }"
-echo "};"
--
2.42.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 06/12] target/arm: Move the reference to arm-core.xml
2023-09-12 22:40 [PATCH v3 00/12] gdbstub and TCG plugin improvements Akihiko Odaki
` (4 preceding siblings ...)
2023-09-12 22:40 ` [PATCH v3 05/12] gdbstub: Introduce GDBFeature structure Akihiko Odaki
@ 2023-09-12 22:40 ` Akihiko Odaki
2023-09-12 22:40 ` [PATCH v3 07/12] hw/core/cpu: Return static value with gdb_arch_name() Akihiko Odaki
` (6 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2023-09-12 22:40 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Richard Henderson,
Peter Maydell, open list:ARM TCG CPUs
Some subclasses overwrite gdb_core_xml_file member but others don't.
Always initialize the member in the subclasses for consistency.
This especially helps for AArch64; in a following change, the file
specified by gdb_core_xml_file is always looked up even if it's going to
be overwritten later. Looking up arm-core.xml results in an error as
it will not be embedded in the AArch64 build.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 0bb0585441..6ff6ff2d55 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2389,7 +2389,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
cc->sysemu_ops = &arm_sysemu_ops;
#endif
cc->gdb_num_core_regs = 26;
- cc->gdb_core_xml_file = "arm-core.xml";
cc->gdb_arch_name = arm_gdb_arch_name;
cc->gdb_get_dynamic_xml = arm_gdb_get_dynamic_xml;
cc->gdb_stop_before_watchpoint = true;
@@ -2411,8 +2410,10 @@ static void arm_cpu_instance_init(Object *obj)
static void cpu_register_class_init(ObjectClass *oc, void *data)
{
ARMCPUClass *acc = ARM_CPU_CLASS(oc);
+ CPUClass *cc = CPU_CLASS(acc);
acc->info = data;
+ cc->gdb_core_xml_file = "arm-core.xml";
}
void arm_cpu_register(const ARMCPUInfo *info)
--
2.42.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 07/12] hw/core/cpu: Return static value with gdb_arch_name()
2023-09-12 22:40 [PATCH v3 00/12] gdbstub and TCG plugin improvements Akihiko Odaki
` (5 preceding siblings ...)
2023-09-12 22:40 ` [PATCH v3 06/12] target/arm: Move the reference to arm-core.xml Akihiko Odaki
@ 2023-09-12 22:40 ` Akihiko Odaki
2023-09-12 22:40 ` [PATCH v3 08/12] gdbstub: Use g_markup_printf_escaped() Akihiko Odaki
` (5 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2023-09-12 22:40 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Eduardo Habkost,
Marcel Apfelbaum, Yanan Wang, Peter Maydell, Song Gao,
Xiaojuan Yang, Daniel Henrique Barboza, Cédric Le Goater,
David Gibson, Greg Kurz, Nicholas Piggin, Palmer Dabbelt,
Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei, Thomas Huth,
Richard Henderson, David Hildenbrand, Ilya Leoshkevich,
Bastian Koppelmann, open list:ARM TCG CPUs,
open list:PowerPC TCG CPUs, open list:RISC-V TCG CPUs,
open list:S390 general arch...
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 <akihiko.odaki@daynix.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
include/hw/core/cpu.h | 2 +-
target/ppc/internal.h | 2 +-
gdbstub/gdbstub.c | 3 +--
target/arm/cpu.c | 6 +++---
target/arm/cpu64.c | 4 ++--
target/i386/cpu.c | 6 +++---
target/loongarch/cpu.c | 8 ++++----
target/ppc/gdbstub.c | 6 +++---
target/riscv/cpu.c | 6 +++---
target/s390x/cpu.c | 4 ++--
target/tricore/cpu.c | 4 ++--
11 files changed, 25 insertions(+), 26 deletions(-)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index fdcbe87352..4f5c7eb04e 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -164,7 +164,7 @@ struct CPUClass {
vaddr (*gdb_adjust_breakpoint)(CPUState *cpu, vaddr addr);
const char *gdb_core_xml_file;
- gchar * (*gdb_arch_name)(CPUState *cpu);
+ const gchar * (*gdb_arch_name)(CPUState *cpu);
const char * (*gdb_get_dynamic_xml)(CPUState *cpu, const char *xmlname);
void (*disas_set_info)(CPUState *cpu, disassemble_info *info);
diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index 57acb3212c..974b37aa60 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -221,7 +221,7 @@ void destroy_ppc_opcodes(PowerPCCPU *cpu);
/* gdbstub.c */
void ppc_gdb_init(CPUState *cs, PowerPCCPUClass *ppc);
-gchar *ppc_gdb_arch_name(CPUState *cs);
+const gchar *ppc_gdb_arch_name(CPUState *cs);
/**
* prot_for_access_type:
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 12f4d07046..9db4af41c1 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,
"<target>");
if (cc->gdb_arch_name) {
- g_autofree gchar *arch = cc->gdb_arch_name(cpu);
g_string_append_printf(xml,
"<architecture>%s</architecture>",
- arch);
+ cc->gdb_arch_name(cpu));
}
g_string_append(xml, "<xi:include href=\"");
g_string_append(xml, cc->gdb_core_xml_file);
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 6ff6ff2d55..a13c609249 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2316,15 +2316,15 @@ static Property arm_cpu_properties[] = {
DEFINE_PROP_END_OF_LIST()
};
-static gchar *arm_gdb_arch_name(CPUState *cs)
+static const gchar *arm_gdb_arch_name(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
- return g_strdup("iwmmxt");
+ return "iwmmxt";
}
- return g_strdup("arm");
+ return "arm";
}
#ifndef CONFIG_USER_ONLY
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 96158093cc..6b91aab6b7 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -743,9 +743,9 @@ static void aarch64_cpu_finalizefn(Object *obj)
{
}
-static gchar *aarch64_gdb_arch_name(CPUState *cs)
+static const gchar *aarch64_gdb_arch_name(CPUState *cs)
{
- return g_strdup("aarch64");
+ return "aarch64";
}
static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 00f913b638..5678b52472 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5914,12 +5914,12 @@ static void x86_cpu_load_model(X86CPU *cpu, X86CPUModel *model)
memset(&env->user_features, 0, sizeof(env->user_features));
}
-static gchar *x86_gdb_arch_name(CPUState *cs)
+static const gchar *x86_gdb_arch_name(CPUState *cs)
{
#ifdef TARGET_X86_64
- return g_strdup("i386:x86-64");
+ return "i386:x86-64";
#else
- return g_strdup("i386");
+ return "i386";
#endif
}
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 27fc6e1f33..f88cfa93ce 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -763,9 +763,9 @@ static void loongarch_cpu_class_init(ObjectClass *c, void *data)
#endif
}
-static gchar *loongarch32_gdb_arch_name(CPUState *cs)
+static const gchar *loongarch32_gdb_arch_name(CPUState *cs)
{
- return g_strdup("loongarch32");
+ return "loongarch32";
}
static void loongarch32_cpu_class_init(ObjectClass *c, void *data)
@@ -777,9 +777,9 @@ static void loongarch32_cpu_class_init(ObjectClass *c, void *data)
cc->gdb_arch_name = loongarch32_gdb_arch_name;
}
-static gchar *loongarch64_gdb_arch_name(CPUState *cs)
+static const gchar *loongarch64_gdb_arch_name(CPUState *cs)
{
- return g_strdup("loongarch64");
+ return "loongarch64";
}
static void loongarch64_cpu_class_init(ObjectClass *c, void *data)
diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
index 2ad11510bf..778ef73bd7 100644
--- a/target/ppc/gdbstub.c
+++ b/target/ppc/gdbstub.c
@@ -589,12 +589,12 @@ static int gdb_set_vsx_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
return 0;
}
-gchar *ppc_gdb_arch_name(CPUState *cs)
+const gchar *ppc_gdb_arch_name(CPUState *cs)
{
#if defined(TARGET_PPC64)
- return g_strdup("powerpc:common64");
+ return "powerpc:common64";
#else
- return g_strdup("powerpc:common");
+ return "powerpc:common";
#endif
}
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 6b93b04453..ef8faaaff5 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1939,17 +1939,17 @@ static Property riscv_cpu_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
-static gchar *riscv_gdb_arch_name(CPUState *cs)
+static const gchar *riscv_gdb_arch_name(CPUState *cs)
{
RISCVCPU *cpu = RISCV_CPU(cs);
CPURISCVState *env = &cpu->env;
switch (riscv_cpu_mxl(env)) {
case MXL_RV32:
- return g_strdup("riscv:rv32");
+ return "riscv:rv32";
case MXL_RV64:
case MXL_RV128:
- return g_strdup("riscv:rv64");
+ return "riscv:rv64";
default:
g_assert_not_reached();
}
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index df167493c3..cf4b5e43f2 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -284,9 +284,9 @@ static void s390_cpu_initfn(Object *obj)
#endif
}
-static gchar *s390_gdb_arch_name(CPUState *cs)
+static const gchar *s390_gdb_arch_name(CPUState *cs)
{
- return g_strdup("s390:64-bit");
+ return "s390:64-bit";
}
static Property s390x_cpu_properties[] = {
diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
index 133a9ac70e..44e3ba6f0e 100644
--- a/target/tricore/cpu.c
+++ b/target/tricore/cpu.c
@@ -29,9 +29,9 @@ static inline void set_feature(CPUTriCoreState *env, int feature)
env->features |= 1ULL << feature;
}
-static gchar *tricore_gdb_arch_name(CPUState *cs)
+static const gchar *tricore_gdb_arch_name(CPUState *cs)
{
- return g_strdup("tricore");
+ return "tricore";
}
static void tricore_cpu_set_pc(CPUState *cs, vaddr value)
--
2.42.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 08/12] gdbstub: Use g_markup_printf_escaped()
2023-09-12 22:40 [PATCH v3 00/12] gdbstub and TCG plugin improvements Akihiko Odaki
` (6 preceding siblings ...)
2023-09-12 22:40 ` [PATCH v3 07/12] hw/core/cpu: Return static value with gdb_arch_name() Akihiko Odaki
@ 2023-09-12 22:40 ` Akihiko Odaki
2023-09-12 22:40 ` [PATCH v3 09/12] target/arm: Remove references to gdb_has_xml Akihiko Odaki
` (4 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2023-09-12 22:40 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki
g_markup_printf_escaped() is a safer alternative to simple printf() as
it automatically escapes values.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
gdbstub/gdbstub.c | 36 +++++++++++++++++++++---------------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 9db4af41c1..a4f2bf3723 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("<?xml version=\"1.0\"?>");
+ g_autoptr(GPtrArray) xml = g_ptr_array_new_with_free_func(g_free);
- g_string_append(xml,
- "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
- "<target>");
+ g_ptr_array_add(
+ xml,
+ g_strdup("<?xml version=\"1.0\"?>"
+ "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
+ "<target>"));
if (cc->gdb_arch_name) {
- g_string_append_printf(xml,
- "<architecture>%s</architecture>",
- cc->gdb_arch_name(cpu));
+ g_ptr_array_add(
+ xml,
+ g_markup_printf_escaped("<architecture>%s</architecture>",
+ cc->gdb_arch_name(cpu)));
}
- g_string_append(xml, "<xi:include href=\"");
- g_string_append(xml, cc->gdb_core_xml_file);
- g_string_append(xml, "\"/>");
+ g_ptr_array_add(
+ xml,
+ g_markup_printf_escaped("<xi:include href=\"%s\"/>",
+ cc->gdb_core_xml_file));
for (r = cpu->gdb_regs; r; r = r->next) {
- g_string_append(xml, "<xi:include href=\"");
- g_string_append(xml, r->xml);
- g_string_append(xml, "\"/>");
+ g_ptr_array_add(
+ xml,
+ g_markup_printf_escaped("<xi:include href=\"%s\"/>",
+ r->xml));
}
- g_string_append(xml, "</target>");
+ g_ptr_array_add(xml, g_strdup("</target>"));
+ 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;
}
--
2.42.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 09/12] target/arm: Remove references to gdb_has_xml
2023-09-12 22:40 [PATCH v3 00/12] gdbstub and TCG plugin improvements Akihiko Odaki
` (7 preceding siblings ...)
2023-09-12 22:40 ` [PATCH v3 08/12] gdbstub: Use g_markup_printf_escaped() Akihiko Odaki
@ 2023-09-12 22:40 ` Akihiko Odaki
2023-09-12 22:40 ` [PATCH v3 10/12] target/ppc: " Akihiko Odaki
` (3 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2023-09-12 22:40 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Peter Maydell,
open list:ARM TCG CPUs
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 <akihiko.odaki@daynix.com>
Acked-by: Alex Bennée <alex.bennee@linaro.org>
---
target/arm/gdbstub.c | 32 ++------------------------------
1 file changed, 2 insertions(+), 30 deletions(-)
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 8fc8351df7..b7ace24bfc 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -46,21 +46,7 @@ int arm_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
/* Core integer register. */
return gdb_get_reg32(mem_buf, env->regs[n]);
}
- if (n < 24) {
- /* FPA registers. */
- if (gdb_has_xml()) {
- return 0;
- }
- return gdb_get_zeroes(mem_buf, 12);
- }
- switch (n) {
- case 24:
- /* FPA status register. */
- if (gdb_has_xml()) {
- return 0;
- }
- return gdb_get_reg32(mem_buf, 0);
- case 25:
+ if (n == 25) {
/* CPSR, or XPSR for M-profile */
if (arm_feature(env, ARM_FEATURE_M)) {
return gdb_get_reg32(mem_buf, xpsr_read(env));
@@ -100,21 +86,7 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
env->regs[n] = tmp;
return 4;
}
- if (n < 24) { /* 16-23 */
- /* FPA registers (ignored). */
- if (gdb_has_xml()) {
- return 0;
- }
- return 12;
- }
- switch (n) {
- case 24:
- /* FPA status register (ignored). */
- if (gdb_has_xml()) {
- return 0;
- }
- return 4;
- case 25:
+ if (n == 25) {
/* CPSR, or XPSR for M-profile */
if (arm_feature(env, ARM_FEATURE_M)) {
/*
--
2.42.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 10/12] target/ppc: Remove references to gdb_has_xml
2023-09-12 22:40 [PATCH v3 00/12] gdbstub and TCG plugin improvements Akihiko Odaki
` (8 preceding siblings ...)
2023-09-12 22:40 ` [PATCH v3 09/12] target/arm: Remove references to gdb_has_xml Akihiko Odaki
@ 2023-09-12 22:40 ` Akihiko Odaki
2023-09-12 22:41 ` [PATCH v3 11/12] gdbstub: Remove gdb_has_xml variable Akihiko Odaki
` (2 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2023-09-12 22:40 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki,
Daniel Henrique Barboza, Cédric Le Goater, David Gibson,
Greg Kurz, Nicholas Piggin, open list:PowerPC TCG CPUs
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 <akihiko.odaki@daynix.com>
---
target/ppc/gdbstub.c | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
index 778ef73bd7..ec5731e5d6 100644
--- a/target/ppc/gdbstub.c
+++ b/target/ppc/gdbstub.c
@@ -54,12 +54,6 @@ static int ppc_gdb_register_len(int n)
case 0 ... 31:
/* gprs */
return sizeof(target_ulong);
- case 32 ... 63:
- /* fprs */
- if (gdb_has_xml()) {
- return 0;
- }
- return 8;
case 66:
/* cr */
case 69:
@@ -74,12 +68,6 @@ static int ppc_gdb_register_len(int n)
case 68:
/* ctr */
return sizeof(target_ulong);
- case 70:
- /* fpscr */
- if (gdb_has_xml()) {
- return 0;
- }
- return sizeof(target_ulong);
default:
return 0;
}
@@ -132,9 +120,6 @@ int ppc_cpu_gdb_read_register(CPUState *cs, GByteArray *buf, int n)
if (n < 32) {
/* gprs */
gdb_get_regl(buf, env->gpr[n]);
- } else if (n < 64) {
- /* fprs */
- gdb_get_reg64(buf, *cpu_fpr_ptr(env, n - 32));
} else {
switch (n) {
case 64:
@@ -158,9 +143,6 @@ int ppc_cpu_gdb_read_register(CPUState *cs, GByteArray *buf, int n)
case 69:
gdb_get_reg32(buf, cpu_read_xer(env));
break;
- case 70:
- gdb_get_reg32(buf, env->fpscr);
- break;
}
}
mem_buf = buf->data + buf->len - r;
--
2.42.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 11/12] gdbstub: Remove gdb_has_xml variable
2023-09-12 22:40 [PATCH v3 00/12] gdbstub and TCG plugin improvements Akihiko Odaki
` (9 preceding siblings ...)
2023-09-12 22:40 ` [PATCH v3 10/12] target/ppc: " Akihiko Odaki
@ 2023-09-12 22:41 ` Akihiko Odaki
2023-09-12 22:41 ` [PATCH v3 12/12] gdbstub: Replace gdb_regs with an array Akihiko Odaki
2023-09-14 15:56 ` [PATCH v3 00/12] gdbstub and TCG plugin improvements Alex Bennée
12 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2023-09-12 22:41 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki
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 <akihiko.odaki@daynix.com>
---
gdbstub/internals.h | 2 --
include/exec/gdbstub.h | 8 --------
gdbstub/gdbstub.c | 15 ---------------
3 files changed, 25 deletions(-)
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index fee243081f..7128c4aa85 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -32,8 +32,6 @@ enum {
typedef struct GDBProcess {
uint32_t pid;
bool attached;
-
- /* If gdb sends qXfer:features:read:target.xml this will be populated */
char *target_xml;
} GDBProcess;
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 705be2c5d7..1a01c35f8e 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -45,14 +45,6 @@ int gdbserver_start(const char *port_or_device);
void gdb_set_stop_cpu(CPUState *cpu);
-/**
- * gdb_has_xml() - report of gdb supports modern target descriptions
- *
- * This will report true if the gdb negotiated qXfer:features:read
- * target descriptions.
- */
-bool gdb_has_xml(void);
-
/* in gdbstub-xml.c, generated by scripts/feature_to_c.py */
extern const GDBFeature gdb_static_features[];
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index a4f2bf3723..177dce9ba2 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;
--
2.42.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 12/12] gdbstub: Replace gdb_regs with an array
2023-09-12 22:40 [PATCH v3 00/12] gdbstub and TCG plugin improvements Akihiko Odaki
` (10 preceding siblings ...)
2023-09-12 22:41 ` [PATCH v3 11/12] gdbstub: Remove gdb_has_xml variable Akihiko Odaki
@ 2023-09-12 22:41 ` Akihiko Odaki
2023-09-14 15:56 ` [PATCH v3 00/12] gdbstub and TCG plugin improvements Alex Bennée
12 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2023-09-12 22:41 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Eduardo Habkost,
Marcel Apfelbaum, Yanan Wang
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 <akihiko.odaki@daynix.com>
---
include/hw/core/cpu.h | 2 +-
gdbstub/gdbstub.c | 34 ++++++++++++++++++++--------------
2 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 4f5c7eb04e..c84c631242 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -375,7 +375,7 @@ struct CPUState {
CPUJumpCache *tb_jmp_cache;
- struct GDBRegisterState *gdb_regs;
+ GArray *gdb_regs;
int gdb_num_regs;
int gdb_num_g_regs;
QTAILQ_ENTRY(CPUState) node;
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 177dce9ba2..9810d15278 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("<xi:include href=\"%s\"/>",
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("<xi:include href=\"%s\"/>",
@@ -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,22 @@ 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 +487,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', "
--
2.42.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 00/12] gdbstub and TCG plugin improvements
2023-09-12 22:40 [PATCH v3 00/12] gdbstub and TCG plugin improvements Akihiko Odaki
` (11 preceding siblings ...)
2023-09-12 22:41 ` [PATCH v3 12/12] gdbstub: Replace gdb_regs with an array Akihiko Odaki
@ 2023-09-14 15:56 ` Alex Bennée
2023-09-14 17:18 ` Philippe Mathieu-Daudé
12 siblings, 1 reply; 18+ messages in thread
From: Alex Bennée @ 2023-09-14 15:56 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé
Akihiko Odaki <akihiko.odaki@daynix.com> writes:
> This series extracts fixes and refactorings that can be applied
> independently from "[PATCH RESEND v5 00/26] plugins: Allow to read
> registers" as suggested by Nicholas Piggin.
>
> Patch "target/ppc: Remove references to gdb_has_xml" is also updated to
> remove some dead code I missed earlier and thus the Reviewed-by tag is
> dropped.
Queued to gdbstub/next, thanks.
>
> V2 -> V3:
> Added patch "plugins: Check if vCPU is realized".
>
> V1 -> V2:
> Rebased.
> Added patch "gdbstub: Fix target_xml initialization".
> Added patch "gdbstub: Fix target.xml response".
> Added patch "gdbstub: Replace gdb_regs with an array".
>
> Akihiko Odaki (12):
> gdbstub: Fix target_xml initialization
> gdbstub: Fix target.xml response
> plugins: Check if vCPU is realized
> contrib/plugins: Use GRWLock in execlog
> gdbstub: Introduce GDBFeature structure
> target/arm: Move the reference to arm-core.xml
> hw/core/cpu: Return static value with gdb_arch_name()
> gdbstub: Use g_markup_printf_escaped()
> target/arm: Remove references to gdb_has_xml
> target/ppc: Remove references to gdb_has_xml
> gdbstub: Remove gdb_has_xml variable
> gdbstub: Replace gdb_regs with an array
>
> MAINTAINERS | 2 +-
> meson.build | 2 +-
> gdbstub/internals.h | 2 -
> include/exec/gdbstub.h | 17 +++----
> include/hw/core/cpu.h | 4 +-
> target/ppc/internal.h | 2 +-
> contrib/plugins/execlog.c | 16 ++++---
> gdbstub/gdbstub.c | 94 +++++++++++++++++++--------------------
> gdbstub/softmmu.c | 2 +-
> plugins/core.c | 2 +-
> stubs/gdbstub.c | 6 +--
> target/arm/cpu.c | 9 ++--
> target/arm/cpu64.c | 4 +-
> target/arm/gdbstub.c | 32 +------------
> target/i386/cpu.c | 6 +--
> target/loongarch/cpu.c | 8 ++--
> target/ppc/gdbstub.c | 24 ++--------
> target/riscv/cpu.c | 6 +--
> target/s390x/cpu.c | 4 +-
> target/tricore/cpu.c | 4 +-
> scripts/feature_to_c.py | 48 ++++++++++++++++++++
> scripts/feature_to_c.sh | 69 ----------------------------
> 22 files changed, 146 insertions(+), 217 deletions(-)
> create mode 100755 scripts/feature_to_c.py
> delete mode 100644 scripts/feature_to_c.sh
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 18+ messages in thread