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 DA3006E7427; Mon, 20 Nov 2023 16:34:39 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org DA3006E7427 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1700487280; bh=6lSEnLbWXb9Ig7ZltxubdVHmOIilR+/lH6NnQDV6iU4=; 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=hf2SGNxGBW2bDVWTumphLVStTEC/8CKM/mOiJYkZYsEHOzW/6nPmHJFX/zOvSKekU 3avrFpuYAQra1RIYfhXRgcD6Bx8wuAvtQSD4/qCq7xU8cidqM33FsAjytERWgBehJa V2OiFdioXyzjCUMLbrz0pr5bY3aMedisOsB4xWlI= 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 8A4EA6E7186 for ; Mon, 20 Nov 2023 16:34:38 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8A4EA6E7186 Received: by smtp34.i.mail.ru with esmtpa (envelope-from ) id 1r54Q9-007rt0-2e; Mon, 20 Nov 2023 16:34:38 +0300 Date: Mon, 20 Nov 2023 16:30:05 +0300 To: Maksim Kokryashkin Message-ID: References: <20231117234116.60037-1-max.kokryashkin@gmail.com> <20231117234116.60037-3-max.kokryashkin@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231117234116.60037-3-max.kokryashkin@gmail.com> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD9BE2AA157F0467848E5078B8A6730C0A65F7BE7DDFC63FF0D182A05F5380850404C228DA9ACA6FE27CD712184D0CF0BD5DC6307699B2CEED149E2246B32064B72DCDA5B9864987461 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE72F22E6DC541F75D9EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006377548186386978BE68638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8D66DBBB9F263BDEE17011CA393183156117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAE9A1BBD95851C5BA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD186FD1C55BDD38FC3FD2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE140C956E756FBB7A7F16001415B11694D8FC6C240DEA76429C9F4D5AE37F343AA9539A8B242431040A6AB1C7CE11FEE3E753FA5741D1AD02040F9FF01DFDA4A8C4224003CC836476E2F48590F00D11D6E2021AF6380DFAD1A18204E546F3947CD2DCF9CF1F528DBC2E808ACE2090B5E1725E5C173C3A84C3C5EA940A35A165FF2DBA43225CD8A89F0A35B161A8BF67C1262FEC7FBD7D1F5BB5C8C57E37DE458BEDA766A37F9254B7 X-C1DE0DAB: 0D63561A33F958A5C0D3FB8973D67D3139EA388BE1C5C93D420AFC664287F976F87CCE6106E1FC07E67D4AC08A07B9B0B355ED1E20F5346ACB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFC809A0D5D69947AB3F0E464FE1E97BE2FFCBA8C6057A6D0F334AC68EDB489B599AB7DC20417E211588AD3775A6B0C834380AC001CC569F828B6EC0B932F272C3A74DFFEFA5DC0E7F02C26D483E81D6BE5EF9655DD6DEA7D65774BB76CC95456EEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojmRIjd71J0y1EobsFCHkhJw== X-Mailru-Sender: 11C2EC085EDE56FAC07928AF2646A769064631B94722E871DC6307699B2CEED11E1E156D000DABDBDEDBA653FF35249392D99EB8CC7091A70E183A470755BFD208F19895AA18418972D6B4FCE48DF648AE208404248635DF X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 2/3] Cleanup stack overflow handling. 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 patch! Please consider my comments below. On 18.11.23, Maksim Kokryashkin wrote: > From: Mike Pall > > Reported by Peter Cawley. > > (cherry-picked from commit d2f6c55b05c716e5dbb479b7e684abaee7cf6e12) > > After the previous patch, it is possible to trigger the > `stack overflow` error prematurely. Consider the following > situation: there are already 33000 slots allocated on a Lua > stack, and then there are 30 additional slots needed. In this > case, the actual allocated amount would be twice the already > allocated size, shrunk to the `LJ_STACK_MAXEX` size, which > would lead to the stack overflow error, despite the fact there > is plenty of unused space. This patch completely reworks the > logic of error handling during stack growth to address the issue. > > Maxim Kokryashkin: > * added the description and the test for the problem > > Part of tarantool/tarantool#9145 > --- > src/lj_state.c | 15 +++++-- > .../lj-962-premature-stack-overflow.test.c | 41 +++++++++++++++++++ > 2 files changed, 52 insertions(+), 4 deletions(-) > create mode 100644 test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c > > diff --git a/src/lj_state.c b/src/lj_state.c > index 76153bad..d8a5134c 100644 > --- a/src/lj_state.c > +++ b/src/lj_state.c > @@ -121,8 +121,17 @@ void lj_state_shrinkstack(lua_State *L, MSize used) > void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need) > { > MSize n; > - if (L->stacksize > LJ_STACK_MAXEX) /* Overflow while handling overflow? */ > - lj_err_throw(L, LUA_ERRERR); > + if (L->stacksize >= LJ_STACK_MAXEX) { > + /* 4. Throw 'error in error handling' when we are _over_ the limit. */ > + if (L->stacksize > LJ_STACK_MAXEX) > + lj_err_throw(L, LUA_ERRERR); /* Does not invoke an error handler. */ > + /* 1. We are _at_ the limit after the last growth. */ > + if (!L->status) { /* 2. Throw 'stack overflow'. */ > + L->status = LUA_ERRRUN; /* Prevent ending here again for pushed msg. */ > + lj_err_msg(L, LJ_ERR_STKOV); /* May invoke an error handler. */ > + } > + /* 3. Add space (over the limit) for pushed message and error handler. */ > + } > n = L->stacksize + need; > if (n > LJ_STACK_MAX) { > n += 2*LUA_MINSTACK; > @@ -132,8 +141,6 @@ void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need) > n = LJ_STACK_MAX; > } > resizestack(L, n); > - if (L->stacksize >= LJ_STACK_MAXEX) > - lj_err_msg(L, LJ_ERR_STKOV); > } > > void LJ_FASTCALL lj_state_growstack1(lua_State *L) > diff --git a/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c > new file mode 100644 > index 00000000..0873636a > --- /dev/null > +++ b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c > @@ -0,0 +1,41 @@ > +#include "lua.h" > +#include "lauxlib.h" > + > +#include "test.h" > +#include "utils.h" > + > +#include "lj_obj.h" > +#include "luaconf.h" > + > +static int cur_slots = -1; > + > +static int fill_stack(lua_State *L) > +{ > + cur_slots = L->base - tvref(L->stack); Can we just use `LJ_GC64 + 1` with a comment here? It helps to avoid usage of internal objects. > + > + while(lua_gettop(L) < LUAI_MAXSTACK) { > + cur_slots += 1; > + lua_pushinteger(L, 42); > + } > + > + return 1; > +} > + > +static int premature_stackoverflow(void *test_state) > +{ > + lua_State *L = test_state; > + lua_cpcall(L, fill_stack, NULL); > + assert_true(cur_slots == LUAI_MAXSTACK - 1); > + return TEST_EXIT_SUCCESS; > +} > + > +int main(void) > +{ > + lua_State *L = utils_lua_init(); > + const struct test_unit tgroup[] = { > + test_unit_def(premature_stackoverflow), This test doesn't check the case when we do not invoke an error handler (case 4). Also, there is no check for the error handler itself to be called (it may be done in a separate test). OTOH, I'm not sure that this should be done as a part of this particular test. I suggest waiting for the Sergey's opinion here. Also, I suggest adding a test when without set `L->status = LUA_ERRRUN` we are in trouble. > + }; > + const int test_result = test_run_group(tgroup, L); > + utils_lua_close(L); > + return test_result; > +} > -- > 2.39.3 (Apple Git-145) > -- Best regards, Sergey Kaplun