From e5c7f1695e8cf774d073e7b103c1d9289cad56ee Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Tue, 10 Jun 2025 20:52:57 +0900 Subject: [PATCH] YJIT: x86: Fix panic writing 32-bit number with top bit set Previously, `asm.mov(m32, imm32)` panicked when `imm32 > 0x80000000`. It attempted to split imm32 into a register before doing the store, but then the register size didn't match the destination size. Instead of splitting, use the `MOV r/m32, imm32` form which works for all 32-bit values. Adjust asserts that assumed that all forms undergo sign extension, which is not true for this case. See: 54edc930f9f0a658da45cfcef46648d1b6f82467 --- yjit/src/asm/x86_64/mod.rs | 10 ++++++++-- yjit/src/asm/x86_64/tests.rs | 1 + yjit/src/backend/x86_64/mod.rs | 30 +++++++++++++++++++++++++----- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/yjit/src/asm/x86_64/mod.rs b/yjit/src/asm/x86_64/mod.rs index fbbfa714d8..0ef5e92117 100644 --- a/yjit/src/asm/x86_64/mod.rs +++ b/yjit/src/asm/x86_64/mod.rs @@ -1027,7 +1027,10 @@ pub fn mov(cb: &mut CodeBlock, dst: X86Opnd, src: X86Opnd) { } let output_num_bits:u32 = if mem.num_bits > 32 { 32 } else { mem.num_bits.into() }; - assert!(imm_num_bits(imm.value) <= (output_num_bits as u8)); + assert!( + mem.num_bits < 64 || imm_num_bits(imm.value) <= (output_num_bits as u8), + "immediate value should be small enough to survive sign extension" + ); cb.write_int(imm.value as u64, output_num_bits); }, // M + UImm @@ -1042,7 +1045,10 @@ pub fn mov(cb: &mut CodeBlock, dst: X86Opnd, src: X86Opnd) { } let output_num_bits = if mem.num_bits > 32 { 32 } else { mem.num_bits.into() }; - assert!(imm_num_bits(uimm.value as i64) <= (output_num_bits as u8)); + assert!( + mem.num_bits < 64 || imm_num_bits(uimm.value as i64) <= (output_num_bits as u8), + "immediate value should be small enough to survive sign extension" + ); cb.write_int(uimm.value, output_num_bits); }, // * + Imm/UImm diff --git a/yjit/src/asm/x86_64/tests.rs b/yjit/src/asm/x86_64/tests.rs index 5ae983270f..eefcbfd52e 100644 --- a/yjit/src/asm/x86_64/tests.rs +++ b/yjit/src/asm/x86_64/tests.rs @@ -193,6 +193,7 @@ fn test_mov() { check_bytes("48c7470801000000", |cb| mov(cb, mem_opnd(64, RDI, 8), imm_opnd(1))); //check_bytes("67c7400411000000", |cb| mov(cb, mem_opnd(32, EAX, 4), imm_opnd(0x34))); // We don't distinguish between EAX and RAX here - that's probably fine? check_bytes("c7400411000000", |cb| mov(cb, mem_opnd(32, RAX, 4), imm_opnd(17))); + check_bytes("c7400401000080", |cb| mov(cb, mem_opnd(32, RAX, 4), uimm_opnd(0x80000001))); check_bytes("41895814", |cb| mov(cb, mem_opnd(32, R8, 20), EBX)); check_bytes("4d8913", |cb| mov(cb, mem_opnd(64, R11, 0), R10)); check_bytes("48c742f8f4ffffff", |cb| mov(cb, mem_opnd(64, RDX, -8), imm_opnd(-12))); diff --git a/yjit/src/backend/x86_64/mod.rs b/yjit/src/backend/x86_64/mod.rs index c0d42e79e6..ef435bca7e 100644 --- a/yjit/src/backend/x86_64/mod.rs +++ b/yjit/src/backend/x86_64/mod.rs @@ -315,19 +315,24 @@ impl Assembler let opnd1 = asm.load(*src); asm.mov(*dest, opnd1); }, - (Opnd::Mem(_), Opnd::UImm(value)) => { - // 32-bit values will be sign-extended - if imm_num_bits(*value as i64) > 32 { + (Opnd::Mem(Mem { num_bits, .. }), Opnd::UImm(value)) => { + // For 64 bit destinations, 32-bit values will be sign-extended + if *num_bits == 64 && imm_num_bits(*value as i64) > 32 { let opnd1 = asm.load(*src); asm.mov(*dest, opnd1); } else { asm.mov(*dest, *src); } }, - (Opnd::Mem(_), Opnd::Imm(value)) => { - if imm_num_bits(*value) > 32 { + (Opnd::Mem(Mem { num_bits, .. }), Opnd::Imm(value)) => { + // For 64 bit destinations, 32-bit values will be sign-extended + if *num_bits == 64 && imm_num_bits(*value) > 32 { let opnd1 = asm.load(*src); asm.mov(*dest, opnd1); + } else if uimm_num_bits(*value as u64) <= *num_bits { + // If the bit string is short enough for the destination, use the unsigned representation. + // Note that 64-bit and negative values are ruled out. + asm.mov(*dest, Opnd::UImm(*value as u64)); } else { asm.mov(*dest, *src); } @@ -1317,4 +1322,19 @@ mod tests { 0x13: mov qword ptr [rbx], rax "}); } + + #[test] + fn test_mov_m32_imm32() { + let (mut asm, mut cb) = setup_asm(); + + let shape_opnd = Opnd::mem(32, C_RET_OPND, 0); + asm.mov(shape_opnd, Opnd::UImm(0x8000_0001)); + asm.mov(shape_opnd, Opnd::Imm(0x8000_0001)); + + asm.compile_with_num_regs(&mut cb, 0); + assert_disasm!(cb, "c70001000080c70001000080", {" + 0x0: mov dword ptr [rax], 0x80000001 + 0x6: mov dword ptr [rax], 0x80000001 + "}); + } }