Re: [Tarantool-patches] [PATCH 1/2] test: disable JIT for Lua Fun totable function

["Alexander V. Tikhonov" <avtikhon@tarantool.org>]

Hi Igor, many thanks for the patch. It was successfully checked
for a long time run, the patch LGTM.

On Tue, Jun 23, 2020 at 09:04:08PM +0300, Igor Munkin wrote:
> Sasha,
> 
> Many thanks for such precise testing you've made for this patch!
> 
> It turned out the patch was totally wrong, since it disables JIT for the
> wrong function (i.e. <totable> method) and its subfunctions. However it
> doesn't disable JIT for <totable> *callees*, so invalid traces continue
> to be compiled. I've made a totally new patch (at the end of the reply)
> that is quite similar to this one. Sasha has already tested it manually
> and CI is green (I've rebased my branch on the bleeding master).
> 
> Sasha, Vlad, could you please glance at the changes once more?
> 
> On 19.06.20, Igor Munkin wrote:
> > JIT compiler can generate invalid trace for <totable> function breaking
> > its semantics (see LuaJIT/LuaJIT#584). Since interpreter works fine and
> > produces right results, disabling JIT for this function (and all its
> > subfunctions) stops execution failures.
> > 
> > Relates to LuaJIT/LuaJIT#584
> > Fixes #4252
> > 
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> >  test/box-tap/key_def.test.lua | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/test/box-tap/key_def.test.lua b/test/box-tap/key_def.test.lua
> > index d7dbf5b88..ce7a3cb63 100755
> > --- a/test/box-tap/key_def.test.lua
> > +++ b/test/box-tap/key_def.test.lua
> > @@ -4,6 +4,14 @@ local tap = require('tap')
> >  local ffi = require('ffi')
> >  local json = require('json')
> >  local fun = require('fun')
> > +
> > +-- Fix for gh-4252: to prevent invalid trace assembling (see
> > +-- LuaJIT/LuaJIT#584) disable JIT for fun.totable function and
> > +-- method (these functions are different GCfunc objects) and all
> > +-- their subfunctions.
> > +jit.off(fun.totable, true)
> > +jit.off(fun.iter({}).totable, true)
> > +
> >  local key_def_lib = require('key_def')
> >  
> >  local usage_error = 'Bad params, use: key_def.new({' ..
> > -- 
> > 2.25.0
> > 
> 
> ================================================================================
> 
> [PATCH 1/2] test: disable JIT for Lua Fun chain iterator
> 
> JIT compiler can generate an invalid trace for <fun.chain> iterator
> (i.e. chain_gen_r1) breaking its semantics (see LuaJIT/LuaJIT#584).
> Since interpreter works fine and produces the right results, disabling
> JIT for this function stops execution failures.
> 
> As a result box-tap/key_def.test.lua is removed from box-tap suite
> fragile tests list.
> 
> Relates to LuaJIT/LuaJIT#584
> Fixes #4252
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>  test/box-tap/key_def.test.lua | 8 ++++++++
>  test/box-tap/suite.ini        | 1 -
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/test/box-tap/key_def.test.lua b/test/box-tap/key_def.test.lua
> index d7dbf5b88..3a4aad687 100755
> --- a/test/box-tap/key_def.test.lua
> +++ b/test/box-tap/key_def.test.lua
> @@ -4,6 +4,14 @@ local tap = require('tap')
>  local ffi = require('ffi')
>  local json = require('json')
>  local fun = require('fun')
> +
> +-- XXX fix for gh-4252: to prevent invalid trace assembling (see
> +-- https://github.com/LuaJIT/LuaJIT/issues/584) disable JIT for
> +-- <fun.chain> iterator (i.e. <chain_gen_r1>). Since the function
> +-- is local, the dummy chain generator is created to obtain the
> +-- function GC object.
> +jit.off(fun.chain({}).gen)
> +
>  local key_def_lib = require('key_def')
>  
>  local usage_error = 'Bad params, use: key_def.new({' ..
> diff --git a/test/box-tap/suite.ini b/test/box-tap/suite.ini
> index e62399a7e..07f8880b6 100644
> --- a/test/box-tap/suite.ini
> +++ b/test/box-tap/suite.ini
> @@ -5,5 +5,4 @@ is_parallel = True
>  pretest_clean = True
>  use_unix_sockets_iproto = True
>  fragile = cfg.test.lua     ; gh-4344
> -          key_def.test.lua ; gh-4252
>  config = suite.cfg
> -- 
> 2.25.0
> 
> ================================================================================
> 
> -- 
> Best regards,
> IM