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 00578978F8A; Thu, 18 Jan 2024 16:02:13 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 00578978F8A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1705582934; bh=9Sp4tvqrLHgWqs+Hbak8VfnUlLFBIlDPErNleMj2gYI=; 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=Rhz/QxovxqWG0WFBbF06vuhZuE2M46ORdUJqT2BeTjb4RPHRS91IJ8bOom/R5pjy6 Kh0tbh8lNRn5il920UFFeOQuZY42h4sMUM1kwxCjOd9Y2ppEeWFkeZOJGx87uya8gH +xq2weqEG7s+5LlFO2j37/31g/qxlVSWHA1JA9Rs= 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 A5559755240 for ; Thu, 18 Jan 2024 16:02:12 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A5559755240 Received: by smtp31.i.mail.ru with esmtpa (envelope-from ) id 1rQS27-00000003Whe-2IQV; Thu, 18 Jan 2024 16:02:11 +0300 Content-Type: multipart/alternative; boundary="------------5gqSde2w0qM01UCZdtaqbn8b" Message-ID: Date: Thu, 18 Jan 2024 16:02:11 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Maxim Kokryashkin References: <20231122143534.11330-1-max.kokryashkin@gmail.com> <20231122143534.11330-4-max.kokryashkin@gmail.com> <5e448fb8-91d4-40a3-b9ca-8d08d307467c@tarantool.org> <1701874042.721661155@f762.i.mail.ru> Content-Language: en-US In-Reply-To: <1701874042.721661155@f762.i.mail.ru> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9AE5B4AFB3AE2A590E10A57BB36FAC7D4F681D642268DCBB5182A05F538085040AF5A98D8E2B30BC7A6D5EE0DB6E1EC8D2014C4B46E36F8904CBE91D84A52AAB6D971BDCFD067E642 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7F9D3BE5B596754B8C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE7F9D05773942AAE9CEA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38B73AB1701401CD871C8672D957468391645B8FED25E86FC61CA02247D960A1014A471835C12D1D9774AD6D5ED66289B5278DA827A17800CE7ABB305BD10C6E5099FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C37EF884183F8E4D67117882F4460429728AD0CFFFB425014E868A13BD56FB6657D81D268191BDAD3DC09775C1D3CA48CFA333A05395E4745B76E601842F6C81A12EF20D2F80756B5FB606B96278B59C4276E601842F6C81A127C277FBC8AE2E8B9FC99A4BA45EE8B4D81D268191BDAD3D3666184CF4C3C14F3FC91FA280E0CE3D1A620F70A64A45A98AA50765F7900637F1CEADB5F7626D0D6D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F790063723B0D190C46BCD7DEFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-C1DE0DAB: 0D63561A33F958A56A13DF0CA9C67D4C5002B1117B3ED6967A3DB57DFF2025B05B6221DB6D7A72AD823CB91A9FED034534781492E4B8EEADFCE5E68AF59B345DBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFDA3807020A02C01AF74C974F7713D94DD152576B8992D099E832142CD35A53978FF2D56E53A087286197189FEE4B1C7AB8EED10E2ECE1245CEE5400F2C249FC84881E310BE3D059EC226CC413062362A913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojjkiC0IEfsqJZsuiKDGaXtw== X-Mailru-Sender: C4F68CFF4024C8867DFDF7C7F25884584D42BEB0194EFF6E601A8E04A89F3FFC6A025324B560F2DF879D26B2AC41B2A0645D15D82EE4B272BD6E4642A116CA93524AA66B5ACBE6721EF430B9A63E2A504198E0F3ECE9B5443453F38A29522196 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit v3 3/3] Follow-up fix for stack overflow handling cleanup. 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 Cc: Maksim Kokryashkin , tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This is a multi-part message in MIME format. --------------5gqSde2w0qM01UCZdtaqbn8b Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi, Max my bad, test triggers a fixed problem. Thanks for the patch! LGTM On 12/6/23 17:47, Maxim Kokryashkin wrote: > Hi, Sergey! > Thanks for the review! > Tested on GC64 and non-GC64, and Linux/MacOS builds — test fails > before the patch everywhere. > Tested with API check and assertions enabled. > -- > Best regards, > Maxim Kokryashkin > > Пятница, 24 ноября 2023, 15:34 +03:00 от Sergey Bronnikov > : > Hello, Max > > added testcase is passed with reverted patch > > Sergey > > On 11/22/23 17:35, Maksim Kokryashkin wrote: > > From: Mike Pall > > > > (cherry-picked from commit aa6b15c1a8922848bd6f596ba384824ca3fe0f5f) > > > > The stack overflow error is thrown in `lj_state_growstack` only > > if the coroutine status is `OK`, however, stack overflow can > > happen on a yielded coroutine too. This patch fixes the condition > > for status, so now the error thrown on yielded coroutines too. > > > > Maxim Kokryashkin: > > * added the description and the test for the patch > > > > Part of tarantool/tarantool#9145 > > --- > > src/lj_state.c | 2 +- > > .../lj-962-premature-stack-overflow.test.c | 23 +++++++++++++++++++ > > 2 files changed, 24 insertions(+), 1 deletion(-) > > > > diff --git a/src/lj_state.c b/src/lj_state.c > > index d8a5134c..01d4901a 100644 > > --- a/src/lj_state.c > > +++ b/src/lj_state.c > > @@ -126,7 +126,7 @@ void LJ_FASTCALL > lj_state_growstack(lua_State *L, MSize need) > > 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'. */ > > + if (L->status < LUA_ERRRUN) { /* 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. */ > > } > > 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 > > index 12cb9004..461e0ccc 100644 > > --- a/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c > > +++ b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c > > @@ -24,6 +24,20 @@ static int fill_stack(lua_State *L) > > return 0; > > } > > > > +static int immediate_yield(lua_State *L) > > +{ > > + return lua_yield(L, 0); > > +} > > + > > +static int overflow_suspended_coro(lua_State *L) > > +{ > > + lua_State *newL = lua_newthread(L); > > + lua_pushcfunction(newL, immediate_yield); > > + lua_resume(newL, 0); > > + fill_stack(newL); > > + return 0; > > +} > > + > > static int premature_stackoverflow(void *test_state) > > { > > lua_State *L = test_state; > > @@ -50,12 +64,21 @@ static int > stackoverflow_during_stackoverflow(void *test_state) > > return TEST_EXIT_SUCCESS; > > } > > > > +static int stackoverflow_on_suspended_coro(void *test_state) > > +{ > > + lua_State *L = test_state; > > + int status = lua_cpcall(L, overflow_suspended_coro, NULL); > > + assert_true(status == LUA_ERRRUN); > > + return TEST_EXIT_SUCCESS; > > +} > > + > > int main(void) > > { > > lua_State *L = utils_lua_init(); > > const struct test_unit tgroup[] = { > > test_unit_def(premature_stackoverflow), > > test_unit_def(stackoverflow_during_stackoverflow), > > + test_unit_def(stackoverflow_on_suspended_coro), > > }; > > const int test_result = test_run_group(tgroup, L); > > utils_lua_close(L); > --------------5gqSde2w0qM01UCZdtaqbn8b Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

Hi, Max


my bad, test triggers a fixed problem.

Thanks for the patch! LGTM

On 12/6/23 17:47, Maxim Kokryashkin wrote:
Hi, Sergey!
Thanks for the review!
Tested on GC64 and non-GC64, and Linux/MacOS builds — test fails
before the patch everywhere.
Tested with API check and assertions enabled.
--
Best regards,
Maxim Kokryashkin
 
 
Пятница, 24 ноября 2023, 15:34 +03:00 от Sergey Bronnikov <sergeyb@tarantool.org>:
 
Hello, Max

added testcase is passed with reverted patch

Sergey

On 11/22/23 17:35, Maksim Kokryashkin wrote:
> From: Mike Pall <mike>
>
> (cherry-picked from commit aa6b15c1a8922848bd6f596ba384824ca3fe0f5f)
>
> The stack overflow error is thrown in `lj_state_growstack` only
> if the coroutine status is `OK`, however, stack overflow can
> happen on a yielded coroutine too. This patch fixes the condition
> for status, so now the error thrown on yielded coroutines too.
>
> Maxim Kokryashkin:
> * added the description and the test for the patch
>
> Part of tarantool/tarantool#9145
> ---
> src/lj_state.c | 2 +-
> .../lj-962-premature-stack-overflow.test.c | 23 +++++++++++++++++++
> 2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/src/lj_state.c b/src/lj_state.c
> index d8a5134c..01d4901a 100644
> --- a/src/lj_state.c
> +++ b/src/lj_state.c
> @@ -126,7 +126,7 @@ void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need)
> 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'. */
> + if (L->status < LUA_ERRRUN) { /* 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. */
> }
> 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
> index 12cb9004..461e0ccc 100644
> --- a/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
> +++ b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
> @@ -24,6 +24,20 @@ static int fill_stack(lua_State *L)
> return 0;
> }
>
> +static int immediate_yield(lua_State *L)
> +{
> + return lua_yield(L, 0);
> +}
> +
> +static int overflow_suspended_coro(lua_State *L)
> +{
> + lua_State *newL = lua_newthread(L);
> + lua_pushcfunction(newL, immediate_yield);
> + lua_resume(newL, 0);
> + fill_stack(newL);
> + return 0;
> +}
> +
> static int premature_stackoverflow(void *test_state)
> {
> lua_State *L = test_state;
> @@ -50,12 +64,21 @@ static int stackoverflow_during_stackoverflow(void *test_state)
> return TEST_EXIT_SUCCESS;
> }
>
> +static int stackoverflow_on_suspended_coro(void *test_state)
> +{
> + lua_State *L = test_state;
> + int status = lua_cpcall(L, overflow_suspended_coro, NULL);
> + assert_true(status == LUA_ERRRUN);
> + return TEST_EXIT_SUCCESS;
> +}
> +
> int main(void)
> {
> lua_State *L = utils_lua_init();
> const struct test_unit tgroup[] = {
> test_unit_def(premature_stackoverflow),
> test_unit_def(stackoverflow_during_stackoverflow),
> + test_unit_def(stackoverflow_on_suspended_coro),
> };
> const int test_result = test_run_group(tgroup, L);
> utils_lua_close(L);
 
--------------5gqSde2w0qM01UCZdtaqbn8b--