More effort in avoiding errors in finalizers

Before calling a finalizer, Lua not only checks stack limits, but
actually ensures that a minimum number of slots are already allocated
for the call. (If it cannot ensure that, it postpones the finalizer.)
That avoids finalizers not running due to memory errors that the
programmer cannot control.

lapi.c

@@ -366,7 +366,7 @@ LUA_API int lua_compare (lua_State *L, int index1, int index2, int op) {
 }
 
 
-LUA_API unsigned (lua_numbertocstring) (lua_State *L, int idx, char *buff) {
+LUA_API unsigned lua_numbertocstring (lua_State *L, int idx, char *buff) {
   const TValue *o = index2value(L, idx);
   if (ttisnumber(o)) {
     unsigned len = luaO_tostringbuff(o, buff);
@@ -1201,11 +1201,16 @@ LUA_API int lua_gc (lua_State *L, int what, ...) {
     case LUA_GCSTEP: {
       lu_byte oldstp = g->gcstp;
       l_mem n = cast(l_mem, va_arg(argp, size_t));
+      l_mem newdebt;
       int work = 0;  /* true if GC did some work */
       g->gcstp = 0;  /* allow GC to run (other bits must be zero here) */
       if (n <= 0)
-        n = g->GCdebt;  /* force to run one basic step */
-      luaE_setdebt(g, g->GCdebt - n);
+        newdebt = 0;  /* force to run one basic step */
+      else if (g->GCdebt >= n - MAX_LMEM)  /* no overflow? */
+        newdebt = g->GCdebt - n;
+      else  /* overflow */
+        newdebt = -MAX_LMEM;  /* set debt to miminum value */
+      luaE_setdebt(g, newdebt);
       luaC_condGC(L, (void)0, work = 1);
       if (work && g->gcstate == GCSpause)  /* end of cycle? */
         res = 1;  /* signal it */

lauxlib.h

@@ -81,8 +81,8 @@ LUALIB_API int (luaL_checkoption) (lua_State *L, int arg, const char *def,
 LUALIB_API int (luaL_fileresult) (lua_State *L, int stat, const char *fname);
 LUALIB_API int (luaL_execresult) (lua_State *L, int stat);
 
-LUALIB_API void *luaL_alloc (void *ud, void *ptr, size_t osize,
-                                                  size_t nsize);
+LUALIB_API void *(luaL_alloc) (void *ud, void *ptr, size_t osize,
+                                                    size_t nsize);
 
 
 /* predefined references */
@@ -103,7 +103,7 @@ LUALIB_API int (luaL_loadstring) (lua_State *L, const char *s);
 
 LUALIB_API lua_State *(luaL_newstate) (void);
 
-LUALIB_API unsigned luaL_makeseed (lua_State *L);
+LUALIB_API unsigned (luaL_makeseed) (lua_State *L);
 
 LUALIB_API lua_Integer (luaL_len) (lua_State *L, int idx);
 

ldo.c

@@ -221,13 +221,21 @@ l_noret luaD_errerr (lua_State *L) {
 
 
 /*
-** Check whether stack has enough space to run a simple function (such
-** as a finalizer): At least BASIC_STACK_SIZE in the Lua stack and
-** 2 slots in the C stack.
+** Check whether stacks have enough space to run a simple function (such
+** as a finalizer): At least BASIC_STACK_SIZE in the Lua stack, two
+** available CallInfos, and two "slots" in the C stack.
 */
 int luaD_checkminstack (lua_State *L) {
-  return ((stacksize(L) < MAXSTACK - BASIC_STACK_SIZE) &&
-          (getCcalls(L) < LUAI_MAXCCALLS - 2));
+  if (getCcalls(L) >= LUAI_MAXCCALLS - 2)
+    return 0;  /* not enough C-stack slots */
+  if (L->ci->next == NULL && luaE_extendCI(L, 0) == NULL)
+    return 0;  /* unable to allocate first ci */
+  if (L->ci->next->next == NULL && luaE_extendCI(L, 0) == NULL)
+    return 0;  /* unable to allocate second ci */
+  if (L->stack_last.p - L->top.p >= BASIC_STACK_SIZE)
+    return 1;  /* enough (BASIC_STACK_SIZE) free slots in the Lua stack */
+  else  /* try to grow stack to a size with enough free slots */
+    return luaD_growstack(L, BASIC_STACK_SIZE, 0);
 }
 
 
@@ -616,7 +624,7 @@ void luaD_poscall (lua_State *L, CallInfo *ci, int nres) {
 
 
 
-#define next_ci(L)  (L->ci->next ? L->ci->next : luaE_extendCI(L))
+#define next_ci(L)  (L->ci->next ? L->ci->next : luaE_extendCI(L, 1))
 
 
 /*

lgc.c

@@ -1293,7 +1293,7 @@ static void finishgencycle (lua_State *L, global_State *g) {
   correctgraylists(g);
   checkSizes(L, g);
   g->gcstate = GCSpropagate;  /* skip restart */
-  if (!g->gcemergency && luaD_checkminstack(L))
+  if (g->tobefnz != NULL && !g->gcemergency && luaD_checkminstack(L))
     callallpendingfinalizers(L);
 }
 
@@ -1672,7 +1672,7 @@ static l_mem singlestep (lua_State *L, int fast) {
         GCTM(L);  /* call one finalizer */
         stepresult = CWUFIN;
       }
-      else {  /* no more finalizers or emergency mode or no enough stack
+      else {  /* no more finalizers or emergency mode or not enough stack
                  to run finalizers */
         g->gcstate = GCSpause;  /* finish collection */
         stepresult = step2pause;

lstate.c

@@ -68,14 +68,19 @@ void luaE_setdebt (global_State *g, l_mem debt) {
 }
 
 
-CallInfo *luaE_extendCI (lua_State *L) {
+CallInfo *luaE_extendCI (lua_State *L, int err) {
   CallInfo *ci;
-  lua_assert(L->ci->next == NULL);
-  ci = luaM_new(L, CallInfo);
-  lua_assert(L->ci->next == NULL);
-  L->ci->next = ci;
+  ci = luaM_reallocvector(L, NULL, 0, 1, CallInfo);
+  if (l_unlikely(ci == NULL)) {  /* allocation failed? */
+    if (err)
+      luaM_error(L);  /* raise the error */
+    return NULL;  /* else only report it */
+  }
+  ci->next = L->ci->next;
   ci->previous = L->ci;
-  ci->next = NULL;
+  L->ci->next = ci;
+  if (ci->next)
+    ci->next->previous = ci;
   ci->u.l.trap = 0;
   L->nci++;
   return ci;

lstate.h

@@ -438,7 +438,7 @@ union GCUnion {
 LUAI_FUNC void luaE_setdebt (global_State *g, l_mem debt);
 LUAI_FUNC void luaE_freethread (lua_State *L, lua_State *L1);
 LUAI_FUNC lu_mem luaE_threadsize (lua_State *L);
-LUAI_FUNC CallInfo *luaE_extendCI (lua_State *L);
+LUAI_FUNC CallInfo *luaE_extendCI (lua_State *L, int err);
 LUAI_FUNC void luaE_shrinkCI (lua_State *L);
 LUAI_FUNC void luaE_checkcstack (lua_State *L);
 LUAI_FUNC void luaE_incCstack (lua_State *L);

lstrlib.c

@@ -141,8 +141,8 @@ static int str_rep (lua_State *L) {
   const char *s = luaL_checklstring(L, 1, &len);
   lua_Integer n = luaL_checkinteger(L, 2);
   const char *sep = luaL_optlstring(L, 3, "", &lsep);
-  if (n <= 0)
-    lua_pushliteral(L, "");
+  if (n <= 0 || (len | lsep) == 0)
+    lua_pushliteral(L, "");  /* no repetitions or both strings empty */
   else if (l_unlikely(len > MAX_SIZE - lsep ||
                cast_st2S(len + lsep) > cast_st2S(MAX_SIZE) / n))
     return luaL_error(L, "resulting string too large");
@@ -968,7 +968,7 @@ static int str_gsub (lua_State *L) {
     reprepstate(&ms);  /* (re)prepare state for new match */
     if ((e = match(&ms, src, p)) != NULL && e != lastmatch) {  /* match? */
       n++;
-      changed = add_value(&ms, &b, src, e, tr) | changed;
+      changed = add_value(&ms, &b, src, e, tr) || changed;
       src = lastmatch = e;
     }
     else if (src < ms.src_end)  /* otherwise, skip one character */
@@ -1726,7 +1726,7 @@ static int str_packsize (lua_State *L) {
     luaL_argcheck(L, opt != Kstring && opt != Kzstr, 1,
                      "variable-length format");
     size += ntoalign;  /* total space used by option */
-    luaL_argcheck(L, totalsize <= LUA_MAXINTEGER - size,
+    luaL_argcheck(L, totalsize <= MAX_SIZE - size,
                      1, "format result too large");
     totalsize += size;
   }

ltable.c

@@ -651,10 +651,9 @@ static void reinserthash (lua_State *L, Table *ot, Table *t) {
 
 
 /*
-** Exchange the hash part of 't1' and 't2'. (In 'flags', only the
-** dummy bit must be exchanged: The 'isrealasize' is not related
-** to the hash part, and the metamethod bits do not change during
-** a resize, so the "real" table can keep their values.)
+** Exchange the hash part of 't1' and 't2'. (In 'flags', only the dummy
+** bit must be exchanged:  The metamethod bits do not change during a
+** resize, so the "real" table can keep their values.)
 */
 static void exchangehashpart (Table *t1, Table *t2) {
   lu_byte lsizenode = t1->lsizenode;
@@ -1156,14 +1155,15 @@ void luaH_finishset (lua_State *L, Table *t, const TValue *key,
   lua_assert(hres != HOK);
   if (hres == HNOTFOUND) {
     TValue aux;
+    const TValue *actk = key;  /* actual key to insert */
     if (l_unlikely(ttisnil(key)))
       luaG_runerror(L, "table index is nil");
     else if (ttisfloat(key)) {
       lua_Number f = fltvalue(key);
       lua_Integer k;
-      if (luaV_flttointeger(f, &k, F2Ieq)) {
-        setivalue(&aux, k);  /* key is equal to an integer */
-        key = &aux;  /* insert it as an integer */
+      if (luaV_flttointeger(f, &k, F2Ieq)) {  /* is key equal to an integer? */
+        setivalue(&aux, k);
+        actk = &aux;  /* use the integer as the key */
       }
       else if (l_unlikely(luai_numisnan(f)))
         luaG_runerror(L, "table index is NaN");
@@ -1176,7 +1176,7 @@ void luaH_finishset (lua_State *L, Table *t, const TValue *key,
       L->top.p--;
       return;
     }
-    luaH_newkey(L, t, key, value);
+    luaH_newkey(L, t, actk, value);
   }
   else if (hres > 0) {  /* regular Node? */
     setobj2t(L, gval(gnode(t, hres - HFIRSTNODE)), value);

ltests.c

@@ -1106,6 +1106,27 @@ static int stacklevel (lua_State *L) {
 }
 
 
+static int resetCI (lua_State *L) {
+  CallInfo *ci = L->ci;
+  while (ci->next != NULL) {
+    CallInfo *tofree = ci->next;
+    ci->next = ci->next->next;
+    luaM_free(L, tofree);
+    L->nci--;
+  }
+  return 0;
+}
+
+
+static int reallocstack (lua_State *L) {
+  int n = cast_int(luaL_checkinteger(L, 1));
+  lua_lock(L);
+  luaD_reallocstack(L, cast_int(L->top.p - L->stack.p) + n, 1);
+  lua_unlock(L);
+  return 0;
+}
+
+
 static int table_query (lua_State *L) {
   const Table *t;
   int i = cast_int(luaL_optinteger(L, 2, -1));
@@ -2182,6 +2203,8 @@ static const struct luaL_Reg tests_funcs[] = {
   {"s2d", s2d},
   {"sethook", sethook},
   {"stacklevel", stacklevel},
+  {"resetCI", resetCI},
+  {"reallocstack", reallocstack},
   {"sizes", get_sizes},
   {"testC", testC},
   {"makeCfunc", makeCfunc},

ltm.h

@@ -49,7 +49,7 @@ typedef enum {
 ** Mask with 1 in all fast-access methods. A 1 in any of these bits
 ** in the flag of a (meta)table means the metatable does not have the
 ** corresponding metamethod field. (Bit 6 of the flag indicates that
-** the table is using the dummy node; bit 7 is used for 'isrealasize'.)
+** the table is using the dummy node.)
 */
 #define maskflags	cast_byte(~(~0u << (TM_EQ + 1)))
 

testes/gc.lua

@@ -707,4 +707,46 @@ end
 
 collectgarbage(oldmode)
 
+
+if T then
+  print("testing stack issues when calling finalizers")
+
+  local X
+  local obj
+
+  local function initobj ()
+    X = false
+    obj = setmetatable({}, {__gc = function ()  X = true end})
+  end
+
+  local function loop (n)
+    if n > 0 then loop(n - 1) end
+  end
+
+  -- should not try to call finalizer without a CallInfo available
+  initobj()
+  loop(20)   -- ensure stack space
+  T.resetCI()   -- remove extra CallInfos
+  T.alloccount(0)   -- cannot allocate more CallInfos
+  obj = nil
+  collectgarbage()   -- will not call finalizer
+  T.alloccount()
+  assert(X == false)
+  collectgarbage()   -- now will call finalizer (it was still pending)
+  assert(X == true)
+
+  -- should not try to call finalizer without stack space available
+  initobj()
+  loop(5)     -- ensure enough CallInfos
+  T.reallocstack(0)   -- remove extra stack slots
+  T.alloccount(0)   -- cannot reallocate stack
+  obj = nil
+  collectgarbage()   -- will not call finalizer
+  T.alloccount()
+  assert(X == false)
+  collectgarbage()   -- now will call finalizer (it was still pending)
+  assert(X == true)
+end
+
+
 print('OK')

testes/memerr.lua

@@ -282,6 +282,25 @@ testamem("growing stack", function ()
   return foo(100)
 end)
 
+
+collectgarbage()
+collectgarbage()
+global io, T, setmetatable, collectgarbage, print
+
+local Count = 0
+testamem("finalizers", function ()
+  local X = false
+  local obj = setmetatable({}, {__gc = function ()  X = true end})
+  obj = nil
+  T.resetCI()   -- remove extra CallInfos
+  T.reallocstack(18)   -- remove extra stack slots
+  Count = Count + 1
+  io.stderr:write(Count, "\n")
+  T.trick(io)
+  collectgarbage()
+  return X
+end)
+
 -- }==================================================================
 
 

testes/tracegc.lua

@@ -1,10 +1,15 @@
 -- track collections
 
+
 local M = {}
 
 -- import list
-local setmetatable, stderr, collectgarbage =
-         setmetatable, io.stderr, collectgarbage
+local stderr, collectgarbage = io.stderr, collectgarbage
+
+-- the debug version of setmetatable does not create any object (such as
+-- a '__metatable' string), and so it is more appropriate to be used in
+-- a finalizer
+local setmetatable = require"debug".setmetatable
 
 global none