try another fix for the L->top bug

docs/lua_internals_notes.rst

@@ -5,8 +5,26 @@ Lua Internals Notes
 
 These are rough notes - once something useful is identified it will be documented elsewhere.
 
+``L->top`` and ``L->ci->top``
+-----------------------------
+
+``L->top``
+  Appears to track the stack position where the next argument will be pushed (i.e. stack top).
+
 
 How is ``L->top`` updated
 -------------------------
 
+``api_incr_top``
+  Macro defined in ``lapi.h`` - increments L->top, and checks that it doesn't go beyond ``L->ci->top`` (stack overflow). So then this indicates that ``L->top`` cannot go beyond ``L->ci->top``.
+
+  Primarily used in ``lapi.c`` but also in ``ldo.c``, ``ldebug.c`` and ``lstate.c``.
+
+
+
+Lua calls Lua via OP_CALL
+  When this happens, the OP_RETURN must reset L->top to L->ci->top if posD_call() returned non-zero value.
 
+  This can only happen when OP_CALL is involved and the target is a Lua function. 
+  Since an interpreted Lua function will be executed via luaV_execute() which doesn't reset L->top for external calls, we need to handle this.
+  If the Lua function was JITed then we also need to reset.

include/ldo.h

@@ -29,7 +29,7 @@ typedef void (*Pfunc) (lua_State *L, void *ud);
 LUAI_FUNC int luaD_protectedparser (lua_State *L, ZIO *z, const char *name,
                                                   const char *mode);
 LUAI_FUNC void luaD_hook (lua_State *L, int event, int line);
-LUAI_FUNC int luaD_precall (lua_State *L, StkId func, int nresults);
+LUAI_FUNC int luaD_precall (lua_State *L, StkId func, int nresults, int op_call);
 LUAI_FUNC void luaD_call (lua_State *L, StkId func, int nResults,
                                         int allowyield);
 LUAI_FUNC int luaD_pcall (lua_State *L, Pfunc func, void *u,

include/lua.h

@@ -507,7 +507,7 @@ LUA_API void ravi_print_function(const struct Proto *f, int full);
 LUA_API void ravi_dump_function(lua_State *L);
 LUA_API void ravi_set_debuglevel(int level);
 
-#define DEBUG_STACK(p) if ((ravi_parser_debug & 8) != 0) {p;} else {}
+#define RAVI_DEBUG_STACK(p) if ((ravi_parser_debug & 8) != 0) {p;} else {}
 
 /******************************************************************************
 * Copyright (C) 1994-2015 Lua.org, PUC-Rio.

src/ldo.c

@@ -311,7 +311,7 @@ static void tryfuncTM (lua_State *L, StkId func) {
 /*
 ** returns true if function has been executed (C function)
 */
-int luaD_precall (lua_State *L, StkId func, int nresults) {
+int luaD_precall (lua_State *L, StkId func, int nresults, int op_call) {
   lua_CFunction f;
   CallInfo *ci;
   int n;  /* number of arguments (Lua) or returns (C) */
@@ -396,10 +396,12 @@ int luaD_precall (lua_State *L, StkId func, int nresults) {
         L->nny--;
         L->nCcalls--;
         lua_assert(L->ci == prevci);
-        if (b && isLua(L->ci)) 
+        if (op_call && b) {
+          lua_assert(isLua(L->ci));
           /* b is the value returned by luaD_poscall()
            */
           L->top = L->ci->top;
+        }
         /* Return a different value from 1 to
          * allow luaV_execute() to distinguish between
          * JITed function and true C function
@@ -412,7 +414,7 @@ int luaD_precall (lua_State *L, StkId func, int nresults) {
       luaD_checkstack(L, 1);  /* ensure space for metamethod */
       func = restorestack(L, funcr);  /* previous call may change stack */
       tryfuncTM(L, func);  /* try to get '__call' metamethod */
-      return luaD_precall(L, func, nresults);  /* now it must be a function */
+      return luaD_precall(L, func, nresults, op_call);  /* now it must be a function */
     }
   }
 }
@@ -457,7 +459,7 @@ void luaD_call (lua_State *L, StkId func, int nResults, int allowyield) {
       luaD_throw(L, LUA_ERRERR);  /* error while handing stack error */
   }
   if (!allowyield) L->nny++;
-  if (!luaD_precall(L, func, nResults))  /* is a Lua function? */
+  if (!luaD_precall(L, func, nResults, 0))  /* is a Lua function? */
     luaV_execute(L);  /* call it */
   if (!allowyield) L->nny--;
   L->nCcalls--;
@@ -581,7 +583,7 @@ static void resume (lua_State *L, void *ud) {
     if (ci != &L->base_ci)  /* not in base level? */
       resume_error(L, "cannot resume non-suspended coroutine", firstArg);
     /* coroutine is in base level; start running it */
-    if (!luaD_precall(L, firstArg - 1, LUA_MULTRET))  /* Lua function? */
+    if (!luaD_precall(L, firstArg - 1, LUA_MULTRET, 0))  /* Lua function? */
       luaV_execute(L);  /* call it */
   }
   else if (L->status != LUA_YIELD)

src/lvm.c

@@ -821,7 +821,6 @@ int luaV_execute (lua_State *L) {
   TValue *k;
   StkId base;
 newframe:  /* reentry point when frame changes (call/return) */
-  DEBUG_STACK(ravi_dump_stack(L, "On function entry");)
   lua_assert(ci == L->ci);
   cl = clLvalue(ci->func);
   k = cl->p->k;
@@ -836,9 +835,7 @@ newframe:  /* reentry point when frame changes (call/return) */
     }
     /* WARNING: several calls may realloc the stack and invalidate 'ra' */
     OpCode op = GET_OPCODE(i);
-#if 0
-    ravi_debug_trace(L, op, (ci->u.l.savedpc-cl->p->code)-1);
-#endif
+    RAVI_DEBUG_STACK(ravi_debug_trace(L, op, (ci->u.l.savedpc-cl->p->code)-1));
     ra = RA(i);
     lua_assert(base == ci->u.l.base);
     lua_assert(base <= L->top && L->top < L->stack + L->stacksize);
@@ -1134,13 +1131,17 @@ newframe:  /* reentry point when frame changes (call/return) */
         }
     } break;
     case OP_CALL: {
-        DEBUG_STACK(ravi_dump_stack(L, "OP_CALL: before luaD_precall()");)
         int b = GETARG_B(i);
         int nresults = GETARG_C(i) - 1;
         if (b != 0) {
           L->top = ra + b;  /* else previous instruction set top */
         }
-        int c_or_compiled = luaD_precall(L, ra, nresults);
+        /*
+        See note below under OP_RETURN for why we pass the extra
+        argument to luaD_precall() - it is basicaly to tell it that it
+        was called from OP_CALL instruction
+        */
+        int c_or_compiled = luaD_precall(L, ra, nresults, 1 /* OP_CALL */);
         if (c_or_compiled) {  /* C or Lua JITed function? */
           /* RAVI change - if the Lua function was JIT compiled then luaD_precall() returns 2 
            * A return value of 1 indicates non Lua C function
@@ -1162,7 +1163,12 @@ newframe:  /* reentry point when frame changes (call/return) */
         int b = GETARG_B(i);
         if (b != 0) L->top = ra+b;  /* else previous instruction set top */
         lua_assert(GETARG_C(i) - 1 == LUA_MULTRET);
-        if (luaD_precall(L, ra, LUA_MULTRET))  /* C function? or JIT call */
+        /* See note below under OP_RETURN for why we pass the extra
+           argument to luaD_precall() - it is basicaly to tell it that it
+           was called from OP_CALL instruction
+           */
+        if (luaD_precall(L, ra, LUA_MULTRET,
+                         1 /* OP_CALL */)) /* C function? or JIT call */
           base = ci->u.l.base;
         else {
           /* tail call: put called frame (n) in place of caller one (o) */
@@ -1189,20 +1195,24 @@ newframe:  /* reentry point when frame changes (call/return) */
         }
     } break;
     case OP_RETURN: {
-        DEBUG_STACK(ravi_dump_stack(L, "OP_RETURN: before luaD_poscall()");)
         int b = GETARG_B(i);
         if (cl->p->sizep > 0) luaF_close(L, base);
         int nres = (b != 0 ? b - 1 : L->top - ra);
         b = luaD_poscall(L, ra, nres);
         if (!(ci->callstatus & CIST_REENTRY))  /* 'ci' still the called one */ {
-          /* RAVI change in interpreted mode a Lua function call
-           * via OP_CALL always takes the other branch but in JIT mode
-           * this branch is taken.  
-           */
-          //if (b && isLua(L->ci)) {
-          //  L->top = L->ci->top;
-          //}
-          return b;  /* external invocation: return */
+          /* Lua VM assumes that this case is only
+             executed when luaV_execute() is called externally
+             i.e. not via OP_CALL, but in JIT mode this is not true
+             as luaV_execute() will be called even from OP_CALL when
+             a particular function cannot be compiled. So we need
+             to somehow trigger the reset of L->top in this case.
+             Since luaV_execute is called from various places it is
+             more convenient to let the caller decide what to do -
+             so we simply return b here. The caller is either OP_CALL
+             in JIT mode (see how b is handled in OP_CALL JIT implementation)
+             or via luaD_precall() if a JITed function is invoked (see
+             ldo.c for how luaD_precall() handles this */
+          return b; /* external invocation: return */
         }
         else {  /* invocation via reentry: continue execution */
           ci = L->ci;

src/ravi_llvmcall.cpp

@@ -92,8 +92,9 @@ void RaviCodeGenerator::emit_CALL(RaviFunctionDef *def, int A, int B, int C, int
   // int c = luaD_precall(L, ra, nresults);  /* C or JITed function? */
   llvm::Value *ra = emit_gep_register(def, A);
   llvm::Value *precall_result =
-      CreateCall3(def->builder, def->luaD_precallF, def->L, ra,
-                  llvm::ConstantInt::get(def->types->C_intT, nresults));
+      CreateCall4(def->builder, def->luaD_precallF, def->L, ra,
+                  llvm::ConstantInt::get(def->types->C_intT, nresults),
+        def->types->kInt[1]);
   // If luaD_precall() returns 0 then we need to interpret the
   // Lua function
   llvm::Value *do_Lua_interp =

src/ravi_llvmcodegen.cpp

@@ -778,11 +778,10 @@ void RaviCodeGenerator::emit_dump_stacktop(RaviFunctionDef *def, const char *str
 
 void RaviCodeGenerator::emit_debug_trace(RaviFunctionDef *def, int opCode,
                                          int pc) {
-#if 0
-  CreateCall3(def->builder, def->ravi_debug_traceF, def->L,
-              llvm::ConstantInt::get(def->types->C_intT, opCode),
-              llvm::ConstantInt::get(def->types->C_intT, pc));
-#endif
+  RAVI_DEBUG_STACK(
+      CreateCall3(def->builder, def->ravi_debug_traceF, def->L,
+                  llvm::ConstantInt::get(def->types->C_intT, opCode),
+                  llvm::ConstantInt::get(def->types->C_intT, pc)));
 }
 
 void RaviCodeGenerator::emit_raise_lua_error(RaviFunctionDef *def,

src/ravi_llvmtypes.cpp

@@ -687,11 +687,12 @@ LuaLLVMTypes::LuaLLVMTypes(llvm::LLVMContext &context) : mdbuilder(context) {
   luaC_upvalbarrierT =
       llvm::FunctionType::get(llvm::Type::getVoidTy(context), elements, false);
 
-  // int luaD_precall (lua_State *L, StkId func, int nresults);
+  // int luaD_precall (lua_State *L, StkId func, int nresults, int op_call);
   elements.clear();
   elements.push_back(plua_StateT);
   elements.push_back(StkIdT);
   elements.push_back(C_intT);
+  elements.push_back(C_intT);
   luaD_precallT = llvm::FunctionType::get(C_intT, elements, false);
 
   // void luaD_call (lua_State *L, StkId func, int nResults,