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 8E7F46F70E5; Wed, 22 Nov 2023 13:43:52 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8E7F46F70E5 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1700649832; bh=BpKhGKuF+syHF/nimMxEL7jWUaqVIwm9j7LQdsz553Q=; 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=asIm2/P+31xHBKyTcn1Ttx4pAxc5hqDZdyYyUDhNys15vv2pRXOVX4Gsl09Dr4Rs1 XfDll1vIIduH0Ey66wLzMMbaHhdTthGobzXMrQ3dJDvLbzD6yr/q0Ls61qFSdEjsd8 Hbhu1WvCj7tV2k2DysC8beJPsJQlQDuMxpkujPd0= Received: from smtp31.i.mail.ru (smtp31.i.mail.ru [95.163.41.72]) (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 23F9B2B8A01 for ; Wed, 22 Nov 2023 13:43:51 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 23F9B2B8A01 Received: by smtp31.i.mail.ru with esmtpa (envelope-from ) id 1r5khx-001uY8-2f; Wed, 22 Nov 2023 13:43:50 +0300 Date: Wed, 22 Nov 2023 13:39:18 +0300 To: Maksim Kokryashkin Message-ID: References: <20231121231647.92696-1-max.kokryashkin@gmail.com> <20231121231647.92696-2-max.kokryashkin@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231121231647.92696-2-max.kokryashkin@gmail.com> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD92D71B79D2A671AE6422F1D012930A708C4696DB667808124182A05F538085040370294D29E087317D9E7F9D9DB6DC4885BD681BE44F1DA9A18B9AEEEBFA5EC58 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE77BF46084C0059042EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006376B6794A086D6ADA58638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8742F66F18EDAB8B19E08F2043CF55C51117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC2EE5AD8F952D28FBA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18CB629EEF1311BF91D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE0AC5B80A05675ACDC3123C4324A5CF10D8FC6C240DEA76429C9F4D5AE37F343AA9539A8B242431040A6AB1C7CE11FEE3D56D36E97F3F038C6E0066C2D8992A16C4224003CC836476E2F48590F00D11D6E2021AF6380DFAD1A18204E546F3947CD2DCF9CF1F528DBC2E808ACE2090B5E1725E5C173C3A84C3C5EA940A35A165FF2DBA43225CD8A89F0A35B161A8BF67C1CE5475246E174218B5C8C57E37DE458BEDA766A37F9254B7 X-C1DE0DAB: 0D63561A33F958A5BCBDE551FD3131E8BAF52AFF316ECC2976CA2EF022DFA699F87CCE6106E1FC07E67D4AC08A07B9B0CF7CD7A0D5AA5F25CB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF86FD7196BA2F63E0AF6D3FD3C22A4F00182AD9DD93995F8BA81140A10D557DE3C2F598A3E54AB45188AD3775A6B0C83466169641D40F0962DD681E0F09453A9FA74DFFEFA5DC0E7F02C26D483E81D6BE5EF9655DD6DEA7D65774BB76CC95456EEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojGZhPsaRkbbkGlI8XH/6L6Q== X-Mailru-Sender: 11C2EC085EDE56FAC07928AF2646A769536974FB7217519DD9E7F9D9DB6DC488C479C2914A57FB1FDEDBA653FF35249392D99EB8CC7091A70E183A470755BFD208F19895AA18418972D6B4FCE48DF648AE208404248635DF X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit v2 1/3] Improve error reporting on 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 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" Hi, Maxim! Thanks for the fixes! LGTM, after fixing a few nits below. On 22.11.23, Maksim Kokryashkin wrote: > From: Mike Pall > > Thanks to Nicolas Lebedenco. > > (cherry-picked from commit 8135de2a0204e6acd92b231131c4a1e0be03ac1c) > > The `lj_state_growstack` doesn't account for a potential error > handler invocation by xpcall, which may lead to the second error Minor: s/xpcall/`xpcall`/ Feel free to ignore. > while handling a stack overflow, resulting in a misleading error > "error in error handling", while the real issue is a stack > overflow. This patch addresses this issue by fixing the condition > at which stack overflow errors are thrown. Now it's thrown if the > stack size is at least at the limit, instead of when it is over > the limit. > > This commit also disables the second test from > `lj-603-err-snap-restore`, since after this patch and the two > follow-ups for it, there is no such amount of stack slots with > which the test works the way it should. > > Lastly, this patch adds an alternative to `luacmd` to the > `utils.exec` module, which is called `luabin`. That function is > similar to the `luacmd`, with the only difference of returning > only the executable itself without any additional CLI options > passed. > > Maxim Kokryashkin: > * added the description and the test for the problem > > Part of tarantool/tarantool#9145 > --- > src/lj_state.c | 2 +- > .../lj-603-err-snap-restore.test.lua | 1 + > .../lj-962-stack-overflow-report.test.lua | 9 +++++++++ > .../lj-962-stack-overflow-report/script.lua | 2 ++ > test/tarantool-tests/utils/exec.lua | 15 ++++++++++++--- > 5 files changed, 25 insertions(+), 4 deletions(-) > create mode 100644 test/tarantool-tests/lj-962-stack-overflow-report.test.lua > create mode 100644 test/tarantool-tests/lj-962-stack-overflow-report/script.lua > > diff --git a/src/lj_state.c b/src/lj_state.c > index 684336d5..76153bad 100644 > --- a/src/lj_state.c > +++ b/src/lj_state.c > diff --git a/test/tarantool-tests/lj-603-err-snap-restore.test.lua b/test/tarantool-tests/lj-603-err-snap-restore.test.lua > index 96ebf92c..f5c8474f 100644 > --- a/test/tarantool-tests/lj-603-err-snap-restore.test.lua > +++ b/test/tarantool-tests/lj-603-err-snap-restore.test.lua > diff --git a/test/tarantool-tests/lj-962-stack-overflow-report.test.lua b/test/tarantool-tests/lj-962-stack-overflow-report.test.lua > new file mode 100644 > index 00000000..26a90f8d > --- /dev/null > +++ b/test/tarantool-tests/lj-962-stack-overflow-report.test.lua > @@ -0,0 +1,9 @@ > +local tap = require('tap') Minor: Please add a comment that the test reproduces the problem only for GC64 mode (and enabled JIT). > +local test = tap.test('lj-962-stack-overflow-report') > +test:plan(1) > + > +local LUABIN = require('utils').exec.luabin(arg) > +local SCRIPT = arg[0]:gsub('%.test%.lua$', '/script.lua') > +local output = io.popen(LUABIN .. ' 2>&1 ' .. SCRIPT, 'r'):read('*all') > +test:like(output, 'stack overflow', 'stack overflow reported correctly') > +test:done(true) > diff --git a/test/tarantool-tests/lj-962-stack-overflow-report/script.lua b/test/tarantool-tests/lj-962-stack-overflow-report/script.lua > new file mode 100644 > index 00000000..52734f2c > --- /dev/null > +++ b/test/tarantool-tests/lj-962-stack-overflow-report/script.lua > @@ -0,0 +1,2 @@ > +-- luacheck: no global Nit: It's better to add a comment about avoiding an additional stack slot usage (local). > +f = function() f() end; f() > diff --git a/test/tarantool-tests/utils/exec.lua b/test/tarantool-tests/utils/exec.lua > index a56ca2dc..7ed0a9d8 100644 > --- a/test/tarantool-tests/utils/exec.lua > +++ b/test/tarantool-tests/utils/exec.lua > @@ -1,14 +1,23 @@ > local M = {} > > -function M.luacmd(args) > +local function cmd_start_idx(args) Nit: I suggest naming this function like `executable_idx()` or something similar. > -- arg[-1] is guaranteed to be not nil. > local idx = -2 > while args[idx] do > assert(type(args[idx]) == 'string', 'Command part have to be a string') > idx = idx - 1 > end > - -- return the full command with flags. > - return table.concat(args, ' ', idx + 1, -1) > + return idx + 1 > +end > + > +function M.luabin(args) > + -- Return only the executable. > + return args[cmd_start_idx(args)] > +end > + > +function M.luacmd(args) > + -- Return the full command with flags. > + return table.concat(args, ' ', cmd_start_idx(args), -1) > end > > local function makeenv(tabenv) > -- > 2.39.3 (Apple Git-145) > -- Best regards, Sergey Kaplun