[Tarantool-patches] [PATCH luajit v4 1/3] Fix math.min()/math.max() inconsistencies.

[Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

From: Mike Pall <mike>

math.min()/math.max() could produce different results when compiled.
Previously, these functions did not check for the number of arguments,
which in some situations led to incorrect behavior.

Consider this example:
```
jit.opt.start(0, "hotloop=1")

local r, msg = pcall(function () math.min() end)
r, msg = pcall(function () math.min() end)
```
Before this patch, this snippet of code completed successfully, although it
was not supposed to, since there were no args to math.min().

This patch adds check for the number of arguments for jit-compiled
version of math.min/max, and it also adds the corresponding test case for
the mentioned issue.

Part of tarantool/tarantool#6163
Part of tarantool/tarantool#6548
---
 src/lj_asm_arm.h                              |  6 +--
 src/lj_asm_arm64.h                            |  6 +--
 src/lj_opt_fold.c                             | 53 +++++++------------
 src/lj_vmmath.c                               |  4 +-
 src/vm_arm.dasc                               |  4 +-
 src/vm_arm64.dasc                             |  4 +-
 src/vm_x64.dasc                               |  2 +-
 src/vm_x86.dasc                               |  2 +-
 .../gh-6163-jit-min-max.test.lua              | 20 +++++++
 9 files changed, 53 insertions(+), 48 deletions(-)
 create mode 100644 test/tarantool-tests/gh-6163-jit-min-max.test.lua

diff --git a/src/lj_asm_arm.h b/src/lj_asm_arm.h
index 4fd08b9e..84ab06c0 100644
--- a/src/lj_asm_arm.h
+++ b/src/lj_asm_arm.h
@@ -1664,8 +1664,8 @@ static void asm_min_max(ASMState *as, IRIns *ir, int cc, int fcc)
     asm_intmin_max(as, ir, cc);
 }
 
-#define asm_min(as, ir)		asm_min_max(as, ir, CC_GT, CC_HI)
-#define asm_max(as, ir)		asm_min_max(as, ir, CC_LT, CC_LO)
+#define asm_min(as, ir)		asm_min_max(as, ir, CC_GT, CC_PL)
+#define asm_max(as, ir)		asm_min_max(as, ir, CC_LT, CC_LE)
 
 /* -- Comparisons --------------------------------------------------------- */
 
@@ -1857,7 +1857,7 @@ static void asm_hiop(ASMState *as, IRIns *ir)
   } else if ((ir-1)->o == IR_MIN || (ir-1)->o == IR_MAX) {
     as->curins--;  /* Always skip the loword min/max. */
     if (uselo || usehi)
-      asm_sfpmin_max(as, ir-1, (ir-1)->o == IR_MIN ? CC_HI : CC_LO);
+      asm_sfpmin_max(as, ir-1, (ir-1)->o == IR_MIN ? CC_PL : CC_LE);
     return;
 #elif LJ_HASFFI
   } else if ((ir-1)->o == IR_CONV) {
diff --git a/src/lj_asm_arm64.h b/src/lj_asm_arm64.h
index cc8c0c9d..e93e29e3 100644
--- a/src/lj_asm_arm64.h
+++ b/src/lj_asm_arm64.h
@@ -1594,7 +1594,7 @@ static void asm_fpmin_max(ASMState *as, IRIns *ir, A64CC fcc)
   Reg dest = (ra_dest(as, ir, RSET_FPR) & 31);
   Reg right, left = ra_alloc2(as, ir, RSET_FPR);
   right = ((left >> 8) & 31); left &= 31;
-  emit_dnm(as, A64I_FCSELd | A64F_CC(fcc), dest, left, right);
+  emit_dnm(as, A64I_FCSELd | A64F_CC(fcc), dest, right, left);
   emit_nm(as, A64I_FCMPd, left, right);
 }
 
@@ -1606,8 +1606,8 @@ static void asm_min_max(ASMState *as, IRIns *ir, A64CC cc, A64CC fcc)
     asm_intmin_max(as, ir, cc);
 }
 
-#define asm_max(as, ir)		asm_min_max(as, ir, CC_GT, CC_HI)
-#define asm_min(as, ir)		asm_min_max(as, ir, CC_LT, CC_LO)
+#define asm_min(as, ir)		asm_min_max(as, ir, CC_LT, CC_PL)
+#define asm_max(as, ir)		asm_min_max(as, ir, CC_GT, CC_LE)
 
 /* -- Comparisons --------------------------------------------------------- */
 
diff --git a/src/lj_opt_fold.c b/src/lj_opt_fold.c
index 3c508062..29b760b4 100644
--- a/src/lj_opt_fold.c
+++ b/src/lj_opt_fold.c
@@ -1774,8 +1774,6 @@ LJFOLDF(reassoc_intarith_k64)
 #endif
 }
 
-LJFOLD(MIN MIN any)
-LJFOLD(MAX MAX any)
 LJFOLD(BAND BAND any)
 LJFOLD(BOR BOR any)
 LJFOLDF(reassoc_dup)
@@ -1785,6 +1783,15 @@ LJFOLDF(reassoc_dup)
   return NEXTFOLD;
 }
 
+LJFOLD(MIN MIN any)
+LJFOLD(MAX MAX any)
+LJFOLDF(reassoc_dup_minmax)
+{
+  if (fins->op2 == fleft->op2)
+    return LEFTFOLD;  /* (a o b) o b ==> a o b */
+  return NEXTFOLD;
+}
+
 LJFOLD(BXOR BXOR any)
 LJFOLDF(reassoc_bxor)
 {
@@ -1823,23 +1830,12 @@ LJFOLDF(reassoc_shift)
   return NEXTFOLD;
 }
 
-LJFOLD(MIN MIN KNUM)
-LJFOLD(MAX MAX KNUM)
 LJFOLD(MIN MIN KINT)
 LJFOLD(MAX MAX KINT)
 LJFOLDF(reassoc_minmax_k)
 {
   IRIns *irk = IR(fleft->op2);
-  if (irk->o == IR_KNUM) {
-    lua_Number a = ir_knum(irk)->n;
-    lua_Number y = lj_vm_foldarith(a, knumright, fins->o - IR_ADD);
-    if (a == y)  /* (x o k1) o k2 ==> x o k1, if (k1 o k2) == k1. */
-      return LEFTFOLD;
-    PHIBARRIER(fleft);
-    fins->op1 = fleft->op1;
-    fins->op2 = (IRRef1)lj_ir_knum(J, y);
-    return RETRYFOLD;  /* (x o k1) o k2 ==> x o (k1 o k2) */
-  } else if (irk->o == IR_KINT) {
+  if (irk->o == IR_KINT) {
     int32_t a = irk->i;
     int32_t y = kfold_intop(a, fright->i, fins->o);
     if (a == y)  /* (x o k1) o k2 ==> x o k1, if (k1 o k2) == k1. */
@@ -1852,24 +1848,6 @@ LJFOLDF(reassoc_minmax_k)
   return NEXTFOLD;
 }
 
-LJFOLD(MIN MAX any)
-LJFOLD(MAX MIN any)
-LJFOLDF(reassoc_minmax_left)
-{
-  if (fins->op2 == fleft->op1 || fins->op2 == fleft->op2)
-    return RIGHTFOLD;  /* (b o1 a) o2 b ==> b; (a o1 b) o2 b ==> b */
-  return NEXTFOLD;
-}
-
-LJFOLD(MIN any MAX)
-LJFOLD(MAX any MIN)
-LJFOLDF(reassoc_minmax_right)
-{
-  if (fins->op1 == fright->op1 || fins->op1 == fright->op2)
-    return LEFTFOLD;  /* a o2 (a o1 b) ==> a; a o2 (b o1 a) ==> a */
-  return NEXTFOLD;
-}
-
 /* -- Array bounds check elimination -------------------------------------- */
 
 /* Eliminate ABC across PHIs to handle t[i-1] forwarding case.
@@ -1995,8 +1973,6 @@ LJFOLDF(comm_comp)
 
 LJFOLD(BAND any any)
 LJFOLD(BOR any any)
-LJFOLD(MIN any any)
-LJFOLD(MAX any any)
 LJFOLDF(comm_dup)
 {
   if (fins->op1 == fins->op2)  /* x o x ==> x */
@@ -2004,6 +1980,15 @@ LJFOLDF(comm_dup)
   return fold_comm_swap(J);
 }
 
+LJFOLD(MIN any any)
+LJFOLD(MAX any any)
+LJFOLDF(comm_dup_minmax)
+{
+  if (fins->op1 == fins->op2)  /* x o x ==> x */
+    return LEFTFOLD;
+  return NEXTFOLD;
+}
+
 LJFOLD(BXOR any any)
 LJFOLDF(comm_bxor)
 {
diff --git a/src/lj_vmmath.c b/src/lj_vmmath.c
index b231d3e8..08ccf467 100644
--- a/src/lj_vmmath.c
+++ b/src/lj_vmmath.c
@@ -50,8 +50,8 @@ double lj_vm_foldarith(double x, double y, int op)
 #if LJ_HASJIT
   case IR_ATAN2 - IR_ADD: return atan2(x, y); break;
   case IR_LDEXP - IR_ADD: return ldexp(x, (int)y); break;
-  case IR_MIN - IR_ADD: return x > y ? y : x; break;
-  case IR_MAX - IR_ADD: return x < y ? y : x; break;
+  case IR_MIN - IR_ADD: return x < y ? x : y; break;
+  case IR_MAX - IR_ADD: return x > y ? x : y; break;
 #endif
   default: return x;
   }
diff --git a/src/vm_arm.dasc b/src/vm_arm.dasc
index 21f7fecb..6b511347 100644
--- a/src/vm_arm.dasc
+++ b/src/vm_arm.dasc
@@ -1718,8 +1718,8 @@ static void build_subroutines(BuildCtx *ctx)
   |.endif
   |.endmacro
   |
-  |  math_minmax math_min, gt, hi
-  |  math_minmax math_max, lt, lo
+  |  math_minmax math_min, gt, pl
+  |  math_minmax math_max, lt, le
   |
   |//-- String library -----------------------------------------------------
   |
diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
index 313cc94f..80e795ae 100644
--- a/src/vm_arm64.dasc
+++ b/src/vm_arm64.dasc
@@ -1491,8 +1491,8 @@ static void build_subroutines(BuildCtx *ctx)
   |  b <6
   |.endmacro
   |
-  |  math_minmax math_min, gt, hi
-  |  math_minmax math_max, lt, lo
+  |  math_minmax math_min, gt, pl
+  |  math_minmax math_max, lt, le
   |
   |//-- String library -----------------------------------------------------
   |
diff --git a/src/vm_x64.dasc b/src/vm_x64.dasc
index 974047d3..cbf5fb9b 100644
--- a/src/vm_x64.dasc
+++ b/src/vm_x64.dasc
@@ -1874,7 +1874,7 @@ static void build_subroutines(BuildCtx *ctx)
   |  jmp ->fff_res
   |
   |.macro math_minmax, name, cmovop, sseop
-  |  .ffunc name
+  |  .ffunc_1 name
   |  mov RAd, 2
   |.if DUALNUM
   |  mov RB, [BASE]
diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc
index ab8e6f27..b839eff0 100644
--- a/src/vm_x86.dasc
+++ b/src/vm_x86.dasc
@@ -2292,7 +2292,7 @@ static void build_subroutines(BuildCtx *ctx)
   |  xorps xmm4, xmm4; jmp <1			// Return +-Inf and +-0.
   |
   |.macro math_minmax, name, cmovop, sseop
-  |  .ffunc name
+  |  .ffunc_1 name
   |  mov RA, 2
   |  cmp dword [BASE+4], LJ_TISNUM
   |.if DUALNUM
diff --git a/test/tarantool-tests/gh-6163-jit-min-max.test.lua b/test/tarantool-tests/gh-6163-jit-min-max.test.lua
new file mode 100644
index 00000000..d6eb3f3b
--- /dev/null
+++ b/test/tarantool-tests/gh-6163-jit-min-max.test.lua
@@ -0,0 +1,20 @@
+local tap = require('tap')
+jit.off()
+jit.flush()
+
+local test = tap.test("gh-6163-jit-min-max")
+test:plan(2)
+--
+-- gh-6163: math.min/math.max success with no args
+--
+local pcall = pcall
+
+jit.opt.start(0, 'hotloop=1')
+jit.on()
+
+local r, _ = pcall(function() math.min() end)
+test:ok(false == r)
+r, _ = pcall(function() math.min() end)
+test:ok(false == r)
+
+os.exit(test:check() and 0 or 1)
-- 
2.35.1