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 8ECC2C9084E; Tue, 17 Sep 2024 10:45:10 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8ECC2C9084E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1726559110; bh=0hoArZQb3z16qAC6HUdJcBqdvjAWkTDegn0zMcSdsjI=; 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=Yii6wY3I0M+NQ2XWbuCnWTRxRvcdQhEZJrW5k6I3LHaLN1GWPH7s4EKLyBYjQext7 zeat6g3v8u37IF2uKNlBT6MT6XRm0iXVvQIMy9S6rL0e4JYk+iSsvOr1iIrrxKtTrf jPDVnqRM0iTVQ+xjdyVlfBoZ3DDYtR4GkCazT3mQ= Received: from smtp49.i.mail.ru (smtp49.i.mail.ru [95.163.41.91]) (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 77A88C9084E for ; Tue, 17 Sep 2024 10:45:09 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 77A88C9084E Received: by smtp49.i.mail.ru with esmtpa (envelope-from ) id 1sqStX-00000006zz0-1jl3; Tue, 17 Sep 2024 10:45:08 +0300 Content-Type: multipart/alternative; boundary="------------u5w9esHNV0lDZq3NO7NtgB5s" Message-ID: Date: Tue, 17 Sep 2024 10:45:07 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Sergey Kaplun , Maxim Kokryashkin Cc: tarantool-patches@dev.tarantool.org References: <20240910140509.32724-1-skaplun@tarantool.org> In-Reply-To: <20240910140509.32724-1-skaplun@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD921D24E727BB09B718ADDA9D4F4654BE54F049D65236CD9B4182A05F5380850402D5C7D57BACA1EE52EB5D77EF37489D116C3E9210C2DD1F806494F9BF01AD1A84787F701E017BF3D X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE75C0AD7D016C066E3C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE796563325B6E011C88F08D7030A58E5AD1A62830130A00468AEEEE3FBA3A834EE7353EFBB5533756612F0191129D345364D6BEA5DE2350C1FC2DBEF4130DA04B0D6CB9B54B326CAB3389733CBF5DBD5E913377AFFFEAFD269A417C69337E82CC2CC7F00164DA146DAFE8445B8C89999729449624AB7ADAF37F6B57BC7E64490611E7FA7ABCAF51C92176DF2183F8FC7C000E2D00546020E658941B15DA834481F9449624AB7ADAF37BA3038C0950A5D3613377AFFFEAFD269176DF2183F8FC7C08B62274D4421ED347B076A6E789B0E97A8DF7F3B2552694AD5FFEEA1DED7F25D49FD398EE364050F26055571C92BF10FC43C4D2AE8146675B3661434B16C20ACC84D3B47A649675FE827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8BD9166E38712C78CA75ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: 0D63561A33F958A507340E5B1FF351D85002B1117B3ED6964FC2DDB1A2146F0C03803A57F48E4E5A823CB91A9FED034534781492E4B8EEADADEF88395FA75C5FBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742424CF958EAFF5D571004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D34D16EA493CC1FD9F875DA819ADD0B6F464C71B4455CC4EE2C8C445CC98E1CE7EB2CC981BECE039E741D7E09C32AA3244C25778CCCC579B3A9A0CBE312C330D7D55D63EAD813C52B0CEA455F16B58544A2557BDE0DD54B3590A5AE236DF995FB59978A700BF655EAEEED6A17656DB59BCAD427812AF56FC65B X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojK9mC3H63xFbIEKo32EM/GQ== X-Mailru-Sender: 520A125C2F17F0B1E52FEF5D219D6140C1576F6BF3F525D3FA3B773892146865E8D94001664F71B80152A3D17938EB451EB5A0BCEC6A560B3DDE9B364B0DF289BE2DA36745F2EEB5CEBA01FB949A1F1EEAB4BC95F72C04283CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] Fix stack allocation after on-trace stack check. 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. --------------u5w9esHNV0lDZq3NO7NtgB5s Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi, Sergey, thanks for the patch! LGTM On 10.09.2024 17:05, Sergey Kaplun wrote: > From: Mike Pall > > (cherry picked from commit 204cee2c917f55f288c0b166742e56c134fe578c) > > It is possible that a snapshot topslot is less than the possible topslot > of the Lua stack. In that case, if the Lua stack overflows in > `lj_vmevent_prepare()`, the error is raised inside > `lj_vm_exit_handler()`, which has no corresponding DWARF eh_frame [1], > so it leads to the crash. > > This patch fix-ups the topslot of the snapshot on trace exit to the > maximum possible one. > > Sergey Kaplun: > * added the description and the test for the problem > > [1]:https://refspecs.linuxfoundation.org/LSB_3.0.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html > > Part of tarantool/tarantool#10199 > --- > > Branch:https://github.com/tarantool/luajit/tree/skaplun/fix-stack-alloc-on-trace > Issue:https://github.com/tarantool/tarantool/issues/10199 > > src/lj_trace.c | 6 ++- > .../fix-stack-alloc-on-trace-exit.test.lua | 53 +++++++++++++++++++ > 2 files changed, 58 insertions(+), 1 deletion(-) > create mode 100644 test/tarantool-tests/fix-stack-alloc-on-trace-exit.test.lua > > diff --git a/src/lj_trace.c b/src/lj_trace.c > index 20014ecb..94cb27e5 100644 > --- a/src/lj_trace.c > +++ b/src/lj_trace.c > @@ -522,7 +522,11 @@ static void trace_stop(jit_State *J) > lj_assertJ(J->parent != 0 && J->cur.root != 0, "not a side trace"); > lj_asm_patchexit(J, traceref(J, J->parent), J->exitno, J->cur.mcode); > /* Avoid compiling a side trace twice (stack resizing uses parent exit). */ > - traceref(J, J->parent)->snap[J->exitno].count = SNAPCOUNT_DONE; > + { > + SnapShot *snap = &traceref(J, J->parent)->snap[J->exitno]; > + snap->count = SNAPCOUNT_DONE; > + if (J->cur.topslot > snap->topslot) snap->topslot = J->cur.topslot; > + } > /* Add to side trace chain in root trace. */ > { > GCtrace *root = traceref(J, J->cur.root); > diff --git a/test/tarantool-tests/fix-stack-alloc-on-trace-exit.test.lua b/test/tarantool-tests/fix-stack-alloc-on-trace-exit.test.lua > new file mode 100644 > index 00000000..ca04e54e > --- /dev/null > +++ b/test/tarantool-tests/fix-stack-alloc-on-trace-exit.test.lua > @@ -0,0 +1,53 @@ > +local tap = require('tap') > + > +-- Test file to demonstrate incorrect Lua stack restoration on > +-- exit from trace by the stack overflow. > + > +local test = tap.test('fix-stack-alloc-on-trace-exit'):skipcond({ > + ['Test requires JIT enabled'] = not jit.status(), > +}) > + > +local jit_dump = require('jit.dump') > + > +test:plan(2) > + > +-- Before the patch, it is possible that a snapshot topslot is > +-- less than the possible topslot of the Lua stack. In that case, > +-- if the Lua stack overflows in `lj_vmevent_prepare()`, the error > +-- is raised inside `lj_vm_exit_handler()`, which has no > +-- corresponding DWARF eh_frame, so it leads to the crash. > + > +-- Need for the stack growing in `lj_vmevent_prepare`. > +jit_dump.start('x', '/dev/null') > + > +-- Create a coroutine with a fixed stack size. > +local coro = coroutine.create(function() > + jit.opt.start('hotloop=1', 'hotexit=1', 'callunroll=1') > + > + -- `math.modf` recording is NYI. > + -- Local `math_modf` simplifies `jit.dump()` output. > + local math_modf = math.modf > + > + local function trace(n) > + n = n + 1 > + -- luacheck: ignore > + -- Start a side trace here. > + if n % 2 == 0 then end > + -- Stop the recording of the side trace and a main trace, > + -- stitching. > + math_modf(1, 1) > + -- Grow stack, avoid tail calls. > + local unused = trace(n) > + return unused > + end > + > + local n = 0 > + trace(n) > +end) > + > +local result, errmsg = coroutine.resume(coro) > + > +test:ok(not result, 'correct status and no crash') > +test:like(errmsg, 'stack overflow', 'correct error message') > + > +test:done(true) --------------u5w9esHNV0lDZq3NO7NtgB5s Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit

Hi, Sergey,

thanks for the patch! LGTM

On 10.09.2024 17:05, Sergey Kaplun wrote:
From: Mike Pall <mike>

(cherry picked from commit 204cee2c917f55f288c0b166742e56c134fe578c)

It is possible that a snapshot topslot is less than the possible topslot
of the Lua stack. In that case, if the Lua stack overflows in
`lj_vmevent_prepare()`, the error is raised inside
`lj_vm_exit_handler()`, which has no corresponding DWARF eh_frame [1],
so it leads to the crash.

This patch fix-ups the topslot of the snapshot on trace exit to the
maximum possible one.

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

[1]: https://refspecs.linuxfoundation.org/LSB_3.0.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html

Part of tarantool/tarantool#10199
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/fix-stack-alloc-on-trace
Issue: https://github.com/tarantool/tarantool/issues/10199

 src/lj_trace.c                                |  6 ++-
 .../fix-stack-alloc-on-trace-exit.test.lua    | 53 +++++++++++++++++++
 2 files changed, 58 insertions(+), 1 deletion(-)
 create mode 100644 test/tarantool-tests/fix-stack-alloc-on-trace-exit.test.lua

diff --git a/src/lj_trace.c b/src/lj_trace.c
index 20014ecb..94cb27e5 100644
--- a/src/lj_trace.c
+++ b/src/lj_trace.c
@@ -522,7 +522,11 @@ static void trace_stop(jit_State *J)
     lj_assertJ(J->parent != 0 && J->cur.root != 0, "not a side trace");
     lj_asm_patchexit(J, traceref(J, J->parent), J->exitno, J->cur.mcode);
     /* Avoid compiling a side trace twice (stack resizing uses parent exit). */
-    traceref(J, J->parent)->snap[J->exitno].count = SNAPCOUNT_DONE;
+    {
+      SnapShot *snap = &traceref(J, J->parent)->snap[J->exitno];
+      snap->count = SNAPCOUNT_DONE;
+      if (J->cur.topslot > snap->topslot) snap->topslot = J->cur.topslot;
+    }
     /* Add to side trace chain in root trace. */
     {
       GCtrace *root = traceref(J, J->cur.root);
diff --git a/test/tarantool-tests/fix-stack-alloc-on-trace-exit.test.lua b/test/tarantool-tests/fix-stack-alloc-on-trace-exit.test.lua
new file mode 100644
index 00000000..ca04e54e
--- /dev/null
+++ b/test/tarantool-tests/fix-stack-alloc-on-trace-exit.test.lua
@@ -0,0 +1,53 @@
+local tap = require('tap')
+
+-- Test file to demonstrate incorrect Lua stack restoration on
+-- exit from trace by the stack overflow.
+
+local test = tap.test('fix-stack-alloc-on-trace-exit'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+local jit_dump = require('jit.dump')
+
+test:plan(2)
+
+-- Before the patch, it is possible that a snapshot topslot is
+-- less than the possible topslot of the Lua stack. In that case,
+-- if the Lua stack overflows in `lj_vmevent_prepare()`, the error
+-- is raised inside `lj_vm_exit_handler()`, which has no
+-- corresponding DWARF eh_frame, so it leads to the crash.
+
+-- Need for the stack growing in `lj_vmevent_prepare`.
+jit_dump.start('x', '/dev/null')
+
+-- Create a coroutine with a fixed stack size.
+local coro = coroutine.create(function()
+  jit.opt.start('hotloop=1', 'hotexit=1', 'callunroll=1')
+
+  -- `math.modf` recording is NYI.
+  -- Local `math_modf` simplifies `jit.dump()` output.
+  local math_modf = math.modf
+
+  local function trace(n)
+    n = n + 1
+    -- luacheck: ignore
+    -- Start a side trace here.
+    if n % 2 == 0 then end
+    -- Stop the recording of the side trace and a main trace,
+    -- stitching.
+    math_modf(1, 1)
+    -- Grow stack, avoid tail calls.
+    local unused = trace(n)
+    return unused
+  end
+
+  local n = 0
+  trace(n)
+end)
+
+local result, errmsg = coroutine.resume(coro)
+
+test:ok(not result, 'correct status and no crash')
+test:like(errmsg, 'stack overflow', 'correct error message')
+
+test:done(true)
--------------u5w9esHNV0lDZq3NO7NtgB5s--