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 5F6F573CE22; Thu, 21 Dec 2023 15:01:32 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5F6F573CE22 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1703160092; bh=EBU/TEP8IdhJngaGhdOMmJzJZeDFzM24QTduH1ls28U=; 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=Z7ihzmWfqkJf80Svfe0RSNOWnQ5PAnhzZ67F6gLtBl1w8Zdf8asgblsi5jNYk6Hvl 2wbjxy7l7WejlxXg/YA8mEWZeecIoy4JPp1dRPug8JLpBY97UG1wWJ38ovYiOTIDPF K7bp8GrgxgWoPeQowFmt0u8hKNbq59gJG2vtoqL8= Received: from smtp34.i.mail.ru (smtp34.i.mail.ru [95.163.41.75]) (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 7528F6F17C0 for ; Thu, 21 Dec 2023 15:01:30 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7528F6F17C0 Received: by smtp34.i.mail.ru with esmtpa (envelope-from ) id 1rGHk1-00BAuw-1x; Thu, 21 Dec 2023 15:01:29 +0300 Date: Thu, 21 Dec 2023 15:01:29 +0300 To: Sergey Kaplun Cc: Maxim Kokryashkin , tarantool-patches@dev.tarantool.org Message-ID: References: <20231214114458.17929-1-m.kokryashkin@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Mailru-Src: smtp X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD9F008C97756F746CA8E02FB725DB7BE58AC4BA3EACCC2FC76182A05F538085040A82CC08E20A8601AF82B59149D7D523A27BDE0BE16730FB351AE4A950537C350 X-C1DE0DAB: 0D63561A33F958A52F05DDCA0D6BEC5E6B6F9207A5702C2144291DE55C21DD7DF87CCE6106E1FC07E67D4AC08A07B9B0034D30FDF2F620DBCB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF77DD89D51EBB7742D3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFD134D87241EAC673A2FD62B394611D601E664EB9081F9801CF2FABB2A75141F4508B3DC588E9D1C75FCFFA5448E432FF1D0E2BDAC1D0026C507DC728C10D7B31E48CAC7CA610320002C26D483E81D6BE64ACE4A408B72B61B0CA6F94E606A667A52EF62A646584F811BD90D3D42C882D43082AE146A756F3 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojd2R+qgYPpI2njfEnQYN90Q== X-Mailru-Sender: 11C2EC085EDE56FA38FD4C59F7EFE40702EB1CF47708A02AE4F381B7DA5A774DA0E7647AE5A21EA6D51284F0FE6F529ABC7555A253F5B200DF104D74F62EE79D27EC13EC74F6107F4198E0F3ECE9B5443453F38A29522196 X-Mras: OK Subject: Re: [Tarantool-patches] [PATCH luajit] Avoid assertion in case of stack overflow from stitched trace. 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: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! Thanks for the review! On Thu, Dec 14, 2023 at 03:20:53PM +0300, Sergey Kaplun via Tarantool-patches wrote: > Hi, Maxim! > Thanks for the patch! > Please, consider my comments below. > > On 14.12.23, Maxim Kokryashkin wrote: > > From: Mike Pall > > > > Reported by Sergey Bronnikov. Fixed by Sergey Kaplun. > > > > (cherry-picked from commit 1d75cd4d7be638babe6d4e47bf73ea05fc65d81c) > > > > When we exit from a stitched trace due to the Lua stack overflow > > error, the Lua and JIT stacks are not synchronized, and they > > won't be as long as the mentioned error is raised. Because of > > The reason is not the stack synchornization, but the determination of > the bytecode when the error is raised. In the case of stitched trace > (see the dumps below), the last slot in the snapshot map is the argument > for the function to stitch (in our case, ref to 42). Or for the second > trace, we have any other function where the recording is stopped but not > the function where it was started. > So determine bytecode position isn't trivial. And it is better just > return `NO_BCPOS` in such cases. Fixed, thanks! Branch is force-pushed. Here is the new commit message: === Avoid assertion in case of stack overflow from stitched trace. Reported by Sergey Bronnikov. Fixed by Sergey Kaplun. (cherry-picked from commit 1d75cd4d7be638babe6d4e47bf73ea05fc65d81c) When we exit from a stitched trace due to the Lua stack overflow error, we cannot determine the bytecode position properly, and because of that, we get the incorrect bytecode instruction in `debug_framepc`. This patch fixes this behavior, so the `debug_framepc` now returns `NO_BCPOS` for this case. Maxim Kokryashkin: * added the description and the test for the problem Part of tarantool/tarantool#9145 === > > | Trace 1 start > | ---- TRACE IR > | .... SNAP #0 [ ---- ] > | 0001 fun SLOAD [L ] lit: #0 lit: R > | 0002 tab FLOAD [L ] ref: 0001 lit: func.env > | 0003 int FLOAD [L ] ref: 0002 lit: tab.hmask > | 0004 > int EQ [C ] ref: 0003 ref: integer 63 > | 0005 p32 FLOAD [L ] ref: 0002 lit: tab.node > | 0006 > p32 HREFK [R ] ref: 0005 ref: string "math" @ 0x400061e8 KSLOT: @54 > | 0007 > tab HLOAD [L ] ref: 0006 > | 0008 int FLOAD [L ] ref: 0007 lit: tab.hmask > | 0009 > int EQ [C ] ref: 0008 ref: integer 31 > | 0010 p32 FLOAD [L ] ref: 0007 lit: tab.node > | 0011 > p32 HREFK [R ] ref: 0010 ref: string "fmod" @ 0x40006c88 KSLOT: @18 > | 0012 > fun HLOAD [L ] ref: 0011 > | 0013 > fun EQ [C ] ref: 0012 ref: fast function #58 > | .... SNAP #1 [ ---- {trace 0x1 @ 0x400271d8} contpc ftsz | {number 42} {number 42} ] > | 0014 NOP [N ] > | > | Trace 2 start > | ---- TRACE IR > | .... SNAP #0 [ ---- ] > | 0001 fun SLOAD [L ] lit: #0 lit: R > | 0002 > fun EQ [C ] ref: 0001 ref: Lua function @ 0x400157f0, 1 upvalues, "@../master/test/tarantool-tests/lj-913-so-stitched.test.lua":11 > | .... SNAP #1 [ ftsz | ftsz | ] > | 0003 NOP [N ] > > > that, we get the incorrect bytecode instruction in > > `debug_framepc`. This patch fixes this behavior, so the > > `debug_framepc` now returns `NO_BCPOS` for this case. > > > > Maxim Kokryashkin: > > * added the description and the test for the problem > > > > Part of tarantool/tarantool#9145 > > --- > > Branch: https://github.com/tarantool/luajit/tree/fckxorg/lj-913-avoid-assertion-stkov-from-stitched-trace > > PR: https://github.com/tarantool/tarantool/pull/9484 > > Issues: https://github.com/tarantool/tarantool/issues/9145 > > https://github.com/LuaJIT/LuaJIT/issues/913 > > > > src/lj_debug.c | 9 +++++--- > > ...-913-stackoverflow-stitched-trace.test.lua | 23 +++++++++++++++++++ > > 2 files changed, 29 insertions(+), 3 deletions(-) > > create mode 100644 test/tarantool-tests/lj-913-stackoverflow-stitched-trace.test.lua > > > > diff --git a/src/lj_debug.c b/src/lj_debug.c > > index 46c442c6..107f464c 100644 > > --- a/src/lj_debug.c > > +++ b/src/lj_debug.c > > > > > diff --git a/test/tarantool-tests/lj-913-stackoverflow-stitched-trace.test.lua b/test/tarantool-tests/lj-913-stackoverflow-stitched-trace.test.lua > > new file mode 100644 > > index 00000000..3c12f0d9 > > --- /dev/null > > +++ b/test/tarantool-tests/lj-913-stackoverflow-stitched-trace.test.lua > > @@ -0,0 +1,23 @@ > > +local tap = require('tap') > > +-- Test to demonstrate the incorrect LuaJIT behavior when exiting > > +-- from a snapshot for stitched trace. > > +local test = tap.test('lj-913-stackoverflow-stitched-trace'):skipcond({ > > + ['Test requires JIT enabled'] = not jit.status(), > > +}) > > + > > +test:plan(3) > > + > > +-- Recursion to cause stack overflow. > > +local function callee() > > + -- `math.fmod()` is NYI, so trace will be stitched here. > > + local _ = math.fmod(42, 42) > > We don't need this `_` variable here. Just using `math.fmod()` is > enough. Fixed, thanks! Branch is force-pushed. Here is the diff: === diff --git a/test/tarantool-tests/lj-913-stackoverflow-stitched-trace.test.lua b/test/tarantool-tests/lj-913-stackoverflow-stitched-trace.test.lua index 3c12f0d9..434901dc 100644 --- a/test/tarantool-tests/lj-913-stackoverflow-stitched-trace.test.lua +++ b/test/tarantool-tests/lj-913-stackoverflow-stitched-trace.test.lua @@ -10,7 +10,7 @@ test:plan(3) -- Recursion to cause stack overflow. local function callee() -- `math.fmod()` is NYI, so trace will be stitched here. - local _ = math.fmod(42, 42) + math.fmod(42, 42) callee() end === > > > + callee() > > +end > > + > > +local st, err = pcall(callee) > > + > > +test:ok(true, 'assertion is not triggered') > > +test:ok(not st, 'error happened') > > +test:like(err, 'stack overflow', 'stack overflow happened') > > + > > +test:done(true) > > -- > > 2.43.0 > > > > -- > Best regards, > Sergey Kaplun