diff mbox series

sim: riscv: Fix undefined behaviour in mulh and similar

Message ID AS8P193MB1285843C26CFAD73EC0FAEBEE4162@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM
State New
Headers
Series sim: riscv: Fix undefined behaviour in mulh and similar |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Bernd Edlinger April 26, 2024, 7:50 a.m. UTC 
  This fixes an undefined integer overflow bug in the mulh function,
which caused a test failure in the gcc test suite for riscv64 target:

FAIL: gcc.dg/ftrapv-3.c execution test

Fix that by casting to unsigned when an overflow is possible.

Also the __int128 multiplication in mulhu can overflow, and must be
done with unsigned __int128 to be safe.

And of course, the sign change in mulhsu can overflow too.

And also in execute_m there are a couple of possibly overflowing
multiplications in MULW, MULH and MULHSU.  The latter is probably
harmless, because the signed * unsigned type multiplication is done
in unsigned, but it is at least clearer what is intended, this way.
---
 sim/riscv/sim-main.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
diff mbox series

Patch

diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
index f6f6e2384e8..1815d7f2a6c 100644
--- a/sim/riscv/sim-main.c
+++ b/sim/riscv/sim-main.c
@@ -651,7 +651,7 @@  static uint64_t
 mulhu (uint64_t a, uint64_t b)
 {
 #ifdef HAVE___INT128
-  return ((__int128)a * b) >> 64;
+  return ((unsigned __int128)a * b) >> 64;
 #else
   uint64_t t;
   uint32_t y1, y2, y3;
@@ -677,16 +677,16 @@  static uint64_t
 mulh (int64_t a, int64_t b)
 {
   int negate = (a < 0) != (b < 0);
-  uint64_t res = mulhu (a < 0 ? -a : a, b < 0 ? -b : b);
-  return negate ? ~res + (a * b == 0) : res;
+  uint64_t res = mulhu (a < 0 ? -(uint64_t)a : a, b < 0 ? -(uint64_t)b : b);
+  return negate ? ~res + ((uint64_t)a * (uint64_t)b == 0) : res;
 }
 
 static uint64_t
 mulhsu (int64_t a, uint64_t b)
 {
   int negate = a < 0;
-  uint64_t res = mulhu (a < 0 ? -a : a, b);
-  return negate ? ~res + (a * b == 0) : res;
+  uint64_t res = mulhu (a < 0 ? -(uint64_t)a : a, b);
+  return negate ? ~res + ((uint64_t)a * b == 0) : res;
 }
 
 static sim_cia
@@ -757,16 +757,16 @@  execute_m (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
       TRACE_INSN (cpu, "mulw %s, %s, %s;  // %s = %s * %s",
 		  rd_name, rs1_name, rs2_name, rd_name, rs1_name, rs2_name);
       RISCV_ASSERT_RV64 (cpu, "insn: %s", op->name);
-      store_rd (cpu, rd, EXTEND32 ((int32_t) riscv_cpu->regs[rs1]
-				   * (int32_t) riscv_cpu->regs[rs2]));
+      store_rd (cpu, rd, EXTEND32 ((uint32_t) riscv_cpu->regs[rs1]
+				   * (uint32_t) riscv_cpu->regs[rs2]));
       break;
     case MATCH_MULH:
       TRACE_INSN (cpu, "mulh %s, %s, %s;  // %s = %s * %s",
 		  rd_name, rs1_name, rs2_name, rd_name, rs1_name, rs2_name);
       if (RISCV_XLEN (cpu) == 32)
 	store_rd (cpu, rd,
-		  ((int64_t)(signed_word) riscv_cpu->regs[rs1]
-		   * (int64_t)(signed_word) riscv_cpu->regs[rs2]) >> 32);
+		  ((uint64_t)(signed_word) riscv_cpu->regs[rs1]
+		   * (uint64_t)(signed_word) riscv_cpu->regs[rs2]) >> 32);
       else
 	store_rd (cpu, rd, mulh (riscv_cpu->regs[rs1], riscv_cpu->regs[rs2]));
       break;
@@ -783,7 +783,7 @@  execute_m (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
       TRACE_INSN (cpu, "mulhsu %s, %s, %s;  // %s = %s * %s",
 		  rd_name, rs1_name, rs2_name, rd_name, rs1_name, rs2_name);
       if (RISCV_XLEN (cpu) == 32)
-	store_rd (cpu, rd, ((int64_t)(signed_word) riscv_cpu->regs[rs1]
+	store_rd (cpu, rd, ((uint64_t)(signed_word) riscv_cpu->regs[rs1]
 			    * (uint64_t)riscv_cpu->regs[rs2]) >> 32);
       else
 	store_rd (cpu, rd, mulhsu (riscv_cpu->regs[rs1], riscv_cpu->regs[rs2]));