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 32B2D14020C0; Thu, 12 Jun 2025 12:12:15 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 32B2D14020C0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1749719535; bh=k1db9Z/AbG0PB8NYgtbEU2WOd4swFaMFUgc29lGUYMM=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=I+we3L6tTXiDYL/VUwcOMFm1qdbIFRYvEIeeS6NEQk4Vnd+5GO/cKhkTH8AuZU5Ci S0dmn7EvSt9YJQ6RcfAzo+yZMlo9cVN2FVveSV46TfvxKfkjVQU9dfMjVPy3IuMiH9 /3AHY5kbmPZjJtRjwldXril1a8oJNxtoqv0T1+jo= Received: from send83.i.mail.ru (send83.i.mail.ru [89.221.237.178]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 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 59472461E01 for ; Thu, 12 Jun 2025 12:12:13 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 59472461E01 Received: by exim-smtp-85b97957d7-p8j8t with esmtpa (envelope-from ) id 1uPdym-000000006Lt-0n6s; Thu, 12 Jun 2025 12:12:12 +0300 To: Sergey Bronnikov Date: Thu, 12 Jun 2025 12:12:16 +0300 Message-ID: <20250612091216.29838-1-skaplun@tarantool.org> X-Mailer: git-send-email 2.49.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD909B82214CC988917ED8DD2AD0A6FDDF3DFA67D44F41557AA00894C459B0CD1B9AFF365A377A035C02EB5D77EF37489D1C01E68157206F8A9AFC76E3EFD022FF4505DB70707ED1B7C X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7011EB7026DD4A9BAEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB553375668906014A252870FC3E13B579211B0CA3D2352E4867F944A9AE6D1365DA50E574389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C06030C3405640F6718941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B6E232F00D8D26902CA471835C12D1D977C4224003CC8364762BB6847A3DEAEFB0F43C7A68FF6260569E8FC8737B5C2249EC8D19AE6D49635B68655334FD4449CB9ECD01F8117BC8BEAAAE862A0553A39223F8577A6DFFEA7CB0EC3B1FCAE4A06943847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: 0D63561A33F958A534C105431384A6815002B1117B3ED6969977F2A0D5106865B91D2EB2DEE3878C823CB91A9FED034534781492E4B8EEAD2E48F5DFA0C1F120C79554A2A72441328621D336A7BC284946AD531847A6065A535571D14F44ED41 X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742DC8270968E61249B1004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D3419891600CCEF4607451CDFF5E481F8096D4CBB033FD3C0E7053B3A72674FE4948C4C9C13929F863B1D7E09C32AA3244C9195A7683172909E77DD89D51EBB7742A559C4298E60EBF8EA455F16B58544A2557BDE0DD54B3590A5AE236DF995FB59829709634694AABAED6A17656DB59BCAD427812AF56FC65B X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXMZebaIdHP2ghjoIc/363UZI6Kf1ptIMVSykAyseJQ6/+Dj6WArYNYo= X-DA7885C5: 283C28FFEEE7C79AF255D290C0D534F972DB46C661455851D9C1001659F40F79DD3F1CB3BA9320A65B1A4C17EAA7BC4BEF2421ABFA55128DAF83EF9164C44C7E X-Mailru-Sender: 689FA8AB762F7393FE9E42A757851DB6FF7E83B04CA6C03C040CE5EDE0960721872EEA3E48A191D9E49D44BB4BD9522A059A1ED8796F048DB274557F927329BE89D5A3BC2B10C37545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] Avoid out-of-range PC for stack overflow error from snapshot restore. 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 Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" From: Mike Pall Reported by Sergey Kaplun. (cherry picked from commit cd4af8ad80bb6430ad2e547f7af236268c9be7d9) When restoring a snapshot because of exiting by the check of the stack, the snapshot from the parent trace is used. For the correct error message, the snapshot map uses the next bytecode after the stored one. In case, when the PC in the snapshot is BC_RET* (the last bytecode in the prototype), there is no next value to be referenced, so this approach leads to the assertion failure in `lj_debug_framepc()`. This patch prevents this behaviour by checking the original bytecode and avoiding PC adjusting in the case of the last bytecode in the prototype. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#11278 --- Related issues: * https://github.com/LuaJIT/LuaJIT/issues/1359 * https://github.com/tarantool/tarantool/issues/11278 Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1359-bad-pc-on-snap-restore-stackov src/lj_bc.h | 5 ++ src/lj_parse.c | 14 +---- src/lj_snap.c | 6 ++- ...59-bad-pc-on-snap-restore-stackov.test.lua | 51 +++++++++++++++++++ 4 files changed, 61 insertions(+), 15 deletions(-) create mode 100644 test/tarantool-tests/lj-1359-bad-pc-on-snap-restore-stackov.test.lua diff --git a/src/lj_bc.h b/src/lj_bc.h index 69a45f28..2196dbed 100644 --- a/src/lj_bc.h +++ b/src/lj_bc.h @@ -259,6 +259,11 @@ static LJ_AINLINE int bc_isret(BCOp op) return (op == BC_RETM || op == BC_RET || op == BC_RET0 || op == BC_RET1); } +static LJ_AINLINE int bc_isret_or_tail(BCOp op) +{ + return (op == BC_CALLMT || op == BC_CALLT || bc_isret(op)); +} + LJ_DATA const uint16_t lj_bc_mode[]; LJ_DATA const uint16_t lj_bc_ofs[]; diff --git a/src/lj_parse.c b/src/lj_parse.c index ec85ac9b..88108c01 100644 --- a/src/lj_parse.c +++ b/src/lj_parse.c @@ -1519,23 +1519,11 @@ static void fs_fixup_var(LexState *ls, GCproto *pt, uint8_t *p, size_t ofsvar) #endif -/* Check if bytecode op returns. */ -static int bcopisret(BCOp op) -{ - switch (op) { - case BC_CALLMT: case BC_CALLT: - case BC_RETM: case BC_RET: case BC_RET0: case BC_RET1: - return 1; - default: - return 0; - } -} - /* Fixup return instruction for prototype. */ static void fs_fixup_ret(FuncState *fs) { BCPos lastpc = fs->pc; - if (lastpc <= fs->lasttarget || !bcopisret(bc_op(fs->bcbase[lastpc-1].ins))) { + if (lastpc <= fs->lasttarget || !bc_isret_or_tail(bc_op(fs->bcbase[lastpc-1].ins))) { if ((fs->bl->flags & FSCOPE_UPVAL)) bcemit_AJ(fs, BC_UCLO, 0, 0); bcemit_AD(fs, BC_RET0, 0, 1); /* Need final return. */ diff --git a/src/lj_snap.c b/src/lj_snap.c index 4cfae579..3542a46c 100644 --- a/src/lj_snap.c +++ b/src/lj_snap.c @@ -954,8 +954,10 @@ const BCIns *lj_snap_restore(jit_State *J, void *exptr) const BCIns *pc = snap_pc(&map[nent]); lua_State *L = J->L; - /* Set interpreter PC to the next PC to get correct error messages. */ - setcframe_pc(L->cframe, pc+1); + /* Set interpreter PC to the next PC to get correct error messages. + ** But not for returns or tail calls, since pc+1 may be out-of-range. + */ + setcframe_pc(L->cframe, bc_isret_or_tail(bc_op(*pc)) ? pc : pc+1); setcframe_pc(cframe_raw(cframe_prev(L->cframe)), pc); /* Make sure the stack is big enough for the slots from the snapshot. */ diff --git a/test/tarantool-tests/lj-1359-bad-pc-on-snap-restore-stackov.test.lua b/test/tarantool-tests/lj-1359-bad-pc-on-snap-restore-stackov.test.lua new file mode 100644 index 00000000..d09b5c42 --- /dev/null +++ b/test/tarantool-tests/lj-1359-bad-pc-on-snap-restore-stackov.test.lua @@ -0,0 +1,51 @@ +local tap = require('tap') + +-- The test file to demonstrate the LuaJIT misbehaviour during +-- stack overflow on snapshot restoration for the last bytecode in +-- the prototype. +-- See also: https://github.com/LuaJIT/LuaJIT/issues/1359. + +local test = tap.test('lj-1359-bad-pc-on-snap-restore-stackov'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(2) + +jit.opt.start('hotloop=1') + +-- When restoring a snapshot because of exiting by the check of +-- the stack, the snapshot from the parent trace is used. For the +-- correct error message, the snapshot map uses the next bytecode +-- after the stored one. In case, when the PC in the snapshot is +-- BC_RET* (the last bytecode in the prototype), there is no next +-- value to be referenced, so this approach leads to the assertion +-- failure in lj_debug_framepc(). + +local counter = 0 +-- The second trace started from the side exit by the counter. +-- It ends by entering the first trace. +local function func_side_trace() + if counter > 5 then + -- This RET0 BC is the first recorded for the second trace. + -- The stack check for the child trace uses the first snapshot + -- from the parent trace. + return + end + counter = counter + 1; +end + +-- The trace with up-recursion for the function that causes stack +-- overflow. It is recorded first and inlines func_side_trace. +local function stackov_f() + local r = stackov_f(func_side_trace()) + return r +end + +local result, errmsg = pcall(stackov_f) + +-- There is no assertion failure if we are here. +-- Just sanity checks. +test:ok(not result, 'correct status') +test:like(errmsg, 'stack overflow', 'correct error message') + +test:done(true) -- 2.49.0