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 353166EC55; Wed, 21 Jul 2021 11:51:04 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 353166EC55 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1626857464; bh=QjOufj1DeRJyzX2rTZ8gXecFtX8qwCR4CXNiRz4pY0c=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=JLULkKFEPxp9kU/zitO44c/YwdNN0XHTHDfRHzUe/ICs89FbSH3VavzVMsRooKpJx 5+BR972cCAA0AvlfzFbKd3F5JL7A1o1sN7SXAxUTTCsFnaq2FTaph9C3D44SSJgJX8 +1AfEJYQnV74JuVNUiDYVtc1KM5YGXGo9sq2uk9s= Received: from smtp37.i.mail.ru (smtp37.i.mail.ru [94.100.177.97]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 14DB76EC55 for ; Wed, 21 Jul 2021 11:51:03 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 14DB76EC55 Received: by smtp37.i.mail.ru with esmtpa (envelope-from ) id 1m67wT-00023C-T1; Wed, 21 Jul 2021 11:51:02 +0300 Date: Wed, 21 Jul 2021 11:49:50 +0300 To: Igor Munkin Message-ID: References: <20210618062234.871-1-skaplun@tarantool.org> <20210720122952.GH11494@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210720122952.GH11494@tarantool.org> X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD941C43E597735A9C386C8E0DDEE7E2465E4A01A82547D9FE1182A05F538085040B211EC2BF955246DBC9F3E90D22771FC59E37AE10804B1778C75FE0AF510CDF0 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7A1DB0B089319D380EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637A9E1EE47BE02EAD28638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8F4DC519BF7F486A6C6D49E1CBF0AD5B6117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BF1175FABE1C0F9B6A471835C12D1D977C4224003CC8364762BB6847A3DEAEFB0F43C7A68FF6260569E8FC8737B5C2249EC8D19AE6D49635B68655334FD4449CB9ECD01F8117BC8BEAAAE862A0553A39223F8577A6DFFEA7CE0F3BA37685B2B9043847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: 0D63561A33F958A5AA1E3FB62CD78751ADD4A473288D97FDE7C3110E25A69DADD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7501A9DF589746230F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34F7CC4EA888783AE0997DD33FFE74308C228DA98BB4ACA9B5843A366479AEFD9A4099FD8B8927660B1D7E09C32AA3244C6589C2AA776142EFB2DD4E12F7DA1A5533C9DC155518937FFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojJX8TSRcb/Shso9ayZ6DnWw== X-Mailru-Sender: 3B9A0136629DC91206CBC582EFEF4CB4A1C2AAF8C73C163EE77D45A0801F2E152F70CE752AFB32DAF2400F607609286E924004A7DEC283833C7120B22964430C52B393F8C72A41A89437F6177E88F7363CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit v1] tools: fix luajit-gdb stack dump 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" Igor, On 20.07.21, Igor Munkin wrote: > Sergey, > > Thanks for the patch! Nice catch, actually! I wonder, how much impact > this bug made while digging the crash reports. Anyway, LGTM, but > consider several nits below. Actually, not too much. It is the unique situation, when we have naked guest Lua stack without any function call. Branch is force-pushed with the changes below. > > On 18.06.21, Sergey Kaplun wrote: > > When there is only one frame (so-called dummy frame) with a dummy thread > > on stack bottom at L->base - 1 - LJ_FR2, stack dump does not show stack > > slots from bottom to top, because the frame link is already pointed to > > stack bottom (dummy thread mentioned above). Its output looks like the > > Too much information in a single sentence. I consider the following info > vital to understand the patch. > * Dummy frame is the particular and kinda "initial" coroutine state, > when the framelink slot (i.e. L->base - (1 + LJ_FR2)) is the most > bottom slot of the guest stack (i.e. L->stack). > * Since coroutine stack unwinding is implemented via precondition loop, > no slots are dumped from L->top to L->base in case when only dummy > frame is on the stack. > * Python is a crap language, and seems like postcondition loop violates > The Zen (this is too hard for users, isn't it?), so Guido decided not > to introduce such construction in the language at all. > > #ifdef offtop > I dare to guess, it was just too hard to distinguish precondition > from postcondition in "colon&spaces" paradigm; glad Larry Wall > decided to use braces for blocks, and Roberto just left Pascalish > syntax for this. I'm upset with the GDB guys choice every time we're > back to developing this extension. I suppose that it just was rather simple and one of the most popular languages at the moment of making decision. > #endif > > comment.yield() > > > following: > > > > | 0x7fb512ac40:0x7fb512ac70 [ ] 7 slots: Red zone > > | 0x7fb512ac38 [ M] > > | 0x7fb512ab28:0x7fb512ac30 [ ] 34 slots: Free stack slots > > | 0x7fb512ab20 [ T ] > > | 0x7fb512ab08:0x7fb512ab10 [S ] FRAME: dummy L > > Minor: I guess, you can align the output. At least I see no reason not > to do this. > > > > > comment.resume() > > Hence, there is no other option except unroll the first iteration and > reorder stack slots dump with moving framelinks downwards to the > coroutine stack bottom. > > > This patch unrolled first loop iteration. Stack dump first inspect all > > fields for the top frame and then continue if there are other frames. > > After patch the output looks like the following: > > > > | 0x7fb512ac40:0x7fb512ac70 [ ] 7 slots: Red zone > > | 0x7fb512ac38 [ M] > > | 0x7fb512ab28:0x7fb512ac30 [ ] 34 slots: Free stack slots > > | 0x7fb512ab20 [ T ] > > | 0x7fb512ab18 [ ] VALUE: string 0 "/tmp/net_box.lua:6: err in ser" @ 0x7fb512ade8 > > | 0x7fb512ab10 [ B ] VALUE: table @ 0x7fb512ac80 (asize: 0, hmask: 0x0) > > | 0x7fb512ab00:0x7fb512ab08 [S ] FRAME: dummy L > > Feel free to align the output here too. Thanks for your comments! The new commit message version: | Dummy frame is the "initial" coroutine state, when the framelink slot (i.e. | L->base - (1 + LJ_FR2)) is the bottom slot of the guest stack | (i.e. L->stack). Since coroutine stack unwinding is implemented via | precondition loop, lj-stack doesn't dump the slots for the dummy frame, since | the framelink points to the stack bottom. | | The output looks like the following: | | | 0x7fb512ac40:0x7fb512ac70 [ ] 7 slots: Red zone | | 0x7fb512ac38 [ M] | | 0x7fb512ab28:0x7fb512ac30 [ ] 34 slots: Free stack slots | | 0x7fb512ab20 [ T ] | | 0x7fb512ab08:0x7fb512ab10 [S ] FRAME: dummy L | | Python doesn't provide post-condition (do-while) syntax construction, | that fits better for this case, so the unwinding of the topmost frame is just manually | unrolled. | | As a result of the patch the output looks like the following: | | | 0x7fb512ac40:0x7fb512ac70 [ ] 7 slots: Red zone | | 0x7fb512ac38 [ M] | | 0x7fb512ab28:0x7fb512ac30 [ ] 34 slots: Free stack slots | | 0x7fb512ab20 [ T ] | | 0x7fb512ab18 [ ] VALUE: string 0 "/tmp/net_box.lua:6: err in ser" @ 0x7fb512ade8 | | 0x7fb512ab10 [ B ] VALUE: table @ 0x7fb512ac80 (asize: 0, hmask: 0x0) | | 0x7fb512ab00:0x7fb512ab08 [S ] FRAME: dummy L > > > --- > > > > Branch: https://github.com/tarantool/luajit/tree/skaplun/fix-luajit-gdb > > > > Steps to test behaviour: > > 0) Clone branch > > 1) Build tarantool with GC64 or not (I've tested both cases) > > 2) Run the following script by Tarantool via gdb. > > > > | $ cat /tmp/netbox.lua > > | box.cfg{log_level = 50, listen = "127.0.0.1:3802"} > > | box.schema.user.grant('guest','read,write,execute','universe', nil, {if_not_exists=true}) > > | local net_box = require"net.box" > > | local cn = net_box:connect("127.0.0.1:3802") > > | local function f() > > | return setmetatable({}, {__serialize = function() error[[err in ser]] end}) > > | end > > | _G.f = f > > | local r, st = cn:call("f") > > | print(r,st) > > > > | $ gdb --args ./tarantool /tmp/netbox.lua > > | ... > > | (gdb) source ~/builds_workspace/luajit/fix-luajit-gdb/src/luajit-gdb.py > > | ... > > > > 3) Set breakpoint to ./src/lua/msgpack.c:234, and run > > > > | (gdb) b ./src/lua/msgpack.c:234 > > > > I'm not sure about your sources version, so you can just set breakpoin to > > mp_encode(), run and continue (need two entries). We are interested in > > the line #234: > > > > | 233|.......if (luaL_tofield(L, cfg, opts, lua_gettop(L), &field) < 0) > > | 234|.......|.......return luaT_error(L); > > > > And dump the stack: > > > > | (gdb) lj-stack L > > | 0x7fb512ac40:0x7fb512ac70 [ ] 7 slots: Red zone > > | 0x7fb512ac38 [ M] > > | 0x7fb512ab28:0x7fb512ac30 [ ] 34 slots: Free stack slots > > | 0x7fb512ab20 [ T ] > > | 0x7fb512ab18 [ ] VALUE: string 0 "/tmp/net_box.lua:6: err in ser" @ 0x7fb512ade8 > > | 0x7fb512ab10 [ B ] VALUE: table @ 0x7fb512ac80 (asize: 0, hmask: 0x0) > > | 0x7fb512ab00:0x7fb512ab08 [S ] FRAME: dummy L > > > > src/luajit-gdb.py | 16 +++++++++++++--- > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py > > index f1fd6230..a3dad585 100644 > > --- a/src/luajit-gdb.py > > +++ b/src/luajit-gdb.py > > @@ -424,16 +424,26 @@ def dump_stack(L, base=None, top=None): > > ) > > ]) + '\n' > > > > + # Set framelink for the top frame. Dump frame slots even for dummy frame. > > It would be also great to drop a few words why the iteration is unrolled > (the lack of postcondition loops in Python) via XXX comment. > > > slot = top - 1 > > framelink = base - (1 + LJ_FR2) > > I suggest to split this part into two blocks via blank line: > * variable initialization > * unrolled iteration > > > + while slot > framelink + LJ_FR2: > > + dump += dump_stack_slot(L, slot, base, top) > > + slot -= 1 > > > > + # If there are other frames -- continue. > > Again, this is not the particular reason. As for me the main reason you > can't implement stack unwinding via do-while construction. Adjusted this chunk considering your suggestions. =================================================================== diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py index a3dad585..fab276ed 100644 --- a/src/luajit-gdb.py +++ b/src/luajit-gdb.py @@ -424,14 +424,22 @@ def dump_stack(L, base=None, top=None): ) ]) + '\n' - # Set framelink for the top frame. Dump frame slots even for dummy frame. slot = top - 1 framelink = base - (1 + LJ_FR2) + + # XXX: Lua stack unwinding algorithm consists of the following steps: + # 1. dump all data slots in the (framelink, top) interval + # 2. check whether there are remaining frames + # 3. if there are no slots further, stop the unwinding loop + # 4. otherwise, resolve the next framelink and top and go to (1) + # + # Postcondition (i.e. do-while) loops is the most fitting idiom for such + # case, but Python doesn't provide such lexical construction. Hence step (1) + # is unrolled for the topmost stack frame. while slot > framelink + LJ_FR2: dump += dump_stack_slot(L, slot, base, top) slot -= 1 - # If there are other frames -- continue. while framelink > mref('TValue *', L['stack']): assert slot == framelink + LJ_FR2, "Invalid slot during frame unwind" dump += dump_framelink(L, framelink) @@ -442,7 +450,7 @@ def dump_stack(L, base=None, top=None): slot -= 1 assert slot == framelink + LJ_FR2, "Invalid slot after frame unwind" - # Skip nil slot for the last frame for 2-slot frames. + # Skip a nil slot for the last frame for 2-slot frames. slot -= LJ_FR2 dump += '{fr}{padding} [S ] FRAME: dummy L'.format( =================================================================== > > > while framelink > mref('TValue *', L['stack']): > > - while slot > framelink + LJ_FR2: > > - dump += dump_stack_slot(L, slot, base, top) > > - slot -= 1 > > + assert slot == framelink + LJ_FR2, "Invalid slot during frame unwind" > > Please, use parentheses for assert call. It's done by design to avoid always true assertions [1]: | $ python3 | Python 3.7.10 (default, Mar 18 2021, 04:43:06) | [GCC 9.3.0] on linux | Type "help", "copyright", "credits" or "license" for more information. | >>> assert(1!=1, "stupid assert") | :1: SyntaxWarning: assertion is always true, perhaps remove parentheses? > > > dump += dump_framelink(L, framelink) > > framelink = frame_prev(framelink + LJ_FR2) - LJ_FR2 > > slot -= 1 + LJ_FR2 > > + while slot > framelink + LJ_FR2: > > + dump += dump_stack_slot(L, slot, base, top) > > + slot -= 1 > > + > > + assert slot == framelink + LJ_FR2, "Invalid slot after frame unwind" > > Ditto. Ditto. > > > + # Skip nil slot for the last frame for 2-slot frames. > > + slot -= LJ_FR2 > > > > dump += '{fr}{padding} [S ] FRAME: dummy L'.format( > > fr = slot, > > -- > > 2.31.0 > > > > -- > Best regards, > IM [1]: https://docs.python.org/3/reference/simple_stmts.html#grammar-token-assert-stmt -- Best regards, Sergey Kaplun