From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp17.mail.ru (smtp17.mail.ru [94.100.176.154]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 346D641C5DA for ; Tue, 23 Jun 2020 21:45:43 +0300 (MSK) Date: Tue, 23 Jun 2020 21:45:40 +0300 From: "Alexander V. Tikhonov" Message-ID: <20200623184540.GA6485@hpalx> References: <20200623180408.GH3503@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200623180408.GH3503@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH 1/2] test: disable JIT for Lua Fun totable function List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Munkin Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@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. method) and its subfunctions. However it > doesn't disable JIT for *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 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 > > --- > > 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 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 > --- > 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 > +-- iterator (i.e. ). 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