Re: Re: [PATCH] accel/tcg: fix self-modify-code problem when modify code in a single tb loop

Re: Re: [PATCH] accel/tcg: fix self-modify-code problem when modify code in a single tb loop

[李威威]

[-- Attachment #1: Type: text/plain, Size: 1643 bytes --]

Richard Henderson <richard.henderson@linaro.org&gt; 于2025年9月23日周二 07:22写道：
&gt;
&gt; On 9/17/25 05:47, liweiwei@kubuds.cn wrote:
&gt; &gt; From: Weiwei Li <liweiwei@kubuds.cn&gt;
&gt; &gt;
&gt; &gt; The problem is triggered in following conditions:
&gt; &gt; - thread 1:
&gt; &gt; &nbsp; &nbsp; &nbsp;run spin loop(ended with a direct jump) like "0x0000006f, // jal zero, #0"
&gt; &gt; - thread 2:
&gt; &gt; &nbsp; &nbsp; &nbsp;do something, and then modify the loop code of thread 1 to nop isntruction,
&gt; &gt; &nbsp; &nbsp; &nbsp;finally wait thread 1 exit.
&gt; &gt;
&gt; &gt; The loop tb which is patched to jump to itself, will not be updated in this case
&gt; &gt; and will never exit.
&gt; &gt;
&gt; &gt; Signed-off-by: Weiwei Li <liweiwei@kubuds.cn&gt;
&gt; &gt; ---
&gt; &gt; &nbsp; accel/tcg/cpu-exec.c | 8 ++++++--
&gt; &gt; &nbsp; 1 file changed, 6 insertions(+), 2 deletions(-)
&gt;
&gt; If there's a problem with 1 tb, there's also a problem with 2 tb like
&gt;
&gt; &nbsp; &nbsp; &nbsp; &nbsp; jal &nbsp; &nbsp; zero, #4
&gt; &nbsp; &nbsp; &nbsp; &nbsp; jal &nbsp; &nbsp; zero, #-4
&gt;

I tried this case. And it didn't have this problem.
This problem seems only existed in single tb loop.
&gt;
&gt; But unlinking the tb should be part of invalidation, so I don't quite see where the
&gt; problem is. &nbsp;You need to expand on the description of the problem.
&gt;


I think the problem is the single tb is always in use&nbsp;&nbsp;when the single tb is linked with itself，
and it cannot be updated when we update the code。


Regards，
Weiwei Li

&gt;
&gt; r~

[-- Attachment #2: Type: text/html, Size: 2376 bytes --]

Re: Re: [PATCH] accel/tcg: fix self-modify-code problem when modify code in a single tb loop

[李威威]

[-- Attachment #1: Type: text/plain, Size: 6691 bytes --]

Richard Henderson <richard.henderson@linaro.org&gt; 于2025年9月23日周二 10:10写道：
&gt;
&gt; On 9/22/25 19:04, 李威威 wrote:
&gt; &gt; &nbsp;&gt; If there's a problem with 1 tb, there's also a problem with 2 tb like
&gt; &gt; &nbsp;&gt;
&gt; &gt; &nbsp;&gt; &nbsp; &nbsp; &nbsp; &nbsp; jal &nbsp; &nbsp; zero, #4
&gt; &gt; &nbsp;&gt; &nbsp; &nbsp; &nbsp; &nbsp; jal &nbsp; &nbsp; zero, #-4
&gt; &gt; &nbsp;&gt;
&gt; &gt;
&gt; &gt; I tried this case. And it didn't have this problem.
&gt; &gt; This problem seems only existed in single tb loop.
&gt; &gt;
&gt; &gt; &nbsp;&gt;
&gt; &gt; &nbsp;&gt; But unlinking the tb should be part of invalidation, so I don't quite see where the
&gt; &gt; &nbsp;&gt; problem is. &nbsp;You need to expand on the description of the problem.
&gt; &gt; &nbsp;&gt;
&gt; &gt;
&gt; &gt; I think the problem is the single tb is always in use &nbsp;when the single tb is linked with
&gt; &gt; itself，
&gt; &gt; and it cannot be updated when we update the code。
&gt;
&gt; There's no use count for tb's, so that explanation doesn't make sense.
&gt; Can you please share a testcase for this?

I think the problem is here:
/*
&nbsp;* Invalidate all TBs which intersect with the target address range.
&nbsp;* Called with mmap_lock held for user-mode emulation.
&nbsp;* NOTE: this function must not be called while a TB is running.
&nbsp;*/
void tb_invalidate_phys_range(CPUState *cpu, tb_page_addr_t start,
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; tb_page_addr_t last)
{
&nbsp; &nbsp; TranslationBlock *tb;
&nbsp; &nbsp; PageForEachNext n;

&nbsp; &nbsp; assert_memory_lock();

&nbsp; &nbsp; PAGE_FOR_EACH_TB(start, last, unused, tb, n) {
&nbsp; &nbsp; &nbsp; &nbsp; tb_phys_invalidate__locked(tb);
&nbsp; &nbsp; }
}

Only un-running tb will be invalidated.

This problem is found after a recent change in V8. Following is an example built by Kasper Land:

#include <stdio.h&gt;
#include <stdlib.h&gt;
#include <string.h&gt;
#include <sys/mman.h&gt; // For mmap, mprotect, munmap.
#include <pthread.h&gt; &nbsp;// For pthread functions.
#include <stdint.h&gt; &nbsp; // For intptr_t, uint32_t.
#include <unistd.h&gt; &nbsp; // For sleep.

// This program is specific to the RISC-V architecture.
#if !defined(__riscv)
#error "This code is intended for RISC-V architectures only."
#endif

// Define a function pointer type that takes no arguments and returns an int.
typedef int (*jit_func)();

/**
&nbsp;* @brief The function that will be executed by the new thread.
&nbsp;* * @param arg A pointer to the JIT-compiled function.
&nbsp;* @return void* The integer result from the JIT function, cast to a void pointer.
&nbsp;*/
void* thread_runner(void* arg) {
&nbsp; &nbsp; // Cast the argument back to our function pointer type.
&nbsp; &nbsp; jit_func func = (jit_func)arg;

&nbsp; &nbsp; printf(" &nbsp;[Thread] Executing JIT'ed code...\n");
&nbsp; &nbsp; int result = func();
&nbsp; &nbsp; printf(" &nbsp;[Thread] JIT'ed code returned: %d\n", result);

&nbsp; &nbsp; // To return a simple integer value from a thread, it's safe to cast it
&nbsp; &nbsp; // through intptr_t, which is an integer type guaranteed to be able to hold a pointer.
&nbsp; &nbsp; return (void*)(intptr_t)result;
}

int main() {
&nbsp; &nbsp; // ## 1. RISC-V machine code.
&nbsp; &nbsp; // Assembly:
&nbsp; &nbsp; // &nbsp; L: j L &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;; Jump to self (spin).
&nbsp; &nbsp; // &nbsp; li a0, 42 &nbsp; &nbsp; &nbsp; ; Place 42 into the return value register a0.
&nbsp; &nbsp; // &nbsp; ret &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; ; Return to caller.
&nbsp; &nbsp; uint32_t machine_code[] = {
&nbsp; &nbsp; &nbsp; &nbsp; 0x0000006f, // jal zero, #0
&nbsp; &nbsp; &nbsp; &nbsp; 0x02a00513, // addi a0, zero, 42
&nbsp; &nbsp; &nbsp; &nbsp; 0x00008067 &nbsp;// jalr zero, ra, 0
&nbsp; &nbsp; };
&nbsp; &nbsp; size_t code_size = sizeof(machine_code);

&nbsp; &nbsp; // ## 2. Allocate executable memory.
&nbsp; &nbsp; void* buffer = mmap(
&nbsp; &nbsp; &nbsp; &nbsp; NULL,
&nbsp; &nbsp; &nbsp; &nbsp; code_size,
&nbsp; &nbsp; &nbsp; &nbsp; PROT_READ | PROT_WRITE | PROT_EXEC,
&nbsp; &nbsp; &nbsp; &nbsp; MAP_PRIVATE | MAP_ANONYMOUS,
&nbsp; &nbsp; &nbsp; &nbsp; -1, 0
&nbsp; &nbsp; );

&nbsp; &nbsp; if (buffer == MAP_FAILED) {
&nbsp; &nbsp; &nbsp; &nbsp; perror("mmap failed");
&nbsp; &nbsp; &nbsp; &nbsp; return 1;
&nbsp; &nbsp; }

&nbsp; &nbsp; // ## 3. Copy machine code into buffer.
&nbsp; &nbsp; memcpy(buffer, machine_code, code_size);
&nbsp; &nbsp; printf("[Main] Successfully generated machine code at address: %p\n", buffer);

&nbsp; &nbsp; // ## 4. Execute the code in a separate thread.
&nbsp; &nbsp; pthread_t thread_id;
&nbsp; &nbsp; void* thread_return_value;

&nbsp; &nbsp; // The JIT'ed code is now ready to be run.
&nbsp; &nbsp; jit_func func_to_run = (jit_func)buffer;

&nbsp; &nbsp; printf("[Main] Creating a new thread to run the JIT code...\n");
&nbsp; &nbsp; if (pthread_create(&amp;thread_id, NULL, thread_runner, func_to_run) != 0) {
&nbsp; &nbsp; &nbsp; &nbsp; perror("pthread_create failed");
&nbsp; &nbsp; &nbsp; &nbsp; munmap(buffer, code_size);
&nbsp; &nbsp; &nbsp; &nbsp; return 1;
&nbsp; &nbsp; }

&nbsp; &nbsp; // Wait a second and then try to patch the generated code
&nbsp; &nbsp; // to get the runner thread to get unstuck by patching the
&nbsp; &nbsp; // spin jump.
&nbsp; &nbsp; sleep(1);
&nbsp; &nbsp; printf("[Main] Patching spin jump to nop...\n");
&nbsp; &nbsp; uint32_t* patchable = (uint32_t*) buffer;
&nbsp; &nbsp; patchable[0] = 0x00000013; &nbsp;// nop

&nbsp; &nbsp; printf("[Main] Flush icache...\n");
&nbsp; &nbsp; __builtin___clear_cache((char*) patchable, (char*) &amp;patchable[1]);

&nbsp; &nbsp; printf("[Main] Waiting for the thread to finish...\n");
&nbsp; &nbsp; if (pthread_join(thread_id, &amp;thread_return_value) != 0) {
&nbsp; &nbsp; &nbsp; &nbsp; perror("pthread_join failed");
&nbsp; &nbsp; &nbsp; &nbsp; munmap(buffer, code_size);
&nbsp; &nbsp; &nbsp; &nbsp; return 1;
&nbsp; &nbsp; }

&nbsp; &nbsp; int result = (int)(intptr_t)thread_return_value;
&nbsp; &nbsp; printf("[Main] The JIT'ed function (run in a thread) returned: %d\n", result);
&nbsp; &nbsp; printf("[Main] Execution successful!\n");

&nbsp; &nbsp; // ## 6. Clean Up.
&nbsp; &nbsp; munmap(buffer, code_size);

&nbsp; &nbsp; return 0;
}



&gt;
&gt; For extra kudos, a small assembly test for tests/tcg/loongarch64/system/, so that we have
&gt; a regression test for the issue. &nbsp;:-)
&gt;
&gt;
&gt; r~

[-- Attachment #2: Type: text/html, Size: 7453 bytes --]