From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 61A1A666842; Thu, 12 Oct 2023 16:06:44 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 61A1A666842 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1697116004; bh=AZ/mQsWcR7xdtU+9o9/TaINcYp5KLfkfANANiRrs5lk=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=YhIILylATgLDsB6Iu/LCdIXPJiEIajHH1mWF2WmZyvU0VUocYpmBGF+01/su2T7jr Y0x1jmeQOv8cAMrOcJq8f/yWGkdpz9lXBt8feKAbC5ZjytPEK2pAWDtRrZrv9GQtrw aFkrGnl4Gq1ruhkFzDtPr6nCYhFyQRbrXJDRlt+0= Received: from mail-ed1-f50.google.com (mail-ed1-f50.google.com [209.85.208.50]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 45B6C656C30 for ; Thu, 12 Oct 2023 16:06:43 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 45B6C656C30 Received: by mail-ed1-f50.google.com with SMTP id 4fb4d7f45d1cf-53e0d21a4easo1097523a12.1 for ; Thu, 12 Oct 2023 06:06:43 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697116002; x=1697720802; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=yrNe+WcULD4bi0ncyf/WbZtCMpzxt2AIjnsX+0BwEqU=; b=dr9EV8JpYSNVD57Q38Npq8pEgvO3lmPV13tX5dQUnTm9CtYMIY/xgX3Es69DlYI9Dq 049k/KN6HCbQWEASoxGIozqu4aCEf3wlCd8lYcDM3SNgpfFZ20SeNKx+CvzBxCe45tCd X2V6velwMYgos0wL/oLQ2wK9QKYXlV+IZgZysEtLrCzr4vcJDKEMTC3buAzBqHc6gXGk YeikAX8qJR50JWWQFK1nOSYXdqnW6PzHRGzQ/r0G66uqKyc8bLwTMQ9wcT7HMY9cu0VF G0S9JS5Wjt4tkSxTh2L6JwenqJdqiNcQardLS6toteJEN8uo2HFQSv8wISPuLtpTnXCe X96g== X-Gm-Message-State: AOJu0YyOIaqBssAZffoDFnzEOmjl1CnrtkzN0MGZ8wNW/3ztBqHddjwa rcK3qk8YnDtY+diWSPu9BZfkJMYrjXPqYg== X-Google-Smtp-Source: AGHT+IHkaCB4d9w6jme+sevh7Gmzt/PIlGRRJyzz55RDuy6a36NU0uUkDVk1c/6kJNFLgPrsxkYJOQ== X-Received: by 2002:a05:6402:ca2:b0:53d:e016:ef37 with SMTP id cn2-20020a0564020ca200b0053de016ef37mr3745788edb.18.1697116001881; Thu, 12 Oct 2023 06:06:41 -0700 (PDT) Received: from pony.. ([185.6.247.97]) by smtp.gmail.com with ESMTPSA id i22-20020a50d756000000b0053dda7926fcsm2017664edj.60.2023.10.12.06.06.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Oct 2023 06:06:41 -0700 (PDT) To: tarantool-patches@dev.tarantool.org, Sergey Kaplun , max.kokryashkin@gmail.com Date: Thu, 12 Oct 2023 16:06:25 +0300 Message-Id: <8d9d59bf865fe5764a3c91d5a363f7e2bc78a348.1697115768.git.sergeyb@tarantool.org> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH luajit] LJ_GC64: Always snapshot functions for non-base frames. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" From: Sergey Bronnikov Reported by Arseny Vakhrushev. Analysis and fix contributed by Peter Cawley. (cherry picked from commit ff1e72acead01df7d8ed0fbb31efd32f57953618) The problem is GC64-specific and could be reproduced with enabled compiler options LUA_USE_ASSERT and LUA_USE_APICHECK. Sergey Kaplun: * minimized reproducer made by fuzzing test Sergey Bronnikov: * added the description (see a comment in the test) * added tests for the problem: first one based on the original reproducer and second one based on a reproducer made by fuzzing test. Part of tarantool/tarantool#8825 --- Branch: https://github.com/tarantool/luajit/commits/ligurio/lj-611-always-snapshot-functions-for-non-base-frames PR: https://github.com/tarantool/tarantool/pull/9254 LJ issue: https://github.com/LuaJIT/LuaJIT/issues/611 src/lj_record.c | 1 + src/lj_snap.c | 9 +++- ...t-functions-for-non-base-frames-1.test.lua | 36 +++++++++++++++ ...hot-functions-for-non-base-frames.test.lua | 45 +++++++++++++++++++ 4 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 test/tarantool-tests/lj-611-always-snapshot-functions-for-non-base-frames-1.test.lua create mode 100644 test/tarantool-tests/lj-611-always-snapshot-functions-for-non-base-frames.test.lua diff --git a/src/lj_record.c b/src/lj_record.c index 48a5481b..55785e23 100644 --- a/src/lj_record.c +++ b/src/lj_record.c @@ -211,6 +211,7 @@ static TRef getcurrf(jit_State *J) { if (J->base[-1-LJ_FR2]) return J->base[-1-LJ_FR2]; + /* Non-base frame functions ought to be loaded already. */ lj_assertJ(J->baseslot == 1+LJ_FR2, "bad baseslot"); return sloadt(J, -1-LJ_FR2, IRT_FUNC, IRSLOAD_READONLY); } diff --git a/src/lj_snap.c b/src/lj_snap.c index 6c5e5e53..06ae17eb 100644 --- a/src/lj_snap.c +++ b/src/lj_snap.c @@ -85,8 +85,13 @@ static MSize snapshot_slots(jit_State *J, SnapEntry *map, BCReg nslots) IRIns *ir = &J->cur.ir[ref]; if ((LJ_FR2 || !(sn & (SNAP_CONT|SNAP_FRAME))) && ir->o == IR_SLOAD && ir->op1 == s && ref > retf) { - /* No need to snapshot unmodified non-inherited slots. */ - if (!(ir->op2 & IRSLOAD_INHERIT)) + /* + ** No need to snapshot unmodified non-inherited slots. + ** But always snapshot the function below a frame in LJ_FR2 mode. + */ + if (!(ir->op2 & IRSLOAD_INHERIT) && + (!LJ_FR2 || s == 0 || s+1 == nslots || + !(J->slot[s+1] & (TREF_CONT|TREF_FRAME)))) continue; /* No need to restore readonly slots and unmodified non-parent slots. */ if (!(LJ_DUALNUM && (ir->op2 & IRSLOAD_CONVERT)) && diff --git a/test/tarantool-tests/lj-611-always-snapshot-functions-for-non-base-frames-1.test.lua b/test/tarantool-tests/lj-611-always-snapshot-functions-for-non-base-frames-1.test.lua new file mode 100644 index 00000000..759c2862 --- /dev/null +++ b/test/tarantool-tests/lj-611-always-snapshot-functions-for-non-base-frames-1.test.lua @@ -0,0 +1,36 @@ +local tap = require('tap') +local test = tap.test('lj-611-always-snapshot-functions-for-non-base-frames-1'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +-- GC64: Function missing in snapshot for non-base frame +-- https://github.com/LuaJIT/LuaJIT/issues/611 + +test:plan(1) + +jit.opt.start('hotloop=1', 'hotexit=1') + +local inner_counter = 0 +local SIDE_START = 1 +-- Lower frame to return from `inner()` function side trace. +-- TODO: Give a reason for vararg func. +local function lower_frame(...) + local inner = function() + if inner_counter > SIDE_START then + return + end + inner_counter = inner_counter + 1 + end + inner(..., inner(inner())) +end + +-- Compile `inner()` function. +lower_frame() +lower_frame() +-- Compile hotexit +lower_frame() +-- Take side exit from side trace. +lower_frame(1) + +test:ok(true, 'function is present in snapshot') +test:done(true) diff --git a/test/tarantool-tests/lj-611-always-snapshot-functions-for-non-base-frames.test.lua b/test/tarantool-tests/lj-611-always-snapshot-functions-for-non-base-frames.test.lua new file mode 100644 index 00000000..7305c185 --- /dev/null +++ b/test/tarantool-tests/lj-611-always-snapshot-functions-for-non-base-frames.test.lua @@ -0,0 +1,45 @@ +local tap = require('tap') +local test = tap.test('lj-611-always-snapshot-functions-for-non-base-frames'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(1) + +jit.opt.start('hotloop=1', 'hotexit=1') + +-- Test reproduces a bug "GC64: Function missing in snapshot for non-base +-- frame" [1], and based on reproducer described in [2]. +-- +-- [1]: https://github.com/LuaJIT/LuaJIT/issues/611 +-- [2]: https://github.com/LuaJIT/LuaJIT/issues/611#issuecomment-679228156 +-- +-- Function `outer` is recorded to a trace and calls a builtin function that is +-- not JIT-compilable and therefore triggers exit to interpreter, and then it +-- resumes tracing just after the call returns - this is a trace stitching. +-- Then, within the call, we need the potential for a side trace. Finally, we need +-- that side exit to be taken enough for the exit to be compiled into a trace. +-- The loop at the bottom has enough iterations to trigger JIT compilation, and +-- enough more on top on trigger compilation of the not case. Compilation of +-- this case hits the assertion failure. + +local inner +for _ = 1, 3 do + inner = function(_, i) + return i < 4 + end +end + +local function outer(i) + -- The function `string.gsub` is not JIT-compilable and triggers a trace + -- exit. For example, `string.gmatch` and `string.match` are suitable as + -- well. + -- See https://github.com/tarantool/tarantool/wiki/LuaJIT-Not-Yet-Implemented. + inner(string.gsub('', '', ''), i) +end + +for i = 1, 4 do + outer(i) +end + +test:ok(true, 'function is present in snapshot') +test:done(true) -- 2.34.1