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 73FB313D630F; Tue, 10 Jun 2025 19:15:14 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 73FB313D630F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1749572114; bh=RygRjgw4DMhhOB6/jo5ZsOQdc4yBXwzQ8CWxcX0971Y=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=XZsDOXoDorWU6NIOfA83ecCO/wCG5DxO/da3qerMIvqiKQQmnFXyr5icbgZ2nMDA5 +zD4rWhFf8lruVuXOFbVsTPIGeSqHZ66Z11hoOiVqjCzklFNUxKlL7ClfgnfE+/tmr Szqz4zIqavMepeYser2pwYudgccMLJN/KVD1nd1s= Received: from send195.i.mail.ru (send195.i.mail.ru [95.163.59.34]) (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 F1EED13D6309 for ; Tue, 10 Jun 2025 19:15:12 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org F1EED13D6309 Received: by exim-smtp-85b97957d7-q9hz4 with esmtpa (envelope-from ) id 1uP1d1-00000000JZ1-3iRL; Tue, 10 Jun 2025 19:15:12 +0300 Content-Type: multipart/alternative; boundary="------------m0b6Iuhjp2LrZ9b7F84Lxdbp" Message-ID: <9d1c7b94-b18d-4290-b017-27ac9e4f6ef2@tarantool.org> Date: Tue, 10 Jun 2025 19:15:11 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org References: <1152e27d618a4717c0f48cb77d085434eb183b18.1749550966.git.skaplun@tarantool.org> In-Reply-To: <1152e27d618a4717c0f48cb77d085434eb183b18.1749550966.git.skaplun@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD909B82214CC98891766BB5904FF1352BA1E8CF8C999985CDB00894C459B0CD1B953C8E80BA8CCF2888E7FD94675F3356514346426CF95B9043C852ED1AEF80621EB35CB8B4FE073DF X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE721AF84DC1D70954DEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB553375668906014A252870FC220A0A51BADBAA628B7567F37509CCC8B31BF1D5684A21AB389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C078FCF50C7EAF9C588941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B6E5E764EB5D94DBD4CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB8D32BA5DBAC0009BE9E8FC8737B5C224958C1606C78F2434E76E601842F6C81A12EF20D2F80756B5FB606B96278B59C4276E601842F6C81A127C277FBC8AE2E8B6A4E49BB0F3BA1413AA81AA40904B5D99C9F4D5AE37F343AD1F44FA8B9022EA23BBE47FD9DD3FB595F5C1EE8F4F765FC72CEEB2601E22B093A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4930A3850AC1BE2E735B17145F0B7815491C4224003CC83647689D4C264860C145E X-C1DE0DAB: 0D63561A33F958A5B57D066C790928545002B1117B3ED696CDE7F2829BB72914A9DAB4B68AE4D22F823CB91A9FED034534781492E4B8EEAD2B25D9E4C92BC8ACBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF1BEE1A53B7AE4E311C5A200BC92533776DC1AC4F411AEEFBE685BF5BA4DBA2AA2E7DF6377D6407AA1D3122CD40B080D214E11F9D33815312F624F144AE1C0646C6312566230BB7D4111DC66A97D0BFE2913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXMZebaIdHP2ghjoIc/363UZI6Kf1ptIMVSykAyseJQ6/NRkQeBnTkzU= X-Mailru-Sender: 520A125C2F17F0B1E52FEF5D219D614053C8E80BA8CCF2888E7FD94675F33565C40A896BBB25DA880152A3D17938EB451EB5A0BCEC6A560B3DDE9B364B0DF289BE2DA36745F2EEB5CEBA01FB949A1F1EEAB4BC95F72C04283CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 2/2] Different fix for partial snapshot restore due to stack overflow. 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" This is a multi-part message in MIME format. --------------m0b6Iuhjp2LrZ9b7F84Lxdbp Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hello, Sergey! Thanks for the patch! See a comment below. Sergey On 6/10/25 13:28, Sergey Kaplun wrote: > From: Mike Pall > > Reported by Junlong Li. Fixed by Peter Cawley. > > (cherry picked from commit 86e7123bb1782a5f200ba5e83b8c4f3fbad4f7bc) > > This patch is a follow-up to the previous commit, which leads to a dirty > read of the pseudo-valid PC set for the cframe on snapshot restoration. > To avoid these dirty reads, this patch sets the PC to the outer frame > as well before possible error throwing. > > Sergey Kaplun: > * added the description and the test for the problem > > Part of tarantool/tarantool#11278 > --- > src/lj_snap.c | 3 +- > src/lj_trace.c | 4 +- > ...-1196-stack-overflow-snap-restore.test.lua | 65 +++++++++++++++++++ > 3 files changed, 68 insertions(+), 4 deletions(-) > create mode 100644 test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua > > diff --git a/src/lj_snap.c b/src/lj_snap.c > index 8d7bd868..4cfae579 100644 > --- a/src/lj_snap.c > +++ b/src/lj_snap.c > @@ -955,7 +955,8 @@ const BCIns *lj_snap_restore(jit_State *J, void *exptr) > lua_State *L = J->L; > > /* Set interpreter PC to the next PC to get correct error messages. */ > - setcframe_pc(cframe_raw(L->cframe), pc+1); > + setcframe_pc(L->cframe, pc+1); > + setcframe_pc(cframe_raw(cframe_prev(L->cframe)), pc); > > /* Make sure the stack is big enough for the slots from the snapshot. */ > if (LJ_UNLIKELY(L->base + snap->topslot >= tvref(L->maxstack))) { > diff --git a/src/lj_trace.c b/src/lj_trace.c > index 8a18d3cf..0d1d233a 100644 > --- a/src/lj_trace.c > +++ b/src/lj_trace.c > @@ -909,10 +909,8 @@ int LJ_FASTCALL lj_trace_exit(jit_State *J, void *exptr) > exd.J = J; > exd.exptr = exptr; > errcode = lj_vm_cpcall(L, NULL, &exd, trace_exit_cp); > - if (errcode) { > - setcframe_pc(cframe_raw(L->cframe), L); /* Point to any valid memory. */ > + if (errcode) > return -errcode; /* Return negated error code. */ > - } > > if (exitcode) copyTV(L, L->top++, &exiterr); /* Anchor the error object. */ > > diff --git a/test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua b/test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua > new file mode 100644 > index 00000000..942d1f82 > --- /dev/null > +++ b/test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua > @@ -0,0 +1,65 @@ > +local tap = require('tap') > + > +-- Test file to demonstrate LuaJIT dirty reads after stack > +-- overflow during restoration from the snapshot. > +-- The test fails before the patch under Valgrind. Please specify valgrind option that is required for reproducing the bug. Cannot reproduce with command line below with reverted patch: VALGRIND_OPTS="--leak-check=no --malloc-fill=0x00 --free-fill=0x00" ctest -V -R test/tarantool-tests/lj-1196-partial-snap-restore.test.lua -V > +-- > +-- luacheck: push no max_comment_line_length > +-- > +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1196, > +--https://www.freelists.org/post/luajit/Invalid-read-found-by-valgrind. > +-- > +-- luacheck: pop > + > +local test = tap.test('lj-1196-stack-overflow-snap-restore') > + > +test:plan(4) > + > +-- XXX: This file has the same tests as the > +-- , but without disabling the > +-- compilation for the given functions. Hence, the check here is > +-- less strict -- we just check that there are no dirty reads, > +-- uninitialized memory access, etc. > + > +local function recursive_f_noarg() > + recursive_f_noarg() > +end > + > +local function recursive_one_arg(argument) > + recursive_one_arg(argument) > +end > + > +local function recursive_f_vararg(...) > + recursive_f_vararg(1, ...) > +end > + > +local function recursive_f_vararg_tail(...) > + return recursive_f_vararg_tail(1, ...) > +end > + > +-- Use `coroutine.wrap()`, for independent stack sizes. > +-- The invalid read is done by the error handler > +-- `debug.traceback()`, since it observes the pseudo PC (`L`) and > +-- reads the memory by `L - 4` address before the patch. > + > +coroutine.wrap(function() > + local status = xpcall(recursive_f_noarg, debug.traceback) > +test:ok(not status, 'correct status, recursive no arguments') > +end)() > + > +coroutine.wrap(function() > + local status = xpcall(recursive_one_arg, debug.traceback, 1) > +test:ok(not status, 'correct status, recursive one argument') > +end)() > + > +coroutine.wrap(function() > + local status = xpcall(recursive_f_vararg, debug.traceback, 1) > +test:ok(not status, 'correct status, recursive vararg') > +end)() > + > +coroutine.wrap(function() > + local status = xpcall(recursive_f_vararg_tail, debug.traceback, 1) > +test:ok(not status, 'correct status, recursive vararg tail') > +end)() > + > +test:done(true) --------------m0b6Iuhjp2LrZ9b7F84Lxdbp Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit

Hello, Sergey!

Thanks for the patch! See a comment below.

Sergey

On 6/10/25 13:28, Sergey Kaplun wrote:
From: Mike Pall <mike>

Reported by Junlong Li. Fixed by Peter Cawley.

(cherry picked from commit 86e7123bb1782a5f200ba5e83b8c4f3fbad4f7bc)

This patch is a follow-up to the previous commit, which leads to a dirty
read of the pseudo-valid PC set for the cframe on snapshot restoration.
To avoid these dirty reads, this patch sets the PC to the outer frame
as well before possible error throwing.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#11278
---
 src/lj_snap.c                                 |  3 +-
 src/lj_trace.c                                |  4 +-
 ...-1196-stack-overflow-snap-restore.test.lua | 65 +++++++++++++++++++
 3 files changed, 68 insertions(+), 4 deletions(-)
 create mode 100644 test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua

diff --git a/src/lj_snap.c b/src/lj_snap.c
index 8d7bd868..4cfae579 100644
--- a/src/lj_snap.c
+++ b/src/lj_snap.c
@@ -955,7 +955,8 @@ const BCIns *lj_snap_restore(jit_State *J, void *exptr)
   lua_State *L = J->L;
 
   /* Set interpreter PC to the next PC to get correct error messages. */
-  setcframe_pc(cframe_raw(L->cframe), pc+1);
+  setcframe_pc(L->cframe, pc+1);
+  setcframe_pc(cframe_raw(cframe_prev(L->cframe)), pc);
 
   /* Make sure the stack is big enough for the slots from the snapshot. */
   if (LJ_UNLIKELY(L->base + snap->topslot >= tvref(L->maxstack))) {
diff --git a/src/lj_trace.c b/src/lj_trace.c
index 8a18d3cf..0d1d233a 100644
--- a/src/lj_trace.c
+++ b/src/lj_trace.c
@@ -909,10 +909,8 @@ int LJ_FASTCALL lj_trace_exit(jit_State *J, void *exptr)
   exd.J = J;
   exd.exptr = exptr;
   errcode = lj_vm_cpcall(L, NULL, &exd, trace_exit_cp);
-  if (errcode) {
-    setcframe_pc(cframe_raw(L->cframe), L);  /* Point to any valid memory. */
+  if (errcode)
     return -errcode;  /* Return negated error code. */
-  }
 
   if (exitcode) copyTV(L, L->top++, &exiterr);  /* Anchor the error object. */
 
diff --git a/test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua b/test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua
new file mode 100644
index 00000000..942d1f82
--- /dev/null
+++ b/test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua
@@ -0,0 +1,65 @@
+local tap = require('tap')
+
+-- Test file to demonstrate LuaJIT dirty reads after stack
+-- overflow during restoration from the snapshot.
+-- The test fails before the patch under Valgrind.

Please specify valgrind option that is required for reproducing the bug.

Cannot reproduce with command line below with reverted patch:

VALGRIND_OPTS="--leak-check=no --malloc-fill=0x00 --free-fill=0x00" ctest -V -R test/tarantool-tests/lj-1196-partial-snap-restore.test.lua -V

+--
+-- luacheck: push no max_comment_line_length
+--
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1196,
+-- https://www.freelists.org/post/luajit/Invalid-read-found-by-valgrind.
+--
+-- luacheck: pop
+
+local test = tap.test('lj-1196-stack-overflow-snap-restore')
+
+test:plan(4)
+
+-- XXX: This file has the same tests as the
+-- <test/LuaJIT-tests/lang/stackov.lua>, but without disabling the
+-- compilation for the given functions. Hence, the check here is
+-- less strict -- we just check that there are no dirty reads,
+-- uninitialized memory access, etc.
+
+local function recursive_f_noarg()
+  recursive_f_noarg()
+end
+
+local function recursive_one_arg(argument)
+  recursive_one_arg(argument)
+end
+
+local function recursive_f_vararg(...)
+  recursive_f_vararg(1, ...)
+end
+
+local function recursive_f_vararg_tail(...)
+  return recursive_f_vararg_tail(1, ...)
+end
+
+-- Use `coroutine.wrap()`, for independent stack sizes.
+-- The invalid read is done by the error handler
+-- `debug.traceback()`, since it observes the pseudo PC (`L`) and
+-- reads the memory by `L - 4` address before the patch.
+
+coroutine.wrap(function()
+  local status = xpcall(recursive_f_noarg, debug.traceback)
+  test:ok(not status, 'correct status, recursive no arguments')
+end)()
+
+coroutine.wrap(function()
+  local status = xpcall(recursive_one_arg, debug.traceback, 1)
+  test:ok(not status, 'correct status, recursive one argument')
+end)()
+
+coroutine.wrap(function()
+  local status = xpcall(recursive_f_vararg, debug.traceback, 1)
+  test:ok(not status, 'correct status, recursive vararg')
+end)()
+
+coroutine.wrap(function()
+  local status = xpcall(recursive_f_vararg_tail, debug.traceback, 1)
+  test:ok(not status, 'correct status, recursive vararg tail')
+end)()
+
+test:done(true)
--------------m0b6Iuhjp2LrZ9b7F84Lxdbp--