8000 ZJIT: x86: Fix panic writing 32-bit number with top bit set · github/ruby@59fad96 · GitHub
[go: up one dir, main page]
Skip to content

Commit 59fad96

Browse files
XrXrXrXr
committed
ZJIT: 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: 54edc93
1 parent e5c7f16 commit 59fad96

File tree

3 files changed

+34
-7
lines changed

3 files changed

+34
-7
lines changed

‎zjit/src/asm/x86_64/mod.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,7 +1024,10 @@ pub fn mov(cb: &mut CodeBlock, dst: X86Opnd, src: X86Opnd) {
10241024
}
10251025

10261026
let output_num_bits:u32 = if mem.num_bits > 32 { 32 } else { mem.num_bits.into() };
1027-
assert!(imm_num_bits(imm.value) <= (output_num_bits as u8));
1027+
assert!(
1028+
mem.num_bits < 64 || imm_num_bits(imm.value) <= (output_num_bits as u8),
1029+
"immediate value should be small enough to survive sign extension"
1030+
);
10281031
cb.write_int(imm.value as u64, output_num_bits);
10291032
},
10301033
// M + UImm
@@ -1039,7 +1042,10 @@ pub fn mov(cb: &mut CodeBlock, dst: X86Opnd, src: X86Opnd) {
10391042
}
10401043

10411044
let output_num_bits = if mem.num_bits > 32 { 32 } else { mem.num_bits.into() };
1042-
assert!(imm_num_bits(uimm.value as i64) <= (output_num_bits as u8));
1045+
assert!(
1046+
mem.num_bits < 64 || imm_num_bits(uimm.value as i64) <= (output_num_bits as u8),
1047+
"immediate value should be small enough to survive sign extension"
1048+
);
10431049
cb.write_int(uimm.value, output_num_bits);
10441050
},
10451051
// * + Imm/UImm

‎zjit/src/asm/x86_64/tests.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ fn test_mov() {
193193
check_bytes("48c7470801000000", |cb| mov(cb, mem_opnd(64, RDI, 8), imm_opnd(1)));
194194
//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?
195195
check_bytes("c7400411000000", |cb| mov(cb, mem_opnd(32, RAX, 4), imm_opnd(17)));
196+
check_bytes("c7400401000080", |cb| mov(cb, mem_opnd(32, RAX, 4), uimm_opnd(0x80000001)));
196197
check_bytes("41895814", |cb| mov(cb, mem_opnd(32, R8, 20), EBX));
197198
check_bytes("4d8913", |cb| mov(cb, mem_opnd(64, R11, 0), R10));
198199
check_bytes("48c742f8f4ffffff", |cb| mov(cb, mem_opnd(64, RDX, -8), imm_opnd(-12)));

‎zjit/src/backend/x86_64/mod.rs

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -298,19 +298,24 @@ impl Assembler
298298
let opnd1 = asm.load(*src);
299299
asm.mov(*dest, opnd1);
300300
},
301-
(Opnd::Mem(_), Opnd::UImm(value)) => {
302-
// 32-bit values will be sign-extended
303-
if imm_num_bits(*value as i64) > 32 {
301+
(Opnd::Mem(Mem { num_bits, .. }), Opnd::UImm(value)) => {
302+
// For 64 bit destinations, 32-bit values will be sign-extended
303+
if *num_bits == 64 && imm_num_bits(*value as i64) > 32 {
304304
let opnd1 = asm.load(*src);
305305
asm.mov(*dest, opnd1);
306306
} else {
307307
asm.mov(*dest, *src);
308308
}
309309
},
310-
(Opnd::Mem(_), Opnd::Imm(value)) => {
311-
if imm_num_bits(*value) > 32 {
310+
(Opnd::Mem(Mem { num_bits, .. }), Opnd::Imm(value)) => {
311+
// For 64 bit destinations, 32-bit values will be sign-extended
312+
if *num_bits == 64 && imm_num_bits(*value) > 32 {
312313
let opnd1 = asm.load(*src);
313314
asm.mov(*dest, opnd1);
315+
} else if uimm_num_bits(*value as u64) <= *num_bits {
316+
// If the bit string is short enough for the destination, use the unsigned representation.
317+
// Note that 64-bit and negative values are ruled out.
318+
asm.mov(*dest, Opnd::UImm(*value as u64));
314319
} else {
315320
asm.mov(*dest, *src);
316321
}
@@ -1294,4 +1299,19 @@ mod tests {
12941299
0x13: mov qword ptr [rbx], rax
12951300
"});
12961301
}
1302+
1303+
#[test]
1304+
fn test_mov_m32_imm32() {
1305+
let (mut asm, mut cb) = setup_asm();
1306+
1307+
let shape_opnd = Opnd::mem(32, C_RET_OPND, 0);
1308+
asm.mov(shape_opnd, Opnd::UImm(0x8000_0001));
1309+
asm.mov(shape_opnd, Opnd::Imm(0x8000_0001));
1310+
1311+
asm.compile_with_num_regs(&mut cb, 0);
1312+
assert_disasm!(cb, "c70001000080c70001000080", {"
1313+
0x0: mov dword ptr [rax], 0x80000001
1314+
0x6: mov dword ptr [rax], 0x80000001
1315+
"});
1316+
}
12971317
}

0 commit comments

Comments
 (0)
0