On Fri, 08 Aug 2025 05:06, Pierrick Bouvier <[email protected]> wrote:
>We define a scoreboard that will hold our data per cpu. As well, we
>define a buffer per cpu that will be used to read registers and memories
>in a thread-safe way.
>
>For now, we just instrument all instructions with an empty callback.
>
>Signed-off-by: Pierrick Bouvier <[email protected]>
>---
> contrib/plugins/uftrace.c | 74 +++++++++++++++++++++++++++++++++++++
> contrib/plugins/meson.build | 3 +-
> 2 files changed, 76 insertions(+), 1 deletion(-)
> create mode 100644 contrib/plugins/uftrace.c
>
>diff --git a/contrib/plugins/uftrace.c b/contrib/plugins/uftrace.c
>new file mode 100644
>index 00000000000..d60c1077496
>--- /dev/null
>+++ b/contrib/plugins/uftrace.c
>@@ -0,0 +1,74 @@
>+/*
>+ * Copyright (C) 2025, Pierrick Bouvier <[email protected]>
>+ *
>+ * Generates a trace compatible with uftrace (similar to uftrace record).
>+ * https://github.com/namhyung/uftrace
>+ *
>+ * See docs/about/emulation.rst|Uftrace for details and examples.
>+ *
>+ * SPDX-License-Identifier: GPL-2.0-or-later
>+ */
>+
>+#include <qemu-plugin.h>
>+#include <glib.h>
>+
>+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
>+
>+typedef struct Cpu {
>+ GByteArray *buf;
>+} Cpu;
>+
>+static struct qemu_plugin_scoreboard *score;
>+
>+static void track_callstack(unsigned int cpu_index, void *udata)
>+{
>+}
>+
>+static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
>+{
>+ size_t n_insns = qemu_plugin_tb_n_insns(tb);
>+
>+ for (int i = 0; i < n_insns; i++) {
s/int i/size_t i/
>+ struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
This can return NULL,
>+
>+ uintptr_t pc = qemu_plugin_insn_vaddr(insn);
And this would lead to a NULL dereference (it performs insn->vaddr
access)
>+ qemu_plugin_register_vcpu_insn_exec_cb(insn, track_callstack,
>+ QEMU_PLUGIN_CB_R_REGS,
>+ (void *) pc);
Hm indentation got broken here, should be
+ qemu_plugin_register_vcpu_insn_exec_cb(insn, track_callstack,
+ QEMU_PLUGIN_CB_R_REGS,
+ (void *)pc);
>+
>+ }
>+}
>+
>+static void vcpu_init(qemu_plugin_id_t id, unsigned int vcpu_index)
>+{
>+ Cpu *cpu = qemu_plugin_scoreboard_find(score, vcpu_index);
>+ cpu->buf = g_byte_array_new();
>+}
>+
>+static void vcpu_end(unsigned int vcpu_index)
>+{
>+ Cpu *cpu = qemu_plugin_scoreboard_find(score, vcpu_index);
>+ g_byte_array_free(cpu->buf, true);
>+ memset(cpu, 0, sizeof(Cpu));
Nitpick, cpu->buf = NULL; is easier to understand (suggestion)
>+}
>+
>+static void at_exit(qemu_plugin_id_t id, void *data)
>+{
>+ for (size_t i = 0; i < qemu_plugin_num_vcpus(); ++i) {
>+ vcpu_end(i);
>+ }
>+
>+ qemu_plugin_scoreboard_free(score);
>+}
>+
>+QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
>+ const qemu_info_t *info,
>+ int argc, char **argv)
>+{
>+ score = qemu_plugin_scoreboard_new(sizeof(Cpu));
>+ qemu_plugin_register_vcpu_init_cb(id, vcpu_init);
>+ qemu_plugin_register_atexit_cb(id, at_exit, NULL);
>+ qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
>+
>+ return 0;
>+}
>diff --git a/contrib/plugins/meson.build b/contrib/plugins/meson.build
>index 1876bc78438..7eb3629c95d 100644
>--- a/contrib/plugins/meson.build
>+++ b/contrib/plugins/meson.build
>@@ -1,5 +1,6 @@
> contrib_plugins = ['bbv', 'cache', 'cflow', 'drcov', 'execlog', 'hotblocks',
>- 'hotpages', 'howvec', 'hwprofile', 'ips', 'stoptrigger']
>+ 'hotpages', 'howvec', 'hwprofile', 'ips', 'stoptrigger',
>+ 'uftrace']
> if host_os != 'windows'
> # lockstep uses socket.h
> contrib_plugins += 'lockstep'
>--
>2.47.2
>
If no other comments rise for this patch, you can add my r-b after
fixing the NULL check:
Reviewed-by: Manos Pitsidianakis <[email protected]>
