From 9dd24c9b57cde064ca8b356c985b2e1cb7972adc Mon Sep 17 00:00:00 2001 From: Ivan Tetyushkin Date: Mon, 21 Apr 2025 11:59:48 +0300 Subject: [PATCH] riscv64: Fix nan-boxing for single-precision calculations For float values, for arithmetics we expect to have canonical nan if used double register is not currectly nan-boxed. https://bugs.kde.org/show_bug.cgi?id=503098 --- NEWS | 1 + VEX/priv/guest_riscv64_toIR.c | 28 ++++++++++++++++++++------- none/tests/riscv64/float32.c | 6 ++++++ none/tests/riscv64/float32.stdout.exp | 3 +++ 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/NEWS b/NEWS index 1ced90cae..460eb5656 100644 --- a/NEWS +++ b/NEWS @@ -23,6 +23,7 @@ bugzilla (https://bugs.kde.org/enter_bug.cgi?product=valgrind) rather than mailing the developers (or mailing lists) directly -- bugs that are not entered into bugzilla tend to get forgotten about or ignored. +503098 Incorrect NAN-boxing for float registers in RISC-V 503641 close_range syscalls started failing with 3.25.0 503677 duplicated-cond compiler warning in dis_RV64M 503817 s390x: fix 'ordered comparison of pointer with integer zero' compiler warnings diff --git a/VEX/priv/guest_riscv64_toIR.c b/VEX/priv/guest_riscv64_toIR.c index 393a7ca7d..59297acd1 100644 --- a/VEX/priv/guest_riscv64_toIR.c +++ b/VEX/priv/guest_riscv64_toIR.c @@ -522,9 +522,13 @@ static IRExpr* getFReg32(UInt fregNo) vassert(fregNo < 32); /* Note that the following access depends on the host being little-endian which is checked in disInstr_RISCV64(). */ - /* TODO Check that the value is correctly NaN-boxed. If not then return - the 32-bit canonical qNaN, as mandated by the RISC-V ISA. */ - return IRExpr_Get(offsetFReg(fregNo), Ity_F32); + IRExpr* f64 = getFReg64(fregNo); + IRExpr* high_half = unop(Iop_64HIto32, unop(Iop_ReinterpF64asI64, f64)); + IRExpr* cond = binop(Iop_CmpEQ32, high_half, mkU32(0xffffffff)); + IRExpr* res = IRExpr_ITE( + cond, IRExpr_Get(offsetFReg(fregNo), Ity_F32), + /* canonical nan */ unop(Iop_ReinterpI32asF32, mkU32(0x7fc00000))); + return res; } /* Write a 32-bit value into a guest floating-point register. */ @@ -2160,8 +2164,10 @@ static Bool dis_RV64F(/*MB_OUT*/ DisResult* dres, UInt rs2 = INSN(24, 20); UInt imm11_0 = INSN(31, 25) << 5 | INSN(11, 7); ULong simm = vex_sx_to_64(imm11_0, 12); - storeLE(irsb, binop(Iop_Add64, getIReg64(rs1), mkU64(simm)), - getFReg32(rs2)); + // do not modify the bits being transferred; + IRExpr* f64 = getFReg64(rs2); + IRExpr* i32 = unop(Iop_64to32, unop(Iop_ReinterpF64asI64, f64)); + storeLE(irsb, binop(Iop_Add64, getIReg64(rs1), mkU64(simm)), i32); DIP("fsw %s, %lld(%s)\n", nameFReg(rs2), (Long)simm, nameIReg(rs1)); return True; } @@ -2456,8 +2462,16 @@ static Bool dis_RV64F(/*MB_OUT*/ DisResult* dres, INSN(24, 20) == 0b00000 && INSN(31, 25) == 0b1110000) { UInt rd = INSN(11, 7); UInt rs1 = INSN(19, 15); - if (rd != 0) - putIReg32(irsb, rd, unop(Iop_ReinterpF32asI32, getFReg32(rs1))); + if (rd != 0) { + // For RV64, the higher 32 bits of the destination register are filled + // with copies of the floating-point number’s sign bit. + IRExpr* freg = getFReg64(rs1); + IRExpr* low_half = unop(Iop_64to32, unop(Iop_ReinterpF64asI64, freg)); + IRExpr* sign = binop(Iop_And32, low_half, mkU32(1u << 31)); + IRExpr* cond = binop(Iop_CmpEQ32, sign, mkU32(1u << 31)); + IRExpr* high_part = IRExpr_ITE(cond, mkU32(0xffffffff), mkU32(0)); + putIReg64(irsb, rd, binop(Iop_32HLto64, high_part, low_half)); + } DIP("fmv.x.w %s, %s\n", nameIReg(rd), nameFReg(rs1)); return True; } diff --git a/none/tests/riscv64/float32.c b/none/tests/riscv64/float32.c index b63305a64..f635bc761 100644 --- a/none/tests/riscv64/float32.c +++ b/none/tests/riscv64/float32.c @@ -1578,6 +1578,12 @@ static void test_float32_additions(void) TESTINST_1_1_FI(4, "fcvt.s.lu fa0, a0", 0x0000000001000001, 0x60, fa0, a0); /* 2**24+1 (DYN-RMM) -> 2**24+2 (NX) */ TESTINST_1_1_FI(4, "fcvt.s.lu fa0, a0", 0x0000000001000001, 0x80, fa0, a0); + + // check nan-boxing + /* fabs.s rd, rs1 */ + TESTINST_1_2_F(4, "fsgnjx.s fa0, fa1, fa1", 0xfaffffff3f800000, + 0xfaffffff3f800000, 0x00, fa0, fa1, fa1); + } int main(void) diff --git a/none/tests/riscv64/float32.stdout.exp b/none/tests/riscv64/float32.stdout.exp index 013c7eda2..734370d51 100644 --- a/none/tests/riscv64/float32.stdout.exp +++ b/none/tests/riscv64/float32.stdout.exp @@ -1554,3 +1554,6 @@ fcvt.s.lu fa0, a0 :: fcvt.s.lu fa0, a0 :: inputs: a0=0x0000000001000001, fcsr=0x00000080 output: fa0=0xffffffff4b800001, fcsr=0x00000081 +fsgnjx.s fa0, fa1, fa1 :: + inputs: fa1=0xfaffffff3f800000, fa1=0xfaffffff3f800000, fcsr=0x00000000 + output: fa0=0xffffffff7fc00000, fcsr=0x00000000 -- 2.39.5