Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fixed bug: invalid 'oldpc' when returning to a function
The field 'L->oldpc' is not always updated when control returns to a
function; an invalid value can seg. fault when computing 'changedline'.
(One example is an error in a finalizer; control can return to
'luaV_execute' without executing 'luaD_poscall'.) Instead of trying to
fix all possible corner cases, it seems safer to be resilient to invalid
values for 'oldpc'. Valid but wrong values at most cause an extra call
to a line hook.
  • Loading branch information
roberto-ieru committed Jul 17, 2020
1 parent 1ecfbfa commit a219564
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 21 deletions.
41 changes: 25 additions & 16 deletions ldebug.c
Expand Up @@ -33,10 +33,8 @@

#define noLuaClosure(f) ((f) == NULL || (f)->c.tt == LUA_VCCL)


/* Active Lua function (given call info) */
#define ci_func(ci) (clLvalue(s2v((ci)->func)))

/* inverse of 'pcRel' */
#define invpcRel(pc, p) ((p)->code + (pc) + 1)

static const char *funcnamefromcode (lua_State *L, CallInfo *ci,
const char **name);
Expand Down Expand Up @@ -127,20 +125,18 @@ static void settraps (CallInfo *ci) {
/*
** This function can be called during a signal, under "reasonable"
** assumptions.
** Fields 'oldpc', 'basehookcount', and 'hookcount' (set by
** 'resethookcount') are for debug only, and it is no problem if they
** get arbitrary values (causes at most one wrong hook call). 'hookmask'
** is an atomic value. We assume that pointers are atomic too (e.g., gcc
** ensures that for all platforms where it runs). Moreover, 'hook' is
** always checked before being called (see 'luaD_hook').
** Fields 'basehookcount' and 'hookcount' (set by 'resethookcount')
** are for debug only, and it is no problem if they get arbitrary
** values (causes at most one wrong hook call). 'hookmask' is an atomic
** value. We assume that pointers are atomic too (e.g., gcc ensures that
** for all platforms where it runs). Moreover, 'hook' is always checked
** before being called (see 'luaD_hook').
*/
LUA_API void lua_sethook (lua_State *L, lua_Hook func, int mask, int count) {
if (func == NULL || mask == 0) { /* turn off hooks? */
mask = 0;
func = NULL;
}
if (isLua(L->ci))
L->oldpc = L->ci->u.l.savedpc;
L->hook = func;
L->basehookcount = count;
resethookcount(L);
Expand Down Expand Up @@ -795,10 +791,24 @@ static int changedline (const Proto *p, int oldpc, int newpc) {
}


/*
** Traces the execution of a Lua function. Called before the execution
** of each opcode, when debug is on. 'L->oldpc' stores the last
** instruction traced, to detect line changes. When entering a new
** function, 'npci' will be zero and will test as a new line without
** the need for 'oldpc'; so, 'oldpc' does not need to be initialized
** before. Some exceptional conditions may return to a function without
** updating 'oldpc'. In that case, 'oldpc' may be invalid; if so, it is
** reset to zero. (A wrong but valid 'oldpc' at most causes an extra
** call to a line hook.)
*/
int luaG_traceexec (lua_State *L, const Instruction *pc) {
CallInfo *ci = L->ci;
lu_byte mask = L->hookmask;
const Proto *p = ci_func(ci)->p;
int counthook;
/* 'L->oldpc' may be invalid; reset it in this case */
int oldpc = (L->oldpc < p->sizecode) ? L->oldpc : 0;
if (!(mask & (LUA_MASKLINE | LUA_MASKCOUNT))) { /* no hooks? */
ci->u.l.trap = 0; /* don't need to stop again */
return 0; /* turn off 'trap' */
Expand All @@ -819,15 +829,14 @@ int luaG_traceexec (lua_State *L, const Instruction *pc) {
if (counthook)
luaD_hook(L, 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 */
pc <= invpcRel(oldpc, p) || /* when jump back (loop), or when */
changedline(p, oldpc, npci)) { /* enter new line */
int newline = luaG_getfuncline(p, npci);
luaD_hook(L, LUA_HOOKLINE, newline, 0, 0); /* call line hook */
}
L->oldpc = pc; /* 'pc' of last call to line hook */
L->oldpc = npci; /* 'pc' of last call to line hook */
}
if (L->status == LUA_YIELD) { /* did hook yield? */
if (counthook)
Expand Down
5 changes: 5 additions & 0 deletions ldebug.h
Expand Up @@ -13,6 +13,11 @@

#define pcRel(pc, p) (cast_int((pc) - (p)->code) - 1)


/* Active Lua function (given call info) */
#define ci_func(ci) (clLvalue(s2v((ci)->func)))


#define resethookcount(L) (L->hookcount = L->basehookcount)

/*
Expand Down
6 changes: 3 additions & 3 deletions ldo.c
Expand Up @@ -327,7 +327,7 @@ 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;
if (isLuacode(ci)) {
Proto *p = clLvalue(s2v(ci->func))->p;
Proto *p = ci_func(ci)->p;
if (p->is_vararg)
delta = ci->u.l.nextraargs + p->numparams + 1;
if (L->top < ci->top)
Expand All @@ -340,8 +340,8 @@ static StkId rethook (lua_State *L, CallInfo *ci, StkId firstres, int nres) {
luaD_hook(L, LUA_HOOKRET, -1, ftransfer, nres); /* call it */
ci->func -= delta;
}
if (isLua(ci->previous))
L->oldpc = ci->previous->u.l.savedpc; /* update 'oldpc' */
if (isLua(ci = ci->previous))
L->oldpc = pcRel(ci->u.l.savedpc, ci_func(ci)->p); /* update 'oldpc' */
return restorestack(L, oldtop);
}

Expand Down
1 change: 1 addition & 0 deletions lstate.c
Expand Up @@ -301,6 +301,7 @@ static void preinit_thread (lua_State *L, global_State *g) {
L->openupval = NULL;
L->status = LUA_OK;
L->errfunc = 0;
L->oldpc = 0;
}


Expand Down
2 changes: 1 addition & 1 deletion lstate.h
Expand Up @@ -286,7 +286,6 @@ struct lua_State {
StkId top; /* first free slot in the stack */
global_State *l_G;
CallInfo *ci; /* call info for current function */
const Instruction *oldpc; /* last pc traced */
StkId stack_last; /* last free slot in the stack */
StkId stack; /* stack base */
UpVal *openupval; /* list of open upvalues in this stack */
Expand All @@ -297,6 +296,7 @@ struct lua_State {
volatile lua_Hook hook;
ptrdiff_t errfunc; /* current error handling function (stack index) */
l_uint32 nCcalls; /* number of allowed nested C calls - 'nci' */
int oldpc; /* last pc traced */
int stacksize;
int basehookcount;
int hookcount;
Expand Down
2 changes: 1 addition & 1 deletion lvm.c
Expand Up @@ -1794,7 +1794,7 @@ void luaV_execute (lua_State *L, CallInfo *ci) {
ProtectNT(luaT_adjustvarargs(L, GETARG_A(i), ci, cl->p));
if (trap) {
luaD_hookcall(L, ci);
L->oldpc = pc + 1; /* next opcode will be seen as a "new" line */
L->oldpc = 1; /* next opcode will be seen as a "new" line */
}
updatebase(ci); /* function has new base after adjustment */
vmbreak;
Expand Down

1 comment on commit a219564

@spotrh
Copy link

@spotrh spotrh commented on a219564 Jul 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like there is a need for an equivalent fix for lua-5.3 ... is that likely?

Please sign in to comment.