Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH] Mark CONV as non-weak, to prevent elimination of its side-effect.
@ 2023-07-12  9:52 Maksim Kokryashkin via Tarantool-patches
  2023-07-13 12:23 ` Sergey Bronnikov via Tarantool-patches
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Maksim Kokryashkin via Tarantool-patches @ 2023-07-12  9:52 UTC (permalink / raw)
  To: tarantool-patches, sergeyb, skaplun, m.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` omission 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               | 58 +++++++++++++++++++
 2 files changed, 59 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 e8bca275..bf9b9292 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..aad39058
--- /dev/null
+++ b/test/tarantool-tests/mark-conv-non-weak.test.lua
@@ -0,0 +1,58 @@
+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')
+
+-- 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)
-- 
2.39.2 (Apple Git-143)


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-11-23  6:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-12  9:52 [Tarantool-patches] [PATCH] Mark CONV as non-weak, to prevent elimination of its side-effect Maksim Kokryashkin via Tarantool-patches
2023-07-13 12:23 ` Sergey Bronnikov via Tarantool-patches
2023-07-17 15:11   ` Maxim Kokryashkin via Tarantool-patches
2023-11-20  8:24     ` Sergey Bronnikov via Tarantool-patches
2023-08-24  7:26 ` Sergey Kaplun via Tarantool-patches
2023-11-23  6:31 ` Igor Munkin via Tarantool-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox