Skip to content

Commit 05183bd

Browse files
Mike Palligormunkin
Mike Pall
authored andcommitted
Mark CONV as non-weak, to prevent elimination of its side-effect.
An unused guarded CONV int.num cannot be omitted in general. (cherry-picked from commit 881d02d) 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 Reviewed-by: Sergey Kaplun <[email protected]> Reviewed-by: Sergey Bronnikov <[email protected]> Signed-off-by: Igor Munkin <[email protected]> (cherry picked from commit eac9ead)
1 parent f3eff48 commit 05183bd

File tree

2 files changed

+126
-1
lines changed

2 files changed

+126
-1
lines changed

src/lj_ir.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@
132132
_(XBAR, S , ___, ___) \
133133
\
134134
/* Type conversions. */ \
135-
_(CONV, NW, ref, lit) \
135+
_(CONV, N , ref, lit) \
136136
_(TOBIT, N , ref, ref) \
137137
_(TOSTR, N , ref, lit) \
138138
_(STRTO, N , ref, ___) \
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
local tap = require('tap')
2+
local test = tap.test('mark-conv-non-weak'):skipcond({
3+
['Test requires JIT enabled'] = not jit.status(),
4+
})
5+
6+
test:plan(1)
7+
-- XXX: These values were chosen to create type instability
8+
-- in the loop-carried dependency, so the checked `CONV int.num`
9+
-- instruction is emitted. See `loop_unrool` in `lj_opt_loop.c`.
10+
local data = {0, 0.1, 0, 0 / 0}
11+
12+
--- XXX: The sum is required to be initialized with a non-zero
13+
-- floating point value; otherwise, `0023  + num ADD    0017  0007`
14+
-- instruction in the IR below becomes `ADDOV` and the `CONV int.num`
15+
-- conversion is used by it.
16+
local sum = 0.1
17+
18+
jit.opt.start('hotloop=1')
19+
20+
-- XXX: The test fails before the patch only
21+
-- for `DUALNUM` mode. All of the IRs below are
22+
-- produced by the corresponding LuaJIT build.
23+
24+
-- When the trace is recorded, the IR
25+
-- is the following before the patch:
26+
---- TRACE 1 IR
27+
-- .... SNAP #0 [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ]
28+
-- 0001 u8 XLOAD [0x100dac521] V
29+
-- 0002 int BAND 0001 +12
30+
-- 0003 > int EQ 0002 +0
31+
-- 0004 > int SLOAD #8 T
32+
-- .... SNAP #1 [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ]
33+
-- 0005 > num SLOAD #3 T
34+
-- 0006 num CONV 0004 num.int
35+
-- 0007 + num ADD 0006 0005
36+
-- 0008 > fun SLOAD #4 T
37+
-- 0009 > tab SLOAD #5 T
38+
-- 0010 > int SLOAD #6 T
39+
-- 0011 > fun EQ 0008 ipairs_aux
40+
-- 0012 + int ADD 0010 +1
41+
-- 0013 int FLOAD 0009 tab.asize
42+
-- 0014 > int ABC 0013 0012
43+
-- 0015 p64 FLOAD 0009 tab.array
44+
-- 0016 p64 AREF 0015 0012
45+
-- 0017 >+ num ALOAD 0016
46+
-- .... SNAP #2 [ ---- ---- ---- 0007 ---- ---- 0012 0012 0017 ]
47+
-- 0018 ------ LOOP ------------
48+
-- 0019 u8 XLOAD [0x100dac521] V
49+
-- 0020 int BAND 0019 +12
50+
-- 0021 > int EQ 0020 +0
51+
-- 0022 > int CONV 0017 int.num
52+
-- .... SNAP #3 [ ---- ---- ---- 0007 ---- ---- 0012 0012 0017 ]
53+
-- 0023 + num ADD 0017 0007
54+
-- 0024 + int ADD 0012 +1
55+
-- 0025 > int ABC 0013 0024
56+
-- 0026 p64 AREF 0015 0024
57+
-- 0027 >+ num ALOAD 0026
58+
-- 0028 num PHI 0017 0027
59+
-- 0029 num PHI 0007 0023
60+
-- 0030 int PHI 0012 0024
61+
---- TRACE 1 stop -> loop
62+
63+
---- TRACE 1 exit 0
64+
---- TRACE 1 exit 3
65+
--
66+
-- And the following after the patch:
67+
---- TRACE 1 IR
68+
-- .... SNAP #0 [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ]
69+
-- 0001 u8 XLOAD [0x102438521] V
70+
-- 0002 int BAND 0001 +12
71+
-- 0003 > int EQ 0002 +0
72+
-- 0004 > int SLOAD #8 T
73+
-- .... SNAP #1 [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ]
74+
-- 0005 > num SLOAD #3 T
75+
-- 0006 num CONV 0004 num.int
76+
-- 0007 + num ADD 0006 0005
77+
-- 0008 > fun SLOAD #4 T
78+
-- 0009 > tab SLOAD #5 T
79+
-- 0010 > int SLOAD #6 T
80+
-- 0011 > fun EQ 0008 ipairs_aux
81+
-- 0012 + int ADD 0010 +1
82+
-- 0013 int FLOAD 0009 tab.asize
83+
-- 0014 > int ABC 0013 0012
84+
-- 0015 p64 FLOAD 0009 tab.array
85+
-- 0016 p64 AREF 0015 0012
86+
-- 0017 >+ num ALOAD 0016
87+
-- .... SNAP #2 [ ---- ---- ---- 0007 ---- ---- 0012 0012 0017 ]
88+
-- 0018 ------ LOOP ------------
89+
-- 0019 u8 XLOAD [0x102438521] V
90+
-- 0020 int BAND 0019 +12
91+
-- 0021 > int EQ 0020 +0
92+
-- 0022 > int CONV 0017 int.num
93+
-- .... SNAP #3 [ ---- ---- ---- 0007 ---- ---- 0012 0012 0017 ]
94+
-- 0023 + num ADD 0017 0007
95+
-- 0024 + int ADD 0012 +1
96+
-- 0025 > int ABC 0013 0024
97+
-- 0026 p64 AREF 0015 0024
98+
-- 0027 >+ num ALOAD 0026
99+
-- 0028 num PHI 0017 0027
100+
-- 0029 num PHI 0007 0023
101+
-- 0030 int PHI 0012 0024
102+
---- TRACE 1 stop -> loop
103+
104+
---- TRACE 1 exit 0
105+
---- TRACE 1 exit 2
106+
--
107+
-- Before the patch, the `0022 > int CONV 0017 int.num`
108+
-- instruction is omitted due to DCE, which results in the
109+
-- third side exit being taken, instead of the second,
110+
-- and, hence, incorrect summation. After the patch, `CONV`
111+
-- is left intact and is not omitted; it remains as a guarded
112+
-- instruction, so the second side exit is taken and sum is
113+
-- performed correctly.
114+
--
115+
-- Note that DCE happens on the assembly part of the trace
116+
-- compilation. That is why `CONV` is present in both IRs.
117+
118+
for _, val in ipairs(data) do
119+
if val == val then
120+
sum = sum + val
121+
end
122+
end
123+
124+
test:ok(sum == sum, 'NaN check was not omitted')
125+
test:done(true)

0 commit comments

Comments
 (0)