On 04/03/2026 16:45, Florian Hofhammer wrote: > On 04/03/2026 16:35, Florian Hofhammer wrote: >> On 03/03/2026 20:46, Pierrick Bouvier wrote: >>> On 3/3/26 5:07 AM, Florian Hofhammer wrote: >>>> The test plugin intercepts execution in different contexts. Without the >>>> plugin, any of the implemented test functions would trigger an assert >>>> and fail. With the plugin, control flow is redirected to skip the assert >>>> and return cleanly via the qemu_plugin_set_pc() API. >>>> >>>> Signed-off-by: Florian Hofhammer <[email protected]> >>>> --- >>>> MAINTAINERS | 1 + >>>> tests/tcg/arm/Makefile.target | 6 + >>>> tests/tcg/multiarch/Makefile.target | 17 ++- >>>> .../multiarch/{ => plugin}/check-plugin-output.sh | 0 >>>> .../{ => plugin}/test-plugin-mem-access.c | 0 >>>> tests/tcg/multiarch/plugin/test-plugin-set-pc.c | 140 >>>> +++++++++++++++++++++ >>>> tests/tcg/plugins/meson.build | 1 + >>>> tests/tcg/plugins/setpc.c | 120 >>>> ++++++++++++++++++ >>>> 8 files changed, 282 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/MAINTAINERS b/MAINTAINERS >>>> index 6698e5ff69..63c0af4d86 100644 >>>> --- a/MAINTAINERS >>>> +++ b/MAINTAINERS >>>> @@ -4104,6 +4104,7 @@ S: Maintained >>>> F: docs/devel/tcg-plugins.rst >>>> F: plugins/ >>>> F: tests/tcg/plugins/ >>>> +F: tests/tcg/multiarch/plugin/ >>>> F: tests/functional/aarch64/test_tcg_plugins.py >>>> F: contrib/plugins/ >>>> F: scripts/qemu-plugin-symbols.py >>>> diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target >>>> index 6189d7a0e2..613bbf0939 100644 >>>> --- a/tests/tcg/arm/Makefile.target >>>> +++ b/tests/tcg/arm/Makefile.target >>>> @@ -78,4 +78,10 @@ sha512-vector: sha512.c >>>> ARM_TESTS += sha512-vector >>>> +ifeq ($(CONFIG_PLUGIN),y) >>>> +# Require emitting arm32 instructions, otherwise the vCPU might >>>> accidentally >>>> +# try to execute Thumb instructions in arm32 mode after >>>> qemu_plugin_set_pc() >>>> +test-plugin-set-pc: CFLAGS+=-marm >>>> +endif >>>> + >>> >>> Is that still needed? >> >> Yes, unfortunately. The compiler emits Thumb2 instructions where it >> deems it appropriate but the addresses of functions/labels are still at >> least 2-byte aligned and therefore don't have the bottom bit set. >> Setting the PC to an even address therefore makes the vCPU think the >> target is in arm32 mode, and it tries to execute the Thumb2 instructions >> in arm32 mode. I'd either need to ensure that the targets are always in >> Thumb2 mode and set the bottom bit myself, or ensure that everything is >> in arm32 mode from the start. >> >>>> TESTS += $(ARM_TESTS) >>>> diff --git a/tests/tcg/multiarch/Makefile.target >>>> b/tests/tcg/multiarch/Makefile.target >>>> index 07d0b27bdd..a347efbadf 100644 >>>> --- a/tests/tcg/multiarch/Makefile.target >>>> +++ b/tests/tcg/multiarch/Makefile.target >>>> @@ -14,6 +14,10 @@ ifeq ($(filter %-linux-user, $(TARGET)),$(TARGET)) >>>> VPATH += $(MULTIARCH_SRC)/linux >>>> MULTIARCH_SRCS += $(notdir $(wildcard $(MULTIARCH_SRC)/linux/*.c)) >>>> endif >>>> +ifeq ($(CONFIG_PLUGIN),y) >>>> +VPATH += $(MULTIARCH_SRC)/plugin >>>> +MULTIARCH_SRCS += $(notdir $(wildcard $(MULTIARCH_SRC)/plugin/*.c)) >>>> +endif >>>> MULTIARCH_TESTS = $(MULTIARCH_SRCS:.c=) >>>> # >>>> @@ -200,13 +204,20 @@ run-plugin-test-plugin-mem-access-with-libmem.so: \ >>>> PLUGIN_ARGS=$(COMMA)print-accesses=true >>>> run-plugin-test-plugin-mem-access-with-libmem.so: \ >>>> CHECK_PLUGIN_OUTPUT_COMMAND= \ >>>> - $(SRC_PATH)/tests/tcg/multiarch/check-plugin-output.sh \ >>>> + $(SRC_PATH)/tests/tcg/multiarch/plugin/check-plugin-output.sh \ >>>> $(QEMU) $< >>>> run-plugin-test-plugin-syscall-filter-with-libsyscall.so: >>>> +run-plugin-test-plugin-set-pc-with-libsetpc.so: >>>> EXTRA_RUNS_WITH_PLUGIN += >>>> run-plugin-test-plugin-mem-access-with-libmem.so \ >>>> - run-plugin-test-plugin-syscall-filter-with-libsyscall.so >>>> -else >>>> + run-plugin-test-plugin-syscall-filter-with-libsyscall.so \ >>>> + run-plugin-test-plugin-set-pc-with-libsetpc.so >>>> + >>>> +else # CONFIG_PLUGIN=n >>>> +# Do not build the syscall skipping test if it's not tested with the setpc >>>> +# plugin because it will simply fail the test. >>>> +MULTIARCH_TESTS := $(filter-out test-plugin-set-pc, $(MULTIARCH_TESTS)) >>>> + >>>> # test-plugin-syscall-filter needs syscall plugin to succeed >>>> test-plugin-syscall-filter: CFLAGS+=-DSKIP >>>> endif >>>> diff --git a/tests/tcg/multiarch/check-plugin-output.sh >>>> b/tests/tcg/multiarch/plugin/check-plugin-output.sh >>>> similarity index 100% >>>> rename from tests/tcg/multiarch/check-plugin-output.sh >>>> rename to tests/tcg/multiarch/plugin/check-plugin-output.sh >>>> diff --git a/tests/tcg/multiarch/test-plugin-mem-access.c >>>> b/tests/tcg/multiarch/plugin/test-plugin-mem-access.c >>>> similarity index 100% >>>> rename from tests/tcg/multiarch/test-plugin-mem-access.c >>>> rename to tests/tcg/multiarch/plugin/test-plugin-mem-access.c >>>> diff --git a/tests/tcg/multiarch/plugin/test-plugin-set-pc.c >>>> b/tests/tcg/multiarch/plugin/test-plugin-set-pc.c >>>> new file mode 100644 >>>> index 0000000000..40d9a9e8f0 >>>> --- /dev/null >>>> +++ b/tests/tcg/multiarch/plugin/test-plugin-set-pc.c >>>> @@ -0,0 +1,140 @@ >>>> +/* >>>> + * SPDX-License-Identifier: GPL-2.0-or-later >>>> + * >>>> + * Copyright (C) 2026, Florian Hofhammer <[email protected]> >>>> + * >>>> + * This test set exercises the qemu_plugin_set_pc() function in four >>>> different >>>> + * contexts: >>>> + * 1. in a syscall callback, >>>> + * 2. in an instruction callback during normal execution, >>>> + * 3. in an instruction callback during signal handling, >>>> + * 4. in a memory access callback. >>>> + * Note: using the volatile guards is necessary to prevent the compiler >>>> from >>>> + * doing dead code elimination even on -O0, which would cause everything >>>> after >>>> + * the asserts and thus also the target labels to be optimized away. >>>> + */ >>>> +#include <assert.h> >>>> +#include <signal.h> >>>> +#include <stdint.h> >>>> +#include <stdlib.h> >>>> +#include <stdio.h> >>>> +#include <unistd.h> >>>> +#include <setjmp.h> >>>> + >>>> +#define NOINLINE __attribute__((noinline)) >>> >>> Test being compiled in O0, it should not need any noinline attribute. >> >> Ack, removed. >> >>>> +#define NORETURN __attribute__((noreturn)) >>> >>> It's used in a single place, simply move the the attribute there, no need >>> for another define. >> >> Ditto. >> >>>> + >>>> +static int signal_handled; >>>> +/* >>>> + * The volatile variable is used as a guard to prevent the compiler from >>>> + * optimizing away "unreachable" labels. >>>> + */ >>>> +static volatile uint32_t guard = 1; >>>> + >>> >>> As mentioned on v4, you can simply use an external function for failing the >>> test, which won't be tainted with noreturn attribute. >>> This way, you don't need any guard at all or assert(0), and resulting code >>> is linear and easier to read. >>> It test completes, it all worked. If it crashes, something was wrong. >>> >>> void panic(void) >>> { >>> g_assert_not_reached(); >>> } >>> >>> void test_...() { >>> ... >>> on_panic: >>> panic(); >>> after_panic: >>> printf("Hello World\n"); >>> } >> >> I've reworked it to look similar to this (without g_assert_not_reached() >> though, because glib is not available in the test itself). Still >> required some shuffling of stuff to make sure the compiler doesn't >> optimize things away then :) Especially on hexagon, which is the only >> target (by default) built with clang instead of gcc. Clang/LLVM seems to >> be way more aggressive even at -O0 to do deadcode elimination and >> inlining, so I had to add extra CFLAGS for the hexagon target. >> >>>> + * This test executes a magic syscall which communicates two addresses to >>>> the >>>> + * plugin via the syscall arguments. Whenever we reach the "bad" >>>> instruction >>>> + * during normal execution, the plugin should redirect control flow to the >>>> + * "good" instruction instead. >>>> + */ >>>> +NOINLINE void test_insn(void) >>>> +{ >>>> + long ret = syscall(4095, &&bad_insn, &&good_insn, NULL); >>>> + assert(ret == 0 && "Syscall filter did not return expected value"); >>>> + if (guard) { >>>> +bad_insn: >>>> + assert(0 && "PC redirection in instruction callback failed"); >>>> + } else { >>>> +good_insn: >>>> + return; >>>> + } >>>> +} >>>> + >>>> +/* >>>> + * This signal handler communicates a "bad" and a "good" address to the >>>> plugin >>>> + * similar to the previous test, and skips to the "good" address when the >>>> "bad" >>>> + * one is reached. This serves to test whether PC redirection via >>>> + * qemu_plugin_set_pc() also works properly in a signal handler context. >>>> + */ >>>> +NOINLINE void usr1_handler(int signum) >>>> +{ >>>> + long ret = syscall(4095, &&bad_signal, &&good_signal, NULL); >>>> + assert(ret == 0 && "Syscall filter did not return expected value"); >>>> + if (guard) { >>>> +bad_signal: >>>> + assert(0 && "PC redirection in instruction callback failed"); >>>> + } else { >>>> +good_signal: >>>> + signal_handled = 1; >>>> + return; >>>> + } >>>> +} >>>> + >>>> +/* >>>> + * This test sends a signal to the process, which should trigger the above >>>> + * signal handler. The signal handler should then exercise the PC >>>> redirection >>>> + * functionality in the context of a signal handler, which behaves a bit >>>> + * differently from normal execution. >>>> + */ >>>> +NOINLINE void test_sighandler(void) >>>> +{ >>>> + struct sigaction sa = {0}; >>>> + sa.sa_handler = usr1_handler; >>>> + sigaction(SIGUSR1, &sa, NULL); >>>> + pid_t pid = getpid(); >>>> + kill(pid, SIGUSR1); >>>> + assert(signal_handled == 1 && "Signal handler was not executed >>>> properly"); >>>> +} >>>> + >>>> +/* >>>> + * This test communicates a "good" address and the address of a local >>>> variable >>>> + * to the plugin. Upon accessing the local variable, the plugin should >>>> then >>>> + * redirect control flow to the "good" address via qemu_plugin_set_pc(). >>>> + */ >>>> +NOINLINE void test_mem(void) >>>> +{ >>>> + long ret = syscall(4095, NULL, &&good_mem, &guard); >>> >>> Since you use two different syscall numbers, it's worth adding two defines, >>> that you will duplicate in test and plugin. >>> Alternatively, you can use the first parameter of syscall to identify which >>> kind of "service" you want from plugin, with defines duplicated between >>> test and plugin again. >>> Feel free to pick the solution you prefer. >>> >>> Reading 4095 the first time here felt like it was a typo mistake. >> >> Ack, reworked this to be clearer. >> >>> >>>> + assert(ret == 0 && "Syscall filter did not return expected value"); >>>> + if (guard) { >>>> + assert(0 && "PC redirection in memory access callback failed"); >>>> + } else { >>>> +good_mem: >>>> + return; >>>> + } >>>> +} >>>> + >>>> +/* >>>> + * This test executes a magic syscall which is intercepted and its actual >>>> + * execution skipped via the qemu_plugin_set_pc() API. In a proper plugin, >>>> + * syscall skipping would rather be implemented via the syscall filtering >>>> + * callback, but we want to make sure qemu_plugin_set_pc() works in >>>> different >>>> + * contexts. >>>> + */ >>>> +NOINLINE NORETURN >>>> +void test_syscall(void) >>>> +{ >>>> + syscall(4096, &&good_syscall); >>>> + if (guard) { >>>> + assert(0 && "PC redirection in syscall callback failed"); >>>> + } else { >>>> +good_syscall: >>>> + /* >>>> + * Note: we execute this test last and exit straight from here >>>> because >>>> + * when the plugin redirects control flow upon syscall, the stack >>>> frame >>>> + * for the syscall function (and potential other functions in the >>>> call >>>> + * chain in libc) is still live and the stack is not unwound >>>> properly. >>>> + * Thus, returning from here is risky and breaks on some >>>> architectures, >>>> + * so we just exit directly from this test. >>>> + */ >>>> + _exit(EXIT_SUCCESS); >>>> + } >>>> +} >>>> + >>>> + >>>> +int main(int argc, char *argv[]) >>>> +{ >>>> + test_insn(); >>>> + test_sighandler(); >>>> + test_mem(); >>>> + test_syscall(); >>>> +} >>>> diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build >>>> index c5e49753fd..b3e3a9a6d0 100644 >>>> --- a/tests/tcg/plugins/meson.build >>>> +++ b/tests/tcg/plugins/meson.build >>>> @@ -7,6 +7,7 @@ test_plugins = [ >>>> 'mem.c', >>>> 'patch.c', >>>> 'reset.c', >>>> +'setpc.c', >>>> 'syscall.c', >>>> ] >>>> diff --git a/tests/tcg/plugins/setpc.c b/tests/tcg/plugins/setpc.c >>>> new file mode 100644 >>>> index 0000000000..72ae31a0ef >>>> --- /dev/null >>>> +++ b/tests/tcg/plugins/setpc.c >>>> @@ -0,0 +1,120 @@ >>>> +/* >>>> + * SPDX-License-Identifier: GPL-2.0-or-later >>>> + * >>>> + * Copyright (C) 2026, Florian Hofhammer <[email protected]> >>>> + */ >>>> +#include <assert.h> >>>> +#include <glib.h> >>>> +#include <inttypes.h> >>>> +#include <unistd.h> >>>> + >>>> +#include <qemu-plugin.h> >>>> + >>>> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION; >>>> + >>>> +static uint64_t source_pc; >>>> +static uint64_t target_pc; >>>> +static uint64_t target_vaddr; >>>> + >>>> +static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index, >>>> + int64_t num, uint64_t a1, uint64_t a2, >>>> + uint64_t a3, uint64_t a4, uint64_t a5, >>>> + uint64_t a6, uint64_t a7, uint64_t a8) >>>> +{ >>>> + if (num == 4096) { >>>> + qemu_plugin_outs("Marker syscall detected, jump to clean >>>> return\n"); >>>> + qemu_plugin_set_pc(a1); >>>> + } >>>> +} >>>> + >>>> +static bool vcpu_syscall_filter(qemu_plugin_id_t id, unsigned int >>>> vcpu_index, >>>> + int64_t num, uint64_t a1, uint64_t a2, >>>> + uint64_t a3, uint64_t a4, uint64_t a5, >>>> + uint64_t a6, uint64_t a7, uint64_t a8, >>>> + uint64_t *sysret) >>>> +{ >>>> + if (num == 4095) { >>>> + qemu_plugin_outs("Communication syscall detected, set target_pc / >>>> " >>>> + "target_vaddr\n"); >>>> + source_pc = a1; >>>> + target_pc = a2; >>>> + target_vaddr = a3; >>>> + if (source_pc >> 63 || target_pc >> 63 || target_vaddr >> 63) { >>>> + /* >>>> + * Some architectures (e.g., m68k) use 32-bit addresses with >>>> the >>>> + * top bit set, which causes them to get sign-extended >>>> somewhere in >>>> + * the chain to this callback. We mask the top bits off here >>>> to get >>>> + * the actual addresses. >>>> + */ >>> >>> This looks like an actual bug. Using a backtrace here, can you find which >>> function extended it? >>> Parameter should be treated as unsigned everywhere, else something is >>> really going wrong. >> >> tl;dr: register values are considered signed in the syscall handling >> code, which seems incorrect to me. Details below. >> >> This seems to be a bigger issue in the syscall handling code and I'm >> surprised this never surfaced before. Generally, the CPUArchState struct >> in target/*/cpu.h defines the registers as unsigned types. When a >> syscall is encountered, the main cpu loop calls do_syscall() from >> linux-user/syscall.c, which takes and returns abi_long values. abi_long >> is defined as either int32_t or target_long in include/user/abitypes.h >> and therefore a signed type, so the register values get converted from >> unsigned to signed types here. do_syscall() passes those abi_long values >> on, e.g., to record_syscall_start() and send_through_syscall_filters(), >> which still take the register values in as abi_long. Those in turn >> however pass the values on to qemu_plugin_vcpu_syscall() and >> qemu_plugin_vcpu_syscall_filter(), respectively. Those two functions now >> take uint64_t values for the arguments, so for 32-bit architectures the >> conversion chain is uint32_t (CPUArchState) -> int32_t (do_syscall) -> >> uint64_t (callbacks). >> >> This seems to be some bigger refactoring changing all those abi_longs to >> abi_ulongs. Would you like me to prepare a separate patch series for >> this? > > As a follow-up on this, changing this seems to be a bit more complicated > than I initially thought. The arguments could be simply made unsigned, > but the return value (which is also just a register content and should > therefore be unsigned if I understand correctly) can't easily be made > unsigned. The per-target main loop relies on the return value of > do_syscall() to be signed to determine whether one of the QEMU-specific > errors (e.g., QEMU_ERESTARTSYS) was returned.
Note: some architectures already define the return value as unsigned in
the main cpu loop and implicitly cast the return value of do_syscall().
Given that in theory, a syscall's return value could always coincide
with the values of QEMU's special errors, I'm wondering whether it would
make sense to make do_syscall()'s return value unsigned and signal those
special error codes out-of-band instead of in the syscall return value,
e.g., either via a separate pointer argument to do_syscall() or by
making do_syscall() return a struct containing both the return value and
a potential error code.
But maybe I'm getting ahead of myself here, please let me know what you
think!
>
>>>
>>>> + qemu_plugin_outs("High bit in addresses detected: possible
>>>> sign "
>>>> + "extension in syscall, masking off top
>>>> bits\n");
>>>> + source_pc &= UINT32_MAX;
>>>> + target_pc &= UINT32_MAX;
>>>> + target_vaddr &= UINT32_MAX;
>>>> + }
>>>> + *sysret = 0;
>>>> + return true;
>>>> + }
>>>> + return false;
>>>> +}
>>>> +
>>>> +static void vcpu_insn_exec(unsigned int vcpu_index, void *userdata)
>>>> +{
>>>> + uint64_t vaddr = (uint64_t)userdata;
>>>> + if (vaddr == source_pc) {
>>>> + g_assert(target_pc != 0);
>>>> + g_assert(target_vaddr == 0);
>>>> +
>>>> + qemu_plugin_outs("Marker instruction detected, jump to clean
>>>> return\n");
>>>> + qemu_plugin_set_pc(target_pc);
>>>> + }
>>>> +}
>>>> +
>>>> +static void vcpu_mem_access(unsigned int vcpu_index,
>>>> + qemu_plugin_meminfo_t info,
>>>> + uint64_t vaddr, void *userdata)
>>>> +{
>>>> + if (vaddr != 0 && vaddr == target_vaddr) {
>>>> + g_assert(source_pc == 0);
>>>> + g_assert(target_pc != 0);
>>>> + qemu_plugin_mem_value val = qemu_plugin_mem_get_value(info);
>>>> + /* target_vaddr points to our volatile guard ==> should always be
>>>> 1 */
>>>> + g_assert(val.type == QEMU_PLUGIN_MEM_VALUE_U32);
>>>> + g_assert(val.data.u32 == 1);
>>>> +
>>>> + qemu_plugin_outs("Marker mem access detected, jump to clean
>>>> return\n");
>>>> + qemu_plugin_set_pc(target_pc);
>>>> + }
>>>
>>> Thanks for adding this case also!
>>> So you'll definitely need a read for this precise test, to be able to keep
>>> a load. If it would work with a local variable, that's good, else, try with
>>> a local static (eventually marked volatile if it helps), and in last case,
>>> use a global variable.
>>
>> Works fine with a local variable!
>>
>>>
>>>> +}
>>>> +
>>>> +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
>>>> +{
>>>> + size_t insns = qemu_plugin_tb_n_insns(tb);
>>>> + for (size_t i = 0; i < insns; i++) {
>>>> + struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
>>>> + uint64_t insn_vaddr = qemu_plugin_insn_vaddr(insn);
>>>> + /*
>>>> + * Note: we cannot only register the callbacks if the instruction
>>>> is
>>>> + * in one of the functions of interest, because symbol lookup for
>>>> + * filtering does not work for all architectures (e.g., ppc64).
>>>> + */
>>>
>>> Too sad :)
>>> It's not a problem to instrument all instructions though.
>>>
>>>> + qemu_plugin_register_vcpu_insn_exec_cb(insn, vcpu_insn_exec,
>>>> + QEMU_PLUGIN_CB_RW_REGS_PC,
>>>> + (void *)insn_vaddr);
>>>> + qemu_plugin_register_vcpu_mem_cb(insn, vcpu_mem_access,
>>>> + QEMU_PLUGIN_CB_RW_REGS_PC,
>>>> + QEMU_PLUGIN_MEM_R, NULL);
>>>> + }
>>>> +}
>>>> +
>>>> +
>>>> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
>>>> + const qemu_info_t *info,
>>>> + int argc, char **argv)
>>>> +{
>>>> +
>>>> + qemu_plugin_register_vcpu_syscall_cb(id, vcpu_syscall);
>>>> + qemu_plugin_register_vcpu_syscall_filter_cb(id, vcpu_syscall_filter);
>>>> + qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
>>>> + return 0;
>>>> +}
>>>>
>>>
>>> Thanks for the update.
>>> Test looks good, and minus the style nits reported, it looks quite ready.
>>> It would be nice if we could sort the signed parameter extension also to
>>> avoid a hack on this.
>>>
>>> Regards,
>>> Pierrick
>>
>> Best regards,
>> Florian
smime.p7s
Description: S/MIME Cryptographic Signature
