commit f025692c992c ("accel/tcg: Clear PAGE_WRITE before translation")
fixed cross-modifying code handling, but did not add a test. The
changed code was further improved recently [1], and I was not sure
whether these modifications were safe (spoiler: they were fine).

Add a test to make sure there are no regressions.

[1] https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg00034.html

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---

v3: https://lore.kernel.org/qemu-devel/20230316214711.318339-1-iii@linux.ibm.com/
v3 -> v4: Rebase.
          I've been looking into the .NET failures under QEMU again and
          what this test looks for was a suspect. It turned out to be
          https://gitlab.com/qemu-project/qemu/-/issues/2600
          but it would be good to have this test upstream to ensure
          there are no regressions.

v2: https://patchew.org/QEMU/20220905154944.1284289-1-iii@linux.ibm.com/
v2 -> v3: Resend with a trivial rebase.

v1: https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg00455.html
v1 -> v2: Fix tweaking the flags (Alex).
          Keep the custom build rule for now.

 tests/tcg/x86_64/Makefile.target        |  4 ++
 tests/tcg/x86_64/cross-modifying-code.c | 80 +++++++++++++++++++++++++
 2 files changed, 84 insertions(+)
 create mode 100644 tests/tcg/x86_64/cross-modifying-code.c

diff --git a/tests/tcg/x86_64/Makefile.target b/tests/tcg/x86_64/Makefile.target
index 783ab5b21ad..d6dff559c7d 100644
--- a/tests/tcg/x86_64/Makefile.target
+++ b/tests/tcg/x86_64/Makefile.target
@@ -17,6 +17,7 @@ X86_64_TESTS += cmpxchg
 X86_64_TESTS += adox
 X86_64_TESTS += test-1648
 X86_64_TESTS += test-2175
+X86_64_TESTS += cross-modifying-code
 TESTS=$(MULTIARCH_TESTS) $(X86_64_TESTS) test-x86_64
 else
 TESTS=$(MULTIARCH_TESTS)
@@ -27,6 +28,9 @@ adox: CFLAGS=-O2
 run-test-i386-ssse3: QEMU_OPTS += -cpu max
 run-plugin-test-i386-ssse3-%: QEMU_OPTS += -cpu max
 
+cross-modifying-code: CFLAGS+=-pthread
+cross-modifying-code: LDFLAGS+=-pthread
+
 test-x86_64: LDFLAGS+=-lm -lc
 test-x86_64: test-i386.c test-i386.h test-i386-shift.h test-i386-muldiv.h
 	$(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)
diff --git a/tests/tcg/x86_64/cross-modifying-code.c b/tests/tcg/x86_64/cross-modifying-code.c
new file mode 100644
index 00000000000..2704df6061c
--- /dev/null
+++ b/tests/tcg/x86_64/cross-modifying-code.c
@@ -0,0 +1,80 @@
+/*
+ * Test patching code, running in one thread, from another thread.
+ *
+ * Intel SDM calls this "cross-modifying code" and recommends a special
+ * sequence, which requires both threads to cooperate.
+ *
+ * Linux kernel uses a different sequence that does not require cooperation and
+ * involves patching the first byte with int3.
+ *
+ * Finally, there is user-mode software out there that simply uses atomics, and
+ * that seems to be good enough in practice. Test that QEMU has no problems
+ * with this as well.
+ */
+
+#include <assert.h>
+#include <pthread.h>
+#include <stdbool.h>
+#include <stdlib.h>
+
+void add1_or_nop(long *x);
+asm(".pushsection .rwx,\"awx\",@progbits\n"
+    ".globl add1_or_nop\n"
+    /* addq $0x1,(%rdi) */
+    "add1_or_nop: .byte 0x48, 0x83, 0x07, 0x01\n"
+    "ret\n"
+    ".popsection\n");
+
+#define THREAD_WAIT 0
+#define THREAD_PATCH 1
+#define THREAD_STOP 2
+
+static void *thread_func(void *arg)
+{
+    int val = 0x0026748d; /* nop */
+
+    while (true) {
+        switch (__atomic_load_n((int *)arg, __ATOMIC_SEQ_CST)) {
+        case THREAD_WAIT:
+            break;
+        case THREAD_PATCH:
+            val = __atomic_exchange_n((int *)&add1_or_nop, val,
+                                      __ATOMIC_SEQ_CST);
+            break;
+        case THREAD_STOP:
+            return NULL;
+        default:
+            assert(false);
+            __builtin_unreachable();
+        }
+    }
+}
+
+#define INITIAL 42
+#define COUNT 1000000
+
+int main(void)
+{
+    int command = THREAD_WAIT;
+    pthread_t thread;
+    long x = 0;
+    int err;
+    int i;
+
+    err = pthread_create(&thread, NULL, &thread_func, &command);
+    assert(err == 0);
+
+    __atomic_store_n(&command, THREAD_PATCH, __ATOMIC_SEQ_CST);
+    for (i = 0; i < COUNT; i++) {
+        add1_or_nop(&x);
+    }
+    __atomic_store_n(&command, THREAD_STOP, __ATOMIC_SEQ_CST);
+
+    err = pthread_join(thread, NULL);
+    assert(err == 0);
+
+    assert(x >= INITIAL);
+    assert(x <= INITIAL + COUNT);
+
+    return EXIT_SUCCESS;
+}
-- 
2.46.2

On 10/1/24 08:03, Ilya Leoshkevich wrote:
> commit f025692c992c ("accel/tcg: Clear PAGE_WRITE before translation")
> fixed cross-modifying code handling, but did not add a test. The
> changed code was further improved recently [1], and I was not sure
> whether these modifications were safe (spoiler: they were fine).
> 
> Add a test to make sure there are no regressions.
> 
> [1]https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg00034.html
> 
> Signed-off-by: Ilya Leoshkevich<iii@linux.ibm.com>
> ---
> 
> v3:https://lore.kernel.org/qemu-devel/20230316214711.318339-1-iii@linux.ibm.com/
> v3 -> v4: Rebase.
>            I've been looking into the .NET failures under QEMU again and
>            what this test looks for was a suspect. It turned out to be
>            https://gitlab.com/qemu-project/qemu/-/issues/2600
>            but it would be good to have this test upstream to ensure
>            there are no regressions.
> 
> v2:https://patchew.org/QEMU/20220905154944.1284289-1-iii@linux.ibm.com/
> v2 -> v3: Resend with a trivial rebase.
> 
> v1:https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg00455.html
> v1 -> v2: Fix tweaking the flags (Alex).
>            Keep the custom build rule for now.
> 
>   tests/tcg/x86_64/Makefile.target        |  4 ++
>   tests/tcg/x86_64/cross-modifying-code.c | 80 +++++++++++++++++++++++++
>   2 files changed, 84 insertions(+)
>   create mode 100644 tests/tcg/x86_64/cross-modifying-code.c

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~