Pr2349 rebased - #2375
Conversation
Update the embedded QEMU-derived runtime and target code to the 7.2.22 baseline while preserving Unicorn's reduced integration model. This brings in the QEMU 7.2 support code needed by the fork: softfloat/int128 and helper compatibility, TCG and translator glue, updated symbol-postfix headers, target feature masks, and generated binding constants. CPU-visible coverage was expanded across the supported targets: AArch64 SVE/SME/MTE/PMU and M-profile MVE, RISC-V RVV/RVH/bitmanip/crypto/Zfh/Sstc/PMP paths, s390x MIE/VE/Vector-FP/string/crypto paths, PPC POWER10 groups, MIPS public modes and MSA registers, M68K FPU/MSP/ColdFire behavior, SPARC register access, and x86 AVX/AVX2/FMA/BMI/VAES/VPCLMUL validation. The unit test suite was extended with focused regressions for the migrated behavior, and the local MSVC test tree has been validated with full parallel CTest.
Keep the aarch64 backend call emitter on the reduced tree's tcg_out_call pointer ABI so ubuntu-aarch64 builds compile again. Use concise CTest failure output in Build UC2 so parallel CI logs expose the failing test case instead of losing it inside verbose interleaved output.
Align host TCG direct jump patching with the QEMU 7.2 tc_ptr/jmp_rx/jmp_rw ABI, add missing non-goto_ptr host backend definitions, and fix PPC host TCG opcode/table drift. Move variable target page size state out of release-mode macros that depended on a local uc variable, using arch-postfixed target_page_bits_state instead. Fix POSIX MIPS CPU alignment and preserve microMIPS entry state, and avoid Apple JIT state asserts when virtualized macOS runners cannot report SPRR permissions.
Keep variable target page size state per Unicorn engine instead of arch-global state, while preserving the improved finalization path. Add a public control regression that runs two ARM engines with different page sizes in the same process.
Synchronize generated code writes with instruction fetches when patching TCG direct jumps and finalizing generated code. This keeps the reduced JIT path aligned with QEMU 7.2 host cache semantics on non-coherent host caches.
Align the reduced TCG generated-code pointers with the QEMU 7.2 RX/RW address model so host backends compute branch offsets, labels, prologue addresses, and jump patches against executable addresses while still writing through writable addresses. Use runtime MIPS guest endianness for unaligned store helpers and declare the PPC host cache flush helper used by reduced per-target builds.
Keep split-WX conversion inline in the reduced single-mapping TCG runtime so per-arch archives do not define duplicate data symbols during GNU ld links.
Pin the Zig macOS workflow to macos-14 so Zig 0.14 does not run on an unsupported macOS 26 image.
Use pthread JIT write-protection transitions directly and avoid private SPRR state probes on Apple Silicon hosts.
Use helper typemasks when extending TCG call arguments so pointer operands are not treated as 32-bit values on aarch64 hosts. Restore s390x instruction-start metadata emission for early Unicorn exit TBs.
Avoid freeing BF16 VCVT temporaries after neon_store_reg consumes them.
Restore QEMU 7.2 ppc modulo lowering and long goto_tb reset handling. Revalidate TLB entries after Unicorn memory callbacks that can flush or resize TLB state.
QEMU relies on signed arithmetic wrapping in a number of helpers. For example the MVE DO_ABD macro computes (M) - (N) on extremal int32_t operands, which overflows and is undefined behaviour. clang exploits that UB and returns the negated result, which made test_arm_m55_mve_vabd fail on the macOS runners while gcc happened to emit the wrapping subtraction and passed. Upstream QEMU's configure passes -fwrapv for the same reason, so do the same here.
|
Hi, @wtdcode and @Nitr0-G, thank you for the great work on this update. I noticed that this PR fixes the same issue as #2370 (ran the tests from that PR, they pass). Should that PR be closed? And should the tests in it be added as a separate PR? However, I also noticed that |
|
Hi @unknown-1-0 Thanks for your interest and for discovering our "secret" works =). This branch is currently not ready for merging and is just for testing and interactive review. But I do expect this to be included in |
| @@ -0,0 +1,62 @@ | |||
| /* | |||
| RAMBlock *block; | ||
| ram_addr_t offset; | ||
|
|
||
| host = allocation_tag_probe_access(env, clean_ptr, access_type, size, |
There was a problem hiding this comment.
This seems different from https://github.com/qemu/qemu/blob/v7.2.22/target/arm/mte_helper.c#L121
Looks like you forget to port CPUTLBEntryFull? Is that intended? Or are you sure the semantic is equivalent?
| tcg_ctx->cpu = NULL; | ||
|
|
||
| /* generate machine code */ | ||
| tb->jmp_reset_offset[0] = TB_JMP_RESET_OFFSET_INVALID; |
There was a problem hiding this comment.
We should also port https://github.com/qemu/qemu/blob/v7.2.22/accel/tcg/tb-maint.c
|
I used Claude to semi-manually review this PR and the above findings should be the most relevant. I will fix the others later. |
|
@PhilippTakacs Let me know if this port breaks your workflows. |
|
I have run some tests and get a The |
I found this out tonight too, can you check #2377 please? I think I fixed this problem and some other shit too |
|
With #2377 my test reaches |
Yes - qemu/qemu@958e1dd |
* port QEMU 7.2 TLB and TB maintenance Port full TLB entries and preserve translated ARM page attributes for MTE and BTI checks. Move translation block cache, linking, invalidation, and flush maintenance into tb-maint.c while keeping Unicorn per-engine state and SMC behavior. Add guarded-page, self-linked TB, invalidation, and engine-isolation coverage. * fix QEMU 7.2.22 partial ports
|
I found the problem, I had a not correct aligned the stack pointer. |
|
I'm currently working on some improvements related to the tlb (TLB_NOTDIRTY, call all memory hooks, ...). I wounder if I it's better to target this branch to avoid implementing it two times. |
I think yes. @wtdcode |
|
I have run some tests with our fuzzer/emulator (https://github.com/mogikai/mogi) and found a ~10% time lost using this branch only by running the 3mm benchmark without fuzzig. A quick look at the perf data shows most time is lost in the load_help. I assume either the fast load (avoiding the |
I'll test this and try to fix it! Thanks for telling! |
There are a small set of binary SSE insns which have no MMX equivalent, which we create the gen functions for with the BINARY_INT_SSE() macro. This forwards to gen_binary_int_sse() with a NULL pointer for 'mmx'. For almost all of these insns we correctly mark them in the decode table as not permitting a zero prefix byte; however we got this wrong for VPERMILPS, with the result that a bogus instruction would get through the decode checks and end up in gen_binary_int_sse() trying to call a NULL pointer. Correct the decode table entry for VPERMILPS so that we get the expected #UD exception. In the x86 SDM, table A-4 "Three-byte Opcode Map: 08H-FFH (First Two Bytes are 0F 38H)" confirms that there is no pfx 0 version of VPERMILPS. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3199 Andrey Polivoda: Added a unit test Upstream QEMU commit: ebd9ea2947d88f237e20333fe547ca8817d0b0ee Signed-off-by: Andrey Polivoda <apolivodaa433@gmail.com>
…ll` (#2406) The original variant of that test passed an incorrectly calculated size parameter, which led to 0F 38 0C 00 being executed instead of 0F 38 0C FF. This commit fixes that and explicitly tests both variants of the instruction. Signed-off-by: Andrey Polivoda <apolivodaa433@gmail.com>
Hello! Sorry for bothering u, but can u retest it with pr2349-rebased branch in my fork (https://github.com/Nitr0-G/unicorn/tree/pr2349-rebased)? It seems i fixed it |
|
this doesn't bother me at all. Shall I directly test against your pr2349-rebased-test-asan branch? |
Directly test pr2349-rebased. pr2349-rebased-test-asan it's just a test branch for libfuzzer/asan and etc that work in ci\cd on pull requests. I was tired and accidentally made a mistake when I was doing the PR, and because of that I made it into this repository(#2409), and not my local fork, sorry |
a small benchmark from my work |
|
I have run the 3mm benchmark again and it looks pretty good: about 9 min 14 sec on dev and 1 min 45 sec on your pr2349-rebased branch. |
|
Cool, then I'm going to get this merged soon. |
|
Looks like the upstream introduced a new crash bug diff --git a/tests/unit/test_x86.c b/tests/unit/test_x86.c
index ece99098..98409b61 100644
--- a/tests/unit/test_x86.c
+++ b/tests/unit/test_x86.c
@@ -3558,6 +3558,21 @@ static void test_x86_vpermilps_null_ptr_call(void)
OK(uc_close(uc));
}
+static void test_x86_pinsrw_assert_failure(void)
+{
+ char code[] = {
+ 0xc5, 0xf8, 0xc4, 0x00, 0x00
+ };
+
+ uc_engine *uc = NULL;
+ uc_common_setup(&uc, UC_ARCH_X86, UC_MODE_64, code, sizeof(code));
+
+ uc_assert_err(UC_ERR_INSN_INVALID,
+ uc_emu_start(uc, code_start, code_start + sizeof(code), 0, 0));
+
+ OK(uc_close(uc));
+}
+
static bool test_x86_hook_insn_rdtsc_cb(uc_engine *uc, void *user_data)
{
uint64_t h = 0x00000000FEDCBA98;
@@ -4094,6 +4109,7 @@ TEST_LIST = {
{"test_rex_x64", test_rex_x64},
{"test_x86_ro_segfault", test_x86_ro_segfault},
{"test_x86_vpermilps_null_ptr_call", test_x86_vpermilps_null_ptr_call},
+ {"test_x86_pinsrw_assert_failure", test_x86_pinsrw_assert_failure},
{"test_x86_hook_insn_rdtsc", test_x86_hook_insn_rdtsc},
{"test_x86_hook_insn_rdtscp", test_x86_hook_insn_rdtscp},
{"test_x86_hook_insn_wrmsr", test_x86_hook_insn_wrmsr},This same test results in i3-6100 raises a Note: upstream QEMU is affected too |
|
And another one: diff --git a/tests/unit/test_x86.c b/tests/unit/test_x86.c
index 98409b61..ed7b7150 100644
--- a/tests/unit/test_x86.c
+++ b/tests/unit/test_x86.c
@@ -3573,6 +3573,38 @@ static void test_x86_pinsrw_assert_failure(void)
OK(uc_close(uc));
}
+static void test_x86_vroundss_assert_failure(void)
+{
+ char code[] = {
+ 0xc4, 0xe3, 0x7d, 0x0a, 0xc0, 0x00
+ };
+
+ uc_engine *uc = NULL;
+ uc_common_setup(&uc, UC_ARCH_X86, UC_MODE_64, code, sizeof(code));
+
+ // Not sure whether this should raise UC_ERR_INSN_INVALID
+ // i3-6100 executes it without SIGILL at least
+ OK(uc_emu_start(uc, code_start, code_start + sizeof(code), 0, 0));
+
+ OK(uc_close(uc));
+}
+
+static void test_x86_vroundsd_assert_failure(void)
+{
+ char code[] = {
+ 0xc4, 0xe3, 0x7d, 0x0b, 0xc0, 0x00
+ };
+
+ uc_engine *uc = NULL;
+ uc_common_setup(&uc, UC_ARCH_X86, UC_MODE_64, code, sizeof(code));
+
+ // Not sure whether this should raise UC_ERR_INSN_INVALID
+ // i3-6100 executes it without SIGILL at least
+ OK(uc_emu_start(uc, code_start, code_start + sizeof(code), 0, 0));
+
+ OK(uc_close(uc));
+}
+
static bool test_x86_hook_insn_rdtsc_cb(uc_engine *uc, void *user_data)
{
uint64_t h = 0x00000000FEDCBA98;
@@ -4110,6 +4142,8 @@ TEST_LIST = {
{"test_x86_ro_segfault", test_x86_ro_segfault},
{"test_x86_vpermilps_null_ptr_call", test_x86_vpermilps_null_ptr_call},
{"test_x86_pinsrw_assert_failure", test_x86_pinsrw_assert_failure},
+ {"test_x86_vroundss_assert_failure", test_x86_vroundss_assert_failure},
+ {"test_x86_vroundsd_assert_failure", test_x86_vroundsd_assert_failure},
{"test_x86_hook_insn_rdtsc", test_x86_hook_insn_rdtsc},
{"test_x86_hook_insn_rdtscp", test_x86_hook_insn_rdtscp},
{"test_x86_hook_insn_wrmsr", test_x86_hook_insn_wrmsr},On this PR: On current Upstream QEMU is affected by this too: |
|
I have looked at |
|
For VROUNDSS/VROUNDSD, the assertion removal may work (however, I did not test what changes in the hardware behavior when VEX.L = 1 is set, and haven't looked whether QEMU does not rely on For VPINSRW, the issue seems to be the decoder allowing it to be encoded with a VEX prefix with The entry in the Intel SDM looks like this: Chapter 3 of the Intel SDM says this:
So that means that VEX.pp should be 1 (since it maps to 0x66) when the VEX prefix is used with PINSRW. For other asserts: Since Unicorn is a library, I think it would be a better idea to keep these (since they are placed for a reason) and make these crash the process on debug builds, and make |
|
Why the invalid opcode ( |
|
To bad, I though this could be easy to fix by replace the assert() with a self provided macro.
Of course they are there for a reason and should not removed without a replacement. I would say replace them with a macro which causes uc_* to return UC_ERR_INTERNAL even in a debug build. Just keeping the assert might cause an abort() call without the possibility for the user to handle this error. |
That makes sense. I think we can also let the user decide what should happen when an assertion failure occurs (via |
|
Intel Xeon Platinum 8370C (from 2-core instance of GitHub Codespaces) seems to ignore VEX.L with VROUNDSS/VROUNDSD AMD EPYC 7763 (from 4-core instance): Same result |
|
It looks like Volume 4 of AMD64 Architecture Programmer's Manual explicitly documents that VEX.L is ignored for both VROUNDSS and VROUNDSD Edit: Intel SDM documents it too: |
|
Patch for upstream QEMU (in case the upstream maintainers find some issues in it): https://lore.kernel.org/qemu-devel/20260905014629.991586-1-apolivodaa433@gmail.com/T/#u |
4 participants
#2349 Upgrade our QEMU fork to 7.1.22.
We will finally chase up to the upstream QEMU.