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).