[Tarantool-patches] [PATCH luajit v2] Mark CONV as non-weak, to prevent elimination of its side-effect.

[Maksim Kokryashkin]

From: Mike Pall <mike>

An unused guarded CONV int.num cannot be omitted in general.

(cherry-picked from commit 881d02d3117838acaf4fb844332c8e33cc95c8c5)

In some cases, an unused `CONV int.num` omission in `DUALNUM` mode
may lead to a guard absence, resulting in invalid control flow
branching and undefined behavior. For a comprehensive example of
the described situation, please refer to the comment
in `test/tarantool-tests/mark-conv-non-weak.test.lua`.

Maxim Kokryashkin:
* added the description and the test for the problem

Part of tarantool/tarantool#9145
---
Branch: https://github.com/tarantool/luajit/tree/fckxorg/mark-conv-non-weak
PR: https://github.com/tarantool/tarantool/pull/8871

Changes in v2:
- Fixed comments as per review by Sergey Kaplun
 src/lj_ir.h                                   |   2 +-
 .../mark-conv-non-weak.test.lua               | 115 ++++++++++++++++++
 2 files changed, 116 insertions(+), 1 deletion(-)
 create mode 100644 test/tarantool-tests/mark-conv-non-weak.test.lua

diff --git a/src/lj_ir.h b/src/lj_ir.h
index 46af54e4..e3ba6ff3 100644
--- a/src/lj_ir.h
+++ b/src/lj_ir.h
@@ -132,7 +132,7 @@
   _(XBAR,	S , ___, ___) \
   \
   /* Type conversions. */ \
-  _(CONV,	NW, ref, lit) \
+  _(CONV,	N , ref, lit) \
   _(TOBIT,	N , ref, ref) \
   _(TOSTR,	N , ref, lit) \
   _(STRTO,	N , ref, ___) \
diff --git a/test/tarantool-tests/mark-conv-non-weak.test.lua b/test/tarantool-tests/mark-conv-non-weak.test.lua
new file mode 100644
index 00000000..f54f30ba
--- /dev/null
+++ b/test/tarantool-tests/mark-conv-non-weak.test.lua
@@ -0,0 +1,115 @@
+local tap = require('tap')
+local test = tap.test('mark-conv-non-weak'):skipcond({
+    ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(1)
+
+local data = {0.1, 0, 0.1, 0, 0 / 0}
+local sum = 0
+
+jit.opt.start('hotloop=2', 'hotexit=2')
+
+-- XXX: The test fails before the patch only
+-- for `DUALNUM` mode. All of the IRs below are
+-- produced by the corresponding LuaJIT build.
+
+-- When the trace is recorded, the IR
+-- is the following before the patch:
+---- TRACE 1 IR
+-- ....        SNAP   #0   [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ]
+-- 0001    u8  XLOAD  [0x100dac521]  V
+-- 0002    int BAND   0001  +12
+-- 0003 >  int EQ     0002  +0
+-- 0004 >  int SLOAD  #8    T
+-- ....        SNAP   #1   [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ]
+-- 0005 >  num SLOAD  #3    T
+-- 0006    num CONV   0004  num.int
+-- 0007  + num ADD    0006  0005
+-- 0008 >  fun SLOAD  #4    T
+-- 0009 >  tab SLOAD  #5    T
+-- 0010 >  int SLOAD  #6    T
+-- 0011 >  fun EQ     0008  ipairs_aux
+-- 0012  + int ADD    0010  +1
+-- 0013    int FLOAD  0009  tab.asize
+-- 0014 >  int ABC    0013  0012
+-- 0015    p64 FLOAD  0009  tab.array
+-- 0016    p64 AREF   0015  0012
+-- 0017 >+ num ALOAD  0016
+-- ....        SNAP   #2   [ ---- ---- ---- 0007 ---- ---- 0012 0012 0017 ]
+-- 0018 ------ LOOP ------------
+-- 0019    u8  XLOAD  [0x100dac521]  V
+-- 0020    int BAND   0019  +12
+-- 0021 >  int EQ     0020  +0
+-- 0022 >  int CONV   0017  int.num
+-- ....        SNAP   #3   [ ---- ---- ---- 0007 ---- ---- 0012 0012 0017 ]
+-- 0023  + num ADD    0017  0007
+-- 0024  + int ADD    0012  +1
+-- 0025 >  int ABC    0013  0024
+-- 0026    p64 AREF   0015  0024
+-- 0027 >+ num ALOAD  0026
+-- 0028    num PHI    0017  0027
+-- 0029    num PHI    0007  0023
+-- 0030    int PHI    0012  0024
+---- TRACE 1 stop -> loop
+
+---- TRACE 1 exit 0
+---- TRACE 1 exit 3
+--
+-- And the following after the patch:
+---- TRACE 1 IR
+-- ....        SNAP   #0   [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ]
+-- 0001    u8  XLOAD  [0x102438521]  V
+-- 0002    int BAND   0001  +12
+-- 0003 >  int EQ     0002  +0
+-- 0004 >  int SLOAD  #8    T
+-- ....        SNAP   #1   [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ]
+-- 0005 >  num SLOAD  #3    T
+-- 0006    num CONV   0004  num.int
+-- 0007  + num ADD    0006  0005
+-- 0008 >  fun SLOAD  #4    T
+-- 0009 >  tab SLOAD  #5    T
+-- 0010 >  int SLOAD  #6    T
+-- 0011 >  fun EQ     0008  ipairs_aux
+-- 0012  + int ADD    0010  +1
+-- 0013    int FLOAD  0009  tab.asize
+-- 0014 >  int ABC    0013  0012
+-- 0015    p64 FLOAD  0009  tab.array
+-- 0016    p64 AREF   0015  0012
+-- 0017 >+ num ALOAD  0016
+-- ....        SNAP   #2   [ ---- ---- ---- 0007 ---- ---- 0012 0012 0017 ]
+-- 0018 ------ LOOP ------------
+-- 0019    u8  XLOAD  [0x102438521]  V
+-- 0020    int BAND   0019  +12
+-- 0021 >  int EQ     0020  +0
+-- 0022 >  int CONV   0017  int.num
+-- ....        SNAP   #3   [ ---- ---- ---- 0007 ---- ---- 0012 0012 0017 ]
+-- 0023  + num ADD    0017  0007
+-- 0024  + int ADD    0012  +1
+-- 0025 >  int ABC    0013  0024
+-- 0026    p64 AREF   0015  0024
+-- 0027 >+ num ALOAD  0026
+-- 0028    num PHI    0017  0027
+-- 0029    num PHI    0007  0023
+-- 0030    int PHI    0012  0024
+---- TRACE 1 stop -> loop
+
+---- TRACE 1 exit 0
+---- TRACE 1 exit 2
+--
+-- Before the patch, the `0022 >  int CONV   0017  int.num`
+-- instruction is omitted due to DCE, which results in the
+-- third side exit being taken, instead of the second,
+-- and, hence, incorrect summation. After the patch, `CONV`
+-- is left intact and is not omitted; it remains as a guarded
+-- instruction, so the second side exit is taken and sum is
+-- performed correctly.
+
+for _, val in ipairs(data) do
+    if val == val then
+        sum = sum + val
+    end
+end
+
+test:ok(sum == sum, 'NaN check was not omitted')
+test:done(true)
--
2.39.3 (Apple Git-145)