From 511d53a826760dd11cd82947184583e2d094e2d2 Mon Sep 17 00:00:00 2001 From: Roberto Ierusalimschy Date: Tue, 9 Mar 2021 11:42:45 -0300 Subject: [PATCH] lua_settop/lua_pop closes to-be-closed variables The existence of 'lua_closeslot' is no reason for lua_pop not to close to-be-closed variables too. It is too error-prone for lua_pop not to close tbc variables being popped from the stack. --- lapi.c | 15 ++++++++------- manual/manual.of | 23 +++++++++++------------ testes/api.lua | 26 +++++++++++++++++++++++++- 3 files changed, 44 insertions(+), 20 deletions(-) diff --git a/lapi.c b/lapi.c index a9cf2fdb..f8f70cd0 100644 --- a/lapi.c +++ b/lapi.c @@ -173,7 +173,7 @@ LUA_API int lua_gettop (lua_State *L) { LUA_API void lua_settop (lua_State *L, int idx) { CallInfo *ci; - StkId func; + StkId func, newtop; ptrdiff_t diff; /* difference for new top */ lua_lock(L); ci = L->ci; @@ -188,12 +188,13 @@ LUA_API void lua_settop (lua_State *L, int idx) { api_check(L, -(idx+1) <= (L->top - (func + 1)), "invalid new top"); diff = idx + 1; /* will "subtract" index (as it is negative) */ } -#if defined(LUA_COMPAT_5_4_0) - if (diff < 0 && hastocloseCfunc(ci->nresults)) - luaF_close(L, L->top + diff, CLOSEKTOP, 0); -#endif - api_check(L, L->tbclist < L->top + diff, "cannot pop an unclosed slot"); - L->top += diff; + api_check(L, L->tbclist < L->top, "previous pop of an unclosed slot"); + newtop = L->top + diff; + if (diff < 0 && L->tbclist >= newtop) { + lua_assert(hastocloseCfunc(ci->nresults)); + luaF_close(L, newtop, CLOSEKTOP, 0); + } + L->top = newtop; /* correct top only after closing any upvalue */ lua_unlock(L); } diff --git a/manual/manual.of b/manual/manual.of index 2e15839a..c69970d2 100644 --- a/manual/manual.of +++ b/manual/manual.of @@ -4253,12 +4253,8 @@ If the new top is greater than the old one, then the new elements are filled with @nil. If @id{index} @N{is 0}, then all stack elements are removed. -For compatibility reasons, -this function may close slots marked as to-be-closed @see{lua_toclose}, -and therefore it can run arbitrary code. -You should not rely on this behavior: -Instead, always close to-be-closed slots explicitly, -with @Lid{lua_closeslot}, before removing them from the stack. +This function can run arbitrary code when removing an index +marked as to-be-closed from the stack. } @@ -4347,19 +4343,22 @@ otherwise, returns @id{NULL}. @apii{0,0,m} Marks the given index in the stack as a -to-be-closed @Q{variable} @see{to-be-closed}. +to-be-closed slot @see{to-be-closed}. Like a to-be-closed variable in Lua, -the value at that index in the stack will be closed +the value at that slot in the stack will be closed when it goes out of scope. Here, in the context of a C function, to go out of scope means that the running function returns to Lua, -there is an error, +or there is an error, +or the slot is removed from the stack through +@Lid{lua_settop} or @Lid{lua_pop}, or there is a call to @Lid{lua_closeslot}. -An index marked as to-be-closed should neither be removed from the stack -nor modified before a corresponding call to @Lid{lua_closeslot}. +A slot marked as to-be-closed should not be removed from the stack +by any other function in the API except @Lid{lua_settop} or @Lid{lua_pop}, +unless previously deactivated by @Lid{lua_closeslot}. This function should not be called for an index -that is equal to or below an active to-be-closed index. +that is equal to or below an active to-be-closed slot. Note that, both in case of errors and of a regular return, by the time the @idx{__close} metamethod runs, diff --git a/testes/api.lua b/testes/api.lua index fb7e7085..c1bcb4b7 100644 --- a/testes/api.lua +++ b/testes/api.lua @@ -1130,7 +1130,7 @@ do -- closing resources with 'closeslot' _ENV.xxx = true local a = T.testC([[ - pushvalue 2 # stack: S, NR, CH + pushvalue 2 # stack: S, NR, CH, NR call 0 1 # create resource; stack: S, NR, CH, R toclose -1 # mark it to be closed pushvalue 2 # stack: S, NR, CH, R, NR @@ -1151,6 +1151,30 @@ do ]], newresource, check) assert(a == 3 and _ENV.xxx == nil) -- no extra items left in the stack + -- closing resources with 'pop' + local a = T.testC([[ + pushvalue 2 # stack: S, NR, CH, NR + call 0 1 # create resource; stack: S, NR, CH, R + toclose -1 # mark it to be closed + pushvalue 2 # stack: S, NR, CH, R, NR + call 0 1 # create another resource; stack: S, NR, CH, R, R + toclose -1 # mark it to be closed + pushvalue 3 # stack: S, NR, CH, R, R, CH + pushint 2 # there should be two open resources + call 1 0 # stack: S, NR, CH, R, R + pop 1 # pop second resource + pushvalue 3 # stack: S, NR, CH, R, CH + pushint 1 # there should be one open resource + call 1 0 # stack: S, NR, CH, R + pop 1 # pop other resource from the stack + pushvalue 3 # stack: S, NR, CH, CH + pushint 0 # there should be no open resources + call 1 0 # stack: S, NR, CH + pushint * + return 1 # return stack size + ]], newresource, check) + assert(a == 3) -- no extra items left in the stack + -- non-closable value local a, b = pcall(T.makeCfunc[[ pushint 32