Spurious C stack overflow in 5.2

[Alexander Gavrilov <angavrilov@...>]

Hi,

I'd like to report an issue with the current lua version.

Description:

When a deep enough nesting sequence of yieldable pcalls is
used inside a coroutine, the nCcalls counter underflows
during unwind, which causes the stack overflow check to
signal a spurious stack overflow, manifesting as an
"error in error handling". The necessary nesting depth
to reproduce depends on the context around resume.

To reproduce:

Open http://www.lua.org/cgi-bin/demo, and enter the following code:

coroutine.wrap(function() print(pcall(pcall,pcall,pcall,pcall,pcall,error,3)) end)()

Removing even one pcall level makes it produce a normal result.

This patch appears to fix it:

---------------------------- src/ldo.c ----------------------------
@@ -403,7 +403,7 @@ static void finishCcall (lua_State *L) {
   lua_assert(ci->u.c.k != NULL);  /* must have a continuation */
   lua_assert(L->nny == 0);
   /* finish 'luaD_call' */
-  L->nCcalls--;
+  //L->nCcalls--;
   /* finish 'lua_callk' */
   adjustresults(L, ci->nresults);
   /* call continuation function */
@@ -513,7 +513,7 @@ static void resume (lua_State *L, void *ud) {
         api_checknelems(L, n);
         firstArg = L->top - n;  /* yield results come from continuation */
       }
-      L->nCcalls--;  /* finish 'luaD_call' */
+      //L->nCcalls--;  /* finish 'luaD_call' */
       luaD_poscall(L, firstArg);  /* finish 'luaD_precall' */
     }
     unroll(L, NULL);

Since the counter corresponds to the actual depth of the C
stack, and is automatically reset post-longjmp, no subsequent
'finishing' should be necessary for it.

Alexander


P.S. One more minor tweak: lua_absindex should really be called
     in the same stack context as existed on function entry:

-------------------------- src/lauxlib.c --------------------------
@@ -863,8 +863,8 @@ LUALIB_API int luaL_getsubtable (lua_State *L, int idx, const char *fname) {
   lua_getfield(L, idx, fname);
   if (lua_istable(L, -1)) return 1;  /* table already there */
   else {
-    idx = lua_absindex(L, idx);
     lua_pop(L, 1);  /* remove previous result */
+    idx = lua_absindex(L, idx);
     lua_newtable(L);
     lua_pushvalue(L, -1);  /* copy to be left at top */
     lua_setfield(L, idx, fname);  /* assign new table to field */