likely regressions for debugging hooks x lua_resume() in 5.4

[Aurelien DARRAGON <adarragon@...>]

Hi list!

While playing with lua debug hooks in 5.4, I observed 2 strange
behaviors that didn't exist in 5.3.

I tried to isolate both issues and got 2 reproducers that should
hopefully help to understand them.

First, I encountered some random "C stack overflow" runtime errors when
running some dynamic lua code that used to run fine under 5.3.

Attached is a working reproducer that systematically fails on 5.4 and
works well on 5.3 (lua_c_stack_overflow.c)

While trying to troubleshoot myself by looking at lua source code, I
noticed that since commit 74d9905
(https://github.com/lua/lua/commit/74d99057a5146755e737c479850f87fd0e3b6868)
the nCcalls is incremented within resume() function, but it is not
decremented before leaving the function, so when calling resume() over
and over from the same parent context (ie: to resume after being
interrupted by a yield), nCcalls keeps growing until it reaches
LUAI_MAXCCALLS, although no recursion is involved.

In 5.3's lua_resume() implementation, we have an explicit nCcalls
decrement plus and extra assert at the end of the function, so I was
wondering if this could've been simply overlooked in 5.4?
Thus, I tried adding the 'nCcalls--' instruction that I thought was
missing at the end of the lua_resume() function, and it seemed to have
fixed the "C stack overflow" errors I was getting.
But I can't be 100% sure that this is the correct way to fix it since
not decrementing nCcalls here could've been intentional in the first place?

----------------------------------------------------------------------

Now for the second "regression" that I observed:
When executing functions that depend on value(s) being at a specific
index(es) on the stack, I noticed that they were randomly failing when
interrupted (yield through debug hooks) and resumed on 5.4, because some
of the values that would've been there on the stack if not interrupted
weren't there anymore.

I also isolated the issue through a second reproducer attached as
lua_missing_stack_arguments.c file (again, runs fine on 5.3 and fails on
5.4).

While looking at the source code, I noticed that this change was
probably intentional. Indeed, it was introduced in
https://github.com/lua/lua/commit/58aa09a0b91cf81779d6710d7f9d855bb9d3712f:
since that commit, resume(), when called after a yield performed from
inside a hook, now explicitly discards provided arguments.

But now I'm wondering why debug hooks behavior in 5.4 would differ from
5.3 implementation, given that this doesn't seem to be documented
anywhere (outside of the code I mean)?
How should we do to properly yield from a hook function and allow the
following resume() call to properly finish the lua execution, like if
nothing happened (from lua's script point of view)?

Best regards,
Aurelien

#include <stdio.h>
#include "lua.h"
#include "lualib.h"
#include "lauxlib.h"

static void lua_custom_hook(lua_State *L, lua_Debug *ar)
{
	if (lua_isyieldable(L)) {
		lua_yieldk(L, 0, 0, NULL);
		return;
	}
}

int main(int argc, char **argv) {

	lua_State *L = luaL_newstate();

	luaL_openlibs(L);

	const char *payload = "local it = 0\
			       while it < 1000\
			       do\
			         it = it + 1\
			       end";

	if (luaL_loadstring(L, payload) == 0) {
		int ret, nres;

resume:
		lua_sethook(L, lua_custom_hook, LUA_MASKCOUNT, 10);

/* not sure that passing the same pointer for 'from' and 'L'
 * lua_resume() function arguments is a valid use here, but it
 * helps to trigger the issue
 */
#if defined(LUA_VERSION_NUM) && LUA_VERSION_NUM >= 504
		ret = lua_resume(L, L, 0, &nres);
#else
		ret = lua_resume(L, L, 0);
#endif
		switch (ret) {
			case LUA_OK:
				printf("success.\n");
				break;
			case LUA_YIELD:
				goto resume;
			default:
				{
					const char *msg = NULL;

					if (lua_checkstack(L, -1))
						msg = lua_tostring(L, -1);
					printf("failure: '%s'.\n", (msg) ? msg : "N/A");
				}
				break;
		}
	}

	lua_close (L);
	return 0;
}

#include <stdio.h>
#include "lua.h"
#include "lualib.h"
#include "lauxlib.h"

static void lua_custom_hook(lua_State *L, lua_Debug *ar)
{
	if (lua_isyieldable(L)) {
		lua_yieldk(L, 0, 0, NULL);
		return;
	}
}

int main(int argc, char **argv) {

	lua_State *L = luaL_newstate();

	luaL_openlibs(L);

	if (luaL_loadstring(L, "print(\"upvalue: \" .. table.pack(...)[1])") == 0) {
		int ret, nres;

		lua_pushinteger(L, 1);
resume:
		lua_sethook(L, lua_custom_hook, LUA_MASKCOUNT, 1);
#if defined(LUA_VERSION_NUM) && LUA_VERSION_NUM >= 504
		ret = lua_resume(L, L, 1, &nres);
#else
		ret = lua_resume(L, L, 1);
#endif
		switch (ret) {
			case LUA_OK:
				printf("success.\n");
				break;
			case LUA_YIELD:
				goto resume;
			default:
				{
					const char *msg = NULL;

					if (lua_checkstack(L, -1))
						msg = lua_tostring(L, -1);
					printf("Failure: '%s'.\n", (msg) ? msg : "N/A");
				}
				break;
		}
	}

	lua_close (L);
	return 0;
}