From 68cc3599eb7c645e3d8a4b19094908d1b0c4ae11 Mon Sep 17 00:00:00 2001 From: Mike Pall Date: Tue, 22 Sep 2020 11:56:06 +0200 Subject: [PATCH] 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 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#8825 --- src/lj_ir.h | 2 +- .../mark-conv-non-weak.test.lua | 62 +++++++++++++++++++ 2 files changed, 63 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 e8bca275e4..bf9b92923f 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 0000000000..9a325202f6 --- /dev/null +++ b/test/tarantool-tests/mark-conv-non-weak.test.lua @@ -0,0 +1,62 @@ +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=1', 'hotexit=1') + +-- 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 last trace is recorded, the traced bytecode +-- is the following before the patch: +-- ---- TRACE 4 start 2/3 test.lua:6 +-- 0018 ADDVV 1 1 6 +-- 0019 ITERC 5 3 3 +-- 0000 . FUNCC ; ipairs_aux +-- 0020 JITERL 5 1 +-- 0021 GGET 2 7 ; "assert" +-- 0022 ISEQV 1 1 +-- 0023 JMP 4 => 0026 +-- 0024 KPRI 4 1 +-- 0025 JMP 5 => 0027 +-- 0027 CALL 2 1 2 +-- 0000 . FUNCC ; assert +-- +-- And the following after the patch: +-- ---- TRACE 4 start 2/2 test.lua:5 +-- 0016 ISNEV 6 6 +-- 0017 JMP 7 => 0019 +-- 0019 ITERC 5 3 3 +-- 0000 . FUNCC ; ipairs_aux +-- 0020 JITERL 5 1 +-- 0021 GGET 2 7 ; "assert" +-- 0022 ISEQV 1 1 +-- 0023 JMP 4 => 0026 +-- 0026 KPRI 4 2 +-- 0027 CALL 2 1 2 +-- 0000 . FUNCC ; assert +-- 0028 RET0 0 1 +-- +-- The crucial difference here is the abscent +-- `ISNEV` in the first case, which produces the +-- desired guarded `CONV`, when translated to IR. +-- +-- Since there is no guard, NaN is added to the sum, +-- despite the test case logic. + +for _, val in ipairs(data) do + if val == val then + sum = sum + val + end +end + +test:ok(sum == sum, 'NaN check was not omitted') + +os.exit(test:check() and 0 or 1)