Fixes

Author: ligurio

test/tarantool-tests/lj-736-BC_UCLO-triggers-infinite-loop.test.lua

@@ -3,31 +3,23 @@ local test = tap.test('lj-736-BC_UCLO-triggers-infinite-loop'):skipcond({
   ['Test requires JIT enabled'] = not jit.status(),
 })
 
-test:plan(2)
-
 -- Test reproduces an issue when BC_UCLO triggers an infinite loop.
 -- See details in https://github.com/LuaJIT/LuaJIT/issues/736.
---
--- Listing below demonstrates a problem -
--- the bytecode UCLO on the line 13 makes a loop at 0013-0014:
---
--- - BYTECODE -- bc_uclo.lua:0-20
--- 0001    KPRI     0   0
--- 0002    FNEW     1   0      ; bc_uclo.lua:5
--- 0003    KSHORT   2   1
--- 0004    KSHORT   3   4
--- 0005    KSHORT   4   1
--- 0006    FORI     2 => 0011
--- 0007 => ISNEN    5   0      ; 2
--- 0008    JMP      6 => 0010
--- 0009    UCLO     0 => 0012
--- 0010 => FORL     2 => 0007
--- 0011 => UCLO     0 => 0012
--- 0012 => KPRI     0   0
--- 0013    UCLO     0 => 0012
--- 0014    FNEW     1   1      ; bc_uclo.lua:18
--- 0015    UCLO     0 => 0016
--- 0016 => RET0     0   1
+
+test:plan(2)
+
+-- Before the patch, the code flow like in the `testcase()` below
+-- may cause the problem -- use-def analysis for the 0019 UCLO
+-- creates an infinite loop in 0014 - 0019:
+-- | 0008 FORI   base:    4 jump:  => 0013
+-- | 0009 ISNEN  var:     7 num:     0 ; number 2
+-- | 0010 JMP    rbase:   8 jump:  => 0012
+-- | 0011 UCLO   rbase:   2 jump:  => 0014
+-- | 0012 FORL   base:    4 jump:  => 0009
+-- | 0013 UCLO   rbase:   2 jump:  => 0014
+-- | 0014 KPRI   dst:     2 pri:     0 ; Start of `assert()` line.
+-- | ...
+-- | 0019 UCLO   rbase:   2 jump:  => 0014
 
 jit.opt.start('hotloop=1')
 
@@ -36,20 +28,17 @@ local assert = assert
 
 local function testcase()
   -- The code in the first scope `do`/`end` is a prerequisite.
-  -- It is needed so that we have a trace at the exit from which
-  -- the creation of the snapshot will begin.
+  -- It contains the UCLO instruction for the `uv1`. The use-def
+  -- analysis for it escapes this `do`/`end` scope.
   do
-    -- Upvalue below is not used actually, but it is required
-    -- for calling `snap_usedef()` on trace exit.
-    local uv1 -- luacheck: ignore
+    local uv1 -- luacheck: no unused
     local _ = function() return uv1 end
 
-    -- The loop below is required for recording a trace.
-    -- The condition inside a loop executes `goto` to a label
-    -- outside of the loop when the code executed by JIT and
-    -- this triggers snapshotting.
+    -- Records the trace for which use-def analysis is applied.
     for i = 1, 2 do
-      -- Exit to interpreter once trace is compiled.
+      -- This condition triggers snapshoting and use-def analysis.
+      -- Before the patch this triggers the infinite loop in the
+      -- `snap_usedef()`, so the `goto` is never taken.
       if i == 2 then
         goto x
       end
@@ -60,16 +49,12 @@ local function testcase()
   do
     local uv2 -- luacheck: no unused
 
-    -- `goto` if not executed without a patch and generates an
-    -- UCLO bytecode that makes an infinite loop in a function
-    -- `snap_usedef` when patch is not applied. `goto` must point
-    -- to the label on one of the previous lines. `assert()` is
-    -- executed when patch is applied.
+    -- Create a tight loop for the one more upvalue (`uv2`).
+    -- Before the patch, use-def analysis gets stuck in this code
+    -- flow.
     assert(nil, assert_msg)
     goto x
-
-    -- Line below is required, it makes `uv` upvalue, and must be
-    -- placed after `goto`, otherwise reproducer become broken.
+    -- This code is unreachable by design.
     local _ = function() return uv2 end -- luacheck: ignore
   end
 end
@@ -79,4 +64,4 @@ local ok, err = pcall(testcase)
 test:is(ok, false, 'assertion is triggered in a function with testcase')
 test:ok(err:match(assert_msg), 'BC_UCLO does not trigger an infinite loop')
 
-os.exit(test:check() and 0 or 1)
+test:done(true)