CallInfo bit CIST_CLSRET broken in two

Since commit f407b3c4a, it was being used for two distinct (and
incompatible) meanings:
A: Function has TBC variables (now bit CIST_TBC)
B: Interpreter is closing TBC variables (original bit CIST_CLSRET)

B implies A, but A does not imply B.

4 files changed, 44 insertions, 15 deletions

diff --git a/lapi.c b/lapi.c
index c0fd1a1b..7b30617f 100644
--- a/lapi.c
+++ b/lapi.c
@@ -195,7 +195,7 @@ LUA_API void lua_settop (lua_State *L, int idx) {
   }
   newtop = L->top.p + diff;
   if (diff < 0 && L->tbclist.p >= newtop) {
-    lua_assert(ci->callstatus & CIST_CLSRET);
+    lua_assert(ci->callstatus & CIST_TBC);
     newtop = luaF_close(L, newtop, CLOSEKTOP, 0);
   }
   L->top.p = newtop;  /* correct top only after closing any upvalue */
@@ -207,7 +207,7 @@ LUA_API void lua_closeslot (lua_State *L, int idx) {
   StkId level;
   lua_lock(L);
   level = index2stack(L, idx);
-  api_check(L, (L->ci->callstatus & CIST_CLSRET) && L->tbclist.p == level,
+  api_check(L, (L->ci->callstatus & CIST_TBC) && (L->tbclist.p == level),
      "no variable to close at given level");
   level = luaF_close(L, level, CLOSEKTOP, 0);
   setnilvalue(s2v(level));
@@ -1280,7 +1280,7 @@ LUA_API void lua_toclose (lua_State *L, int idx) {
   o = index2stack(L, idx);
   api_check(L, L->tbclist.p < o, "given index below or equal a marked one");
   luaF_newtbcupval(L, o);  /* create new to-be-closed upvalue */
-  L->ci->callstatus |= CIST_CLSRET;  /* mark that function has TBC slots */
+  L->ci->callstatus |= CIST_TBC;  /* mark that function has TBC slots */
   lua_unlock(L);
 }
 
diff --git a/ldo.c b/ldo.c
index 994ad6f0..31c00a21 100644
--- a/ldo.c
+++ b/ldo.c
@@ -505,7 +505,7 @@ l_sinline void genmoveresults (lua_State *L, StkId res, int nres,
 ** Given 'nres' results at 'firstResult', move 'fwanted-1' of them
 ** to 'res'.  Handle most typical cases (zero results for commands,
 ** one result for expressions, multiple results for tail calls/single
-** parameters) separated. The flag CIST_CLSRET in 'fwanted', if set,
+** parameters) separated. The flag CIST_TBC in 'fwanted', if set,
 ** forces the swicth to go to the default case.
 */
 l_sinline void moveresults (lua_State *L, StkId res, int nres,
@@ -526,8 +526,9 @@ l_sinline void moveresults (lua_State *L, StkId res, int nres,
       break;
     default: {  /* two/more results and/or to-be-closed variables */
       int wanted = get_nresults(fwanted);
-      if (fwanted & CIST_CLSRET) {  /* to-be-closed variables? */
+      if (fwanted & CIST_TBC) {  /* to-be-closed variables? */
         L->ci->u2.nres = nres;
+        L->ci->callstatus |= CIST_CLSRET;  /* in case of yields */
         res = luaF_close(L, res, CLOSEKTOP, 1);
         L->ci->callstatus &= ~CIST_CLSRET;
         if (L->hookmask) {  /* if needed, call hook after '__close's */
@@ -552,8 +553,8 @@ l_sinline void moveresults (lua_State *L, StkId res, int nres,
 ** that.
 */
 void luaD_poscall (lua_State *L, CallInfo *ci, int nres) {
-  l_uint32 fwanted = ci->callstatus & (CIST_CLSRET | CIST_NRESULTS);
-  if (l_unlikely(L->hookmask) && !(fwanted & CIST_CLSRET))
+  l_uint32 fwanted = ci->callstatus & (CIST_TBC | CIST_NRESULTS);
+  if (l_unlikely(L->hookmask) && !(fwanted & CIST_TBC))
     rethook(L, ci, nres);
   /* move results to proper place */
   moveresults(L, ci->func.p, nres, fwanted);
@@ -785,7 +786,8 @@ static int finishpcallk (lua_State *L,  CallInfo *ci) {
 */
 static void finishCcall (lua_State *L, CallInfo *ci) {
   int n;  /* actual number of results from C function */
-  if (ci->callstatus & CIST_CLSRET) {  /* was returning? */
+  if (ci->callstatus & CIST_CLSRET) {  /* was closing TBC variable? */
+    lua_assert(ci->callstatus & CIST_TBC);
     n = ci->u2.nres;  /* just redo 'luaD_poscall' */
     /* don't need to reset CIST_CLSRET, as it will be set again anyway */
   }
diff --git a/lstate.h b/lstate.h
index e95c7288..635f41d2 100644
--- a/lstate.h
+++ b/lstate.h
@@ -235,8 +235,10 @@ struct CallInfo {
 #define CIST_FRESH      cast(l_uint32, CIST_C << 1)
 /* function is closing tbc variables */
 #define CIST_CLSRET     (CIST_FRESH << 1)
+/* function has tbc variables to close */
+#define CIST_TBC        (CIST_CLSRET << 1)
 /* original value of 'allowhook' */
-#define CIST_OAH        (CIST_CLSRET << 1)
+#define CIST_OAH        (CIST_TBC << 1)
 /* call is running a debug hook */
 #define CIST_HOOKED     (CIST_OAH << 1)
 /* doing a yieldable protected call */
diff --git a/testes/coroutine.lua b/testes/coroutine.lua
index c1252ab8..78b9bdca 100644
--- a/testes/coroutine.lua
+++ b/testes/coroutine.lua
@@ -515,7 +515,7 @@ else
   print "testing yields inside hooks"
 
   local turn
-  
+
   local function fact (t, x)
     assert(turn == t)
     if x == 0 then return 1
@@ -642,7 +642,7 @@ else
 
 
   print "testing coroutine API"
-  
+
   -- reusing a thread
   assert(T.testC([[
     newthread      # create thread
@@ -920,7 +920,7 @@ do   -- a few more tests for comparison operators
     until res ~= 10
     return res
   end
-  
+
   local function test ()
     local a1 = setmetatable({x=1}, mt1)
     local a2 = setmetatable({x=2}, mt2)
@@ -932,7 +932,7 @@ do   -- a few more tests for comparison operators
     assert(2 >= a2)
     return true
   end
-  
+
   run(test)
 
 end
@@ -1037,6 +1037,31 @@ f = T.makeCfunc([[
         return *
 ]], 23, "huu")
 
+
+do  -- testing bug introduced in commit f407b3c4a
+  local X = false   -- flag "to be closed"
+  local coro = coroutine.wrap(T.testC)
+  -- runs it until 'pcallk' (that yields)
+  -- 4th argument (at index 4): object to be closed
+  local res1, res2 = coro(
+    [[
+      toclose 3   # this could break the next 'pcallk'
+      pushvalue 2   # push function 'yield' to call it
+      pushint 22; pushint 33    # arguments to yield
+      # calls 'yield' (2 args; 2 results; continuation function at index 4)
+      pcallk 2 2 4
+      invalid command (should not arrive here)
+    ]],   -- 1st argument (at index 1): code;
+    coroutine.yield,  -- (at index 2): function to be called
+    func2close(function () X = true end),   -- (index 3): TBC slot
+    "pushint 43; return 3"   -- (index 4): code for continuation function
+  )
+
+  assert(res1 == 22 and res2 == 33 and not X)
+  local res1, res2, res3 = coro(34, "hi")  -- runs continuation function
+  assert(res1 == 34 and res2 == "hi" and res3 == 43 and X)
+end
+
 x = coroutine.wrap(f)
 assert(x() == 102)
 eqtab({x()}, {23, "huu"})
@@ -1094,11 +1119,11 @@ co = coroutine.wrap(function (...) return
           cannot be here!
        ]],
        [[  # 1st continuation
-         yieldk 0 3 
+         yieldk 0 3
          cannot be here!
        ]],
        [[  # 2nd continuation
-         yieldk 0 4 
+         yieldk 0 4
          cannot be here!
        ]],
        [[  # 3th continuation