public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
To: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: [PATCH] sim: riscv: Fix undefined behaviour in mulh and similar
Date: Fri, 26 Apr 2024 09:50:58 +0200	[thread overview]
Message-ID: <AS8P193MB1285843C26CFAD73EC0FAEBEE4162@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM> (raw)

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 --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]));
-- 
2.39.2


                 reply	other threads:[~2024-04-26  7:49 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AS8P193MB1285843C26CFAD73EC0FAEBEE4162@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM \
    --to=bernd.edlinger@hotmail.de \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).