Coverity reported (CID 1450831) an array overrun in
gen_mxu_D16MAX_D16MIN():
1103 } else if (unlikely((XRb == 0) || (XRa == 0))) {
....
1112 if (opc == OPC_MXU_D16MAX) {
1113 tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
1114 } else {
1115 tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
1116 }
>>> Overrunning array "mxu_gpr" of 15 8-byte elements at element
index 4294967295 (byte offset 34359738367) using index "XRa - 1U"
(which evaluates to 4294967295).
Because we check if 'XRa == 0' then access 'XRa - 1' in array.
I figured it could be easier to rewrite this function to something
simpler rather than trying to understand it.
Cc: Craig Janeczek <jancr...@amazon.com>
Fixes: bb84cbf3850 ("target/mips: MXU: Add handlers for max/min instructions")
Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
---
target/mips/mxu_translate.c | 116 ++++++++++++------------------------
1 file changed, 38 insertions(+), 78 deletions(-)
diff --git a/target/mips/mxu_translate.c b/target/mips/mxu_translate.c
index afc008eeeef..8673a0139d4 100644
--- a/target/mips/mxu_translate.c
+++ b/target/mips/mxu_translate.c
@@ -1086,89 +1086,49 @@ static void gen_mxu_S32MAX_S32MIN(DisasContext *ctx)
static void gen_mxu_D16MAX_D16MIN(DisasContext *ctx)
{
uint32_t pad, opc, XRc, XRb, XRa;
+ TCGv_i32 b, c, bs, cs;
+ TCGCond cond;
pad = extract32(ctx->opcode, 21, 5);
- opc = extract32(ctx->opcode, 18, 3);
- XRc = extract32(ctx->opcode, 14, 4);
- XRb = extract32(ctx->opcode, 10, 4);
- XRa = extract32(ctx->opcode, 6, 4);
-
if (unlikely(pad != 0)) {
/* opcode padding incorrect -> do nothing */
- } else if (unlikely(XRc == 0)) {
- /* destination is zero register -> do nothing */
- } else if (unlikely((XRb == 0) && (XRa == 0))) {
- /* both operands zero registers -> just set destination to zero */
- tcg_gen_movi_i32(mxu_gpr[XRc - 1], 0);
- } else if (unlikely((XRb == 0) || (XRa == 0))) {
- /* exactly one operand is zero register - find which one is not...*/
- uint32_t XRx = XRb ? XRb : XRc;
- /* ...and do half-word-wise max/min with one operand 0 */
- TCGv_i32 t0 = tcg_temp_new();
- TCGv_i32 t1 = tcg_const_i32(0);
-
- /* the left half-word first */
- tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0xFFFF0000);
- if (opc == OPC_MXU_D16MAX) {
- tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
- } else {
- tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
- }
-
- /* the right half-word */
- tcg_gen_andi_i32(t0, mxu_gpr[XRx - 1], 0x0000FFFF);
- /* move half-words to the leftmost position */
- tcg_gen_shli_i32(t0, t0, 16);
- /* t0 will be max/min of t0 and t1 */
- if (opc == OPC_MXU_D16MAX) {
- tcg_gen_smax_i32(t0, t0, t1);
- } else {
- tcg_gen_smin_i32(t0, t0, t1);
- }
- /* return resulting half-words to its original position */
- tcg_gen_shri_i32(t0, t0, 16);
- /* finally update the destination */
- tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
-
- tcg_temp_free(t1);
- tcg_temp_free(t0);
- } else if (unlikely(XRb == XRc)) {
- /* both operands same -> just set destination to one of them */
- tcg_gen_mov_i32(mxu_gpr[XRa - 1], mxu_gpr[XRb - 1]);
- } else {
- /* the most general case */
- TCGv_i32 t0 = tcg_temp_new();
- TCGv_i32 t1 = tcg_temp_new();
-
- /* the left half-word first */
- tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0xFFFF0000);
- tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0xFFFF0000);
- if (opc == OPC_MXU_D16MAX) {
- tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
- } else {
- tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
- }
-
- /* the right half-word */
- tcg_gen_andi_i32(t0, mxu_gpr[XRb - 1], 0x0000FFFF);
- tcg_gen_andi_i32(t1, mxu_gpr[XRc - 1], 0x0000FFFF);
- /* move half-words to the leftmost position */
- tcg_gen_shli_i32(t0, t0, 16);
- tcg_gen_shli_i32(t1, t1, 16);
- /* t0 will be max/min of t0 and t1 */
- if (opc == OPC_MXU_D16MAX) {
- tcg_gen_smax_i32(t0, t0, t1);
- } else {
- tcg_gen_smin_i32(t0, t0, t1);
- }
- /* return resulting half-words to its original position */
- tcg_gen_shri_i32(t0, t0, 16);
- /* finally update the destination */
- tcg_gen_or_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], t0);
-
- tcg_temp_free(t1);
- tcg_temp_free(t0);
+ return;
}
+
+ XRa = extract32(ctx->opcode, 6, 4);
+ if (unlikely(XRa == 0)) {
+ /* destination is zero register -> do nothing */
+ return;
+ }
+ b = tcg_temp_new();
+ c = tcg_temp_new();
+ bs = tcg_temp_new();
+ cs = tcg_temp_new();
+
+ opc = extract32(ctx->opcode, 18, 3);
+ cond = (opc == OPC_MXU_D16MAX) ? TCG_COND_GT : TCG_COND_LE;
+
+ XRb = extract32(ctx->opcode, 10, 4);
+ XRc = extract32(ctx->opcode, 14, 4);
+ gen_load_mxu_gpr(b, XRb);
+ gen_load_mxu_gpr(c, XRc);
+
+ /* short0 */
+ tcg_gen_sextract_i32(bs, b, 0, 16);
+ tcg_gen_sextract_i32(cs, c, 0, 16);
+ tcg_gen_movcond_i32(cond, mxu_gpr[XRa - 1], bs, cs, bs, cs);
+
+ /* short1 */
+ tcg_gen_sextract_i32(bs, b, 16, 16);
+ tcg_gen_sextract_i32(cs, c, 16, 16);
+ tcg_gen_movcond_i32(cond, b, bs, cs, bs, cs);
+
+ tcg_gen_deposit_i32(mxu_gpr[XRa - 1], mxu_gpr[XRa - 1], b, 16, 16);
+
+ tcg_temp_free(cs);
+ tcg_temp_free(bs);
+ tcg_temp_free(c);
+ tcg_temp_free(b);
}
/*
--
2.26.2
