Add the concept of a "global hook"

This is meant to address the following shortcomings in the current debug hooks, related to asynchronously setting them: * Coroutines not being interrupted by hooks that are unaware of them * Race conditions causing some tight loops to fail to be interrupted * Memory ordering problems when setting a hook from a different C thread
josephcsible · May 20, 2020 · 0d83524 · 0d83524
1 parent 0be57b9
commit 0d83524
Show file tree

Hide file tree

Showing 10 changed files with 170 additions and 28 deletions.
diff --git a/ldblib.c b/ldblib.c
@@ -27,6 +27,13 @@
 static const char *const HOOKKEY = "_HOOKKEY";
 
 
+/*
+** The hook function at registry[GLOBALHOOKKEY] contains the current
+** global hook function.
+*/
+static const char *const GLOBALHOOKKEY = "_GLOBALHOOKKEY";
+
+
 /*
 ** If L1 != L, L1 can be in any state, and therefore there are no
 ** guarantees about its stack space; any push in L1 must be
@@ -307,23 +314,37 @@ static int db_upvaluejoin (lua_State *L) {
 }
 
 
+static void commonhookf (lua_State *L, lua_Debug *ar) {
+  static const char *const hooknames[] =
+    {"call", "return", "line", "count", "tail call"};
+  lua_pushstring(L, hooknames[(int)ar->event]);  /* push event name */
+  if (ar->currentline >= 0)
+    lua_pushinteger(L, ar->currentline);  /* push current line */
+  else lua_pushnil(L);
+  lua_assert(lua_getinfo(L, "lS", ar));
+  lua_call(L, 2, 0);  /* call hook function */
+}
+
+
 /*
 ** Call hook function registered at hook table for the current
 ** thread (if there is one)
 */
 static void hookf (lua_State *L, lua_Debug *ar) {
-  static const char *const hooknames[] =
-    {"call", "return", "line", "count", "tail call"};
   lua_getfield(L, LUA_REGISTRYINDEX, HOOKKEY);
   lua_pushthread(L);
-  if (lua_rawget(L, -2) == LUA_TFUNCTION) {  /* is there a hook function? */
-    lua_pushstring(L, hooknames[(int)ar->event]);  /* push event name */
-    if (ar->currentline >= 0)
-      lua_pushinteger(L, ar->currentline);  /* push current line */
-    else lua_pushnil(L);
-    lua_assert(lua_getinfo(L, "lS", ar));
-    lua_call(L, 2, 0);  /* call hook function */
-  }
+  if (lua_rawget(L, -2) == LUA_TFUNCTION)  /* is there a hook function? */
+    commonhookf(L, ar);
+}
+
+
+/*
+** Call registered global hook function (if there is one)
+*/
+static void globalhookf (lua_State *L, lua_Debug *ar) {
+  /* is there a hook function? */
+  if (lua_getfield(L, LUA_REGISTRYINDEX, GLOBALHOOKKEY) == LUA_TFUNCTION)
+    commonhookf(L, ar);
 }
 
 
@@ -408,6 +429,35 @@ static int db_gethook (lua_State *L) {
 }
 
 
+static int db_setglobalhook (lua_State *L) {
+  lua_Hook func;
+  if (lua_isnoneornil(L, 1)) {  /* no hook? */
+    lua_settop(L, 1);
+    func = NULL;  /* turn off hooks */
+  }
+  else {
+    luaL_checktype(L, 1, LUA_TFUNCTION);
+    func = globalhookf;
+  }
+  lua_pushvalue(L, 1);  /* value (hook function) */
+  lua_setfield(L, LUA_REGISTRYINDEX, GLOBALHOOKKEY);
+  lua_setglobalhook(L, func);
+  return 0;
+}
+
+
+static int db_getglobalhook (lua_State *L) {
+  lua_Hook globalhook = lua_getglobalhook(L);
+  if (globalhook == NULL)  /* no hook? */
+    luaL_pushfail(L);
+  else if (globalhook != globalhookf)  /* external hook? */
+    lua_pushliteral(L, "external hook");
+  else  /* hook function must exist */
+    lua_getfield(L, LUA_REGISTRYINDEX, GLOBALHOOKKEY);
+  return 1;
+}
+
+
 static int db_debug (lua_State *L) {
   for (;;) {
     char buffer[250];
@@ -451,6 +501,7 @@ static int db_setcstacklimit (lua_State *L) {
 static const luaL_Reg dblib[] = {
   {"debug", db_debug},
   {"getuservalue", db_getuservalue},
+  {"getglobalhook", db_getglobalhook},
   {"gethook", db_gethook},
   {"getinfo", db_getinfo},
   {"getlocal", db_getlocal},
@@ -460,6 +511,7 @@ static const luaL_Reg dblib[] = {
   {"upvaluejoin", db_upvaluejoin},
   {"upvalueid", db_upvalueid},
   {"setuservalue", db_setuservalue},
+  {"setglobalhook", db_setglobalhook},
   {"sethook", db_sethook},
   {"setlocal", db_setlocal},
   {"setmetatable", db_setmetatable},

diff --git a/ldebug.c b/ldebug.c
@@ -163,6 +163,21 @@ LUA_API int lua_gethookcount (lua_State *L) {
 }
 
 
+/*
+** This function can be called asynchronously (e.g. during a signal),
+** under "reasonable" assumptions. We assume that pointers are atomic (e.g.,
+** gcc ensures that for all platforms where it runs).
+*/
+LUA_API void lua_setglobalhook (lua_State *L, lua_Hook func) {
+  G(L)->globalhook = func;
+}
+
+
+LUA_API lua_Hook lua_getglobalhook (lua_State *L) {
+  return G(L)->globalhook;
+}
+
+
 LUA_API int lua_getstack (lua_State *L, int level, lua_Debug *ar) {
   int status;
   CallInfo *ci;
@@ -795,9 +810,10 @@ static int changedline (const Proto *p, int oldpc, int newpc) {
 
 int luaG_traceexec (lua_State *L, const Instruction *pc) {
   CallInfo *ci = L->ci;
+  lua_Hook globalhook = G(L)->globalhook;
   lu_byte mask = L->hookmask;
   int counthook;
-  if (!(mask & (LUA_MASKLINE | LUA_MASKCOUNT))) {  /* no hooks? */
+  if (!globalhook && !(mask & (LUA_MASKLINE | LUA_MASKCOUNT))) {  /* no hooks? */
     ci->u.l.trap = 0;  /* don't need to stop again */
     return 0;  /* turn off 'trap' */
   }
@@ -806,24 +822,25 @@ int luaG_traceexec (lua_State *L, const Instruction *pc) {
   counthook = (--L->hookcount == 0 && (mask & LUA_MASKCOUNT));
   if (counthook)
     resethookcount(L);  /* reset count */
-  else if (!(mask & LUA_MASKLINE))
-    return 1;  /* no line hook and count != 0; nothing to be done now */
+  else if (!globalhook && !(mask & LUA_MASKLINE))
+    return 1;  /* no global or line hook and count != 0; nothing to be done now */
   if (ci->callstatus & CIST_HOOKYIELD) {  /* called hook last time? */
     ci->callstatus &= ~CIST_HOOKYIELD;  /* erase mark */
     return 1;  /* do not call hook again (VM yielded, so it did not move) */
   }
   if (!isIT(*(ci->u.l.savedpc - 1)))
     L->top = ci->top;  /* prepare top */
+  luaD_hook(L, globalhook, LUA_HOOKCOUNT, -1, 0, 0);  /* call global hook */
   if (counthook)
-    luaD_hook(L, LUA_HOOKCOUNT, -1, 0, 0);  /* call count hook */
+    luaD_hook(L, L->hook, LUA_HOOKCOUNT, -1, 0, 0);  /* call count hook */
   if (mask & LUA_MASKLINE) {
     const Proto *p = ci_func(ci)->p;
     int npci = pcRel(pc, p);
     if (npci == 0 ||  /* call linehook when enter a new function, */
         pc <= L->oldpc ||  /* when jump back (loop), or when */
         changedline(p, pcRel(L->oldpc, p), npci)) {  /* enter new line */
       int newline = luaG_getfuncline(p, npci);
-      luaD_hook(L, LUA_HOOKLINE, newline, 0, 0);  /* call line hook */
+      luaD_hook(L, L->hook, LUA_HOOKLINE, newline, 0, 0);  /* call line hook */
     }
     L->oldpc = pc;  /* 'pc' of last call to line hook */
   }

diff --git a/ldo.c b/ldo.c
@@ -272,9 +272,8 @@ void luaD_inctop (lua_State *L) {
 ** called. (Both 'L->hook' and 'L->hookmask', which trigger this
 ** function, can be changed asynchronously by signals.)
 */
-void luaD_hook (lua_State *L, int event, int line,
+void luaD_hook (lua_State *L, lua_Hook hook, int event, int line,
                               int ftransfer, int ntransfer) {
-  lua_Hook hook = L->hook;
   if (hook && L->allowhook) {  /* make sure there is a hook */
     int mask = CIST_HOOKED;
     CallInfo *ci = L->ci;
@@ -312,33 +311,39 @@ void luaD_hook (lua_State *L, int event, int line,
 ** active.
 */
 void luaD_hookcall (lua_State *L, CallInfo *ci) {
+  lua_Hook globalhook = G(L)->globalhook;
   int hook = (ci->callstatus & CIST_TAIL) ? LUA_HOOKTAILCALL : LUA_HOOKCALL;
   Proto *p;
-  if (!(L->hookmask & LUA_MASKCALL))  /* some other hook? */
+  if (!globalhook && !(L->hookmask & LUA_MASKCALL))  /* some other hook? */
     return;  /* don't call hook */
   p = clLvalue(s2v(ci->func))->p;
   L->top = ci->top;  /* prepare top */
   ci->u.l.savedpc++;  /* hooks assume 'pc' is already incremented */
-  luaD_hook(L, hook, -1, 1, p->numparams);
+  luaD_hook(L, globalhook, hook, -1, 1, p->numparams);
+  if (L->hookmask & LUA_MASKCALL)
+    luaD_hook(L, L->hook, hook, -1, 1, p->numparams);
   ci->u.l.savedpc--;  /* correct 'pc' */
 }
 
 
 static StkId rethook (lua_State *L, CallInfo *ci, StkId firstres, int nres) {
   ptrdiff_t oldtop = savestack(L, L->top);  /* hook may change top */
   int delta = 0;
+  lua_Hook globalhook = G(L)->globalhook;
   if (isLuacode(ci)) {
     Proto *p = clLvalue(s2v(ci->func))->p;
     if (p->is_vararg)
       delta = ci->u.l.nextraargs + p->numparams + 1;
     if (L->top < ci->top)
       L->top = ci->top;  /* correct top to run hook */
   }
-  if (L->hookmask & LUA_MASKRET) {  /* is return hook on? */
+  if (globalhook || (L->hookmask & LUA_MASKRET)) {  /* is return hook on? */
     int ftransfer;
     ci->func += delta;  /* if vararg, back to virtual 'func' */
     ftransfer = cast(unsigned short, firstres - ci->func);
-    luaD_hook(L, LUA_HOOKRET, -1, ftransfer, nres);  /* call it */
+    luaD_hook(L, globalhook, LUA_HOOKRET, -1, ftransfer, nres);  /* call it */
+    if (L->hookmask & LUA_MASKRET)
+      luaD_hook(L, L->hook, LUA_HOOKRET, -1, ftransfer, nres);  /* call it */
     ci->func -= delta;
   }
   if (isLua(ci->previous))
@@ -467,16 +472,19 @@ void luaD_call (lua_State *L, StkId func, int nresults) {
      Cfunc: {
       int n;  /* number of returns */
       CallInfo *ci;
+      lua_Hook globalhook = G(L)->globalhook;
       checkstackp(L, LUA_MINSTACK, func);  /* ensure minimum stack size */
       ci = next_ci(L);
       ci->nresults = nresults;
       ci->callstatus = CIST_C;
       ci->top = L->top + LUA_MINSTACK;
       ci->func = func;
       lua_assert(ci->top <= L->stack_last);
-      if (L->hookmask & LUA_MASKCALL) {
+      if (globalhook || (L->hookmask & LUA_MASKCALL)) {
         int narg = cast_int(L->top - func) - 1;
-        luaD_hook(L, LUA_HOOKCALL, -1, 1, narg);
+        luaD_hook(L, globalhook, LUA_HOOKCALL, -1, 1, narg);
+        if (L->hookmask & LUA_MASKCALL)
+          luaD_hook(L, L->hook, LUA_HOOKCALL, -1, 1, narg);
       }
       lua_unlock(L);
       n = (*f)(L);  /* do the actual call */

diff --git a/ldo.h b/ldo.h
@@ -53,7 +53,7 @@ typedef void (*Pfunc) (lua_State *L, void *ud);
 LUAI_FUNC void luaD_seterrorobj (lua_State *L, int errcode, StkId oldtop);
 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 void luaD_hook (lua_State *L, lua_Hook hook, int event, int line,
                                         int fTransfer, int nTransfer);
 LUAI_FUNC void luaD_hookcall (lua_State *L, CallInfo *ci);
 LUAI_FUNC void luaD_pretailcall (lua_State *L, CallInfo *ci, StkId func, int n);

diff --git a/lstate.c b/lstate.c
@@ -419,6 +419,7 @@ LUA_API lua_State *lua_newstate (lua_Alloc f, void *ud) {
   g->gcstepsize = LUAI_GCSTEPSIZE;
   setgcparam(g->genmajormul, LUAI_GENMAJORMUL);
   g->genminormul = LUAI_GENMINORMUL;
+  g->globalhook = NULL;
   for (i=0; i < LUA_NUMTAGS; i++) g->mt[i] = NULL;
   if (luaD_rawrunprotected(L, f_luaopen, NULL) != LUA_OK) {
     /* memory allocation error: free partial state */

diff --git a/lstate.h b/lstate.h
@@ -272,6 +272,7 @@ typedef struct global_State {
   lua_WarnFunction warnf;  /* warning function */
   void *ud_warn;         /* auxiliary data to 'warnf' */
   unsigned int Cstacklimit;  /* current limit for the C stack */
+  volatile lua_Hook globalhook;
 } global_State;
 
 

diff --git a/lua.c b/lua.c
@@ -42,7 +42,7 @@ static const char *progname = LUA_PROGNAME;
 */
 static void lstop (lua_State *L, lua_Debug *ar) {
   (void)ar;  /* unused arg. */
-  lua_sethook(L, NULL, 0, 0);  /* reset hook */
+  lua_setglobalhook(L, NULL);  /* reset hook */
   luaL_error(L, "interrupted!");
 }
 
@@ -55,7 +55,7 @@ static void lstop (lua_State *L, lua_Debug *ar) {
 */
 static void laction (int i) {
   signal(i, SIG_DFL); /* if another SIGINT happens, terminate process */
-  lua_sethook(globalL, lstop, LUA_MASKCALL | LUA_MASKRET | LUA_MASKCOUNT, 1);
+  lua_setglobalhook(globalL, lstop);
 }
 
 

diff --git a/lua.h b/lua.h
@@ -463,6 +463,8 @@ LUA_API void (lua_sethook) (lua_State *L, lua_Hook func, int mask, int count);
 LUA_API lua_Hook (lua_gethook) (lua_State *L);
 LUA_API int (lua_gethookmask) (lua_State *L);
 LUA_API int (lua_gethookcount) (lua_State *L);
+LUA_API void (lua_setglobalhook) (lua_State *L, lua_Hook func);
+LUA_API lua_Hook (lua_getglobalhook) (lua_State *L);
 
 LUA_API int (lua_setcstacklimit) (lua_State *L, unsigned int limit);
 

diff --git a/lvm.c b/lvm.c
@@ -1044,7 +1044,7 @@ void luaV_finishOp (lua_State *L) {
 
 
 
-#define updatetrap(ci)  (trap = ci->u.l.trap)
+#define updatetrap(ci)  (trap = ci->u.l.trap || G(L)->globalhook)
 
 #define updatebase(ci)	(base = ci->func + 1)
 
@@ -1134,7 +1134,7 @@ void luaV_execute (lua_State *L, CallInfo *ci) {
 #include "ljumptab.h"
 #endif
  tailcall:
-  trap = L->hookmask;
+  trap = L->hookmask || G(L)->globalhook;
   cl = clLvalue(s2v(ci->func));
   k = cl->p->k;
   pc = ci->u.l.savedpc;