Re: Lua 5.4.0-rc5 segfault in low memory conditions

[Sergey Zakharchenko <doublef.mobile@...>]

Andrew,

Sergey Zakharchenko <doublef.mobile@gmail.com>:
> > Andrew Gierth <andrew@tao11.riddles.org.uk>:
> > > See if this fixes both of them:
> > > --- src/lparser.c.orig  2020-06-13 19:57:47.428734000 +0100
> > > +++ src/lparser.c       2020-06-13 19:58:08.039573000 +0100
> >
> > Thanks for your awesome analysis! I'll start testing this now, but
> > please note one thing I hadn't mentioned before: the original issue
> > discovered on MIPS was actually discovered when loading bytecode, with
> > no parser involved. It's possible we aren't done with this yet...
>
> Huh, similar code in lundump.c has no barriers as well... Does it make
> sense to add them?
>
>     f->p[i] = luaF_newproto(S->L);
>    /* here */
>     loadFunction(S, f->p[i], f->source);
>
>   cl->p = luaF_newproto(L);
>    /* here */
>  loadFunction(&S, cl->p, NULL);

After applying the attached patch, I can no longer reproduce the issue
with a single allocation failure on x86-64. The patch may still be way
off, but at least it seems like it doesn't break normal operation on
x86-64 or MIPS. However, more serious testing on MIPS can still
trigger other issues. In particular, an assertion failure is triggered
here in lgc.c's traverseproto:

  for (i = 0; i < f->sizek; i++)  /* mark literals */
    markvalue(g, &f->k[i]); /* <--- here */

Other, possibly related, bad news is that I see more issues when
multiple (non-consecutive) failures are considered, at least on MIPS.
In particular, I'm seeing an assertion error in lvm.c's OP_LOADK
handling:

      vmcase(OP_LOADK) {
        TValue *rb = k + GETARG_Bx(i);
        setobj2s(L, ra, rb); /* <--- here */
        vmbreak;
      }

So, it seems string handling may still be a part of the problem(s)...
The particular OP_LOADK failure took 12 (out of ~3000) l_alloc calls
to fail.

I'll see if I can reproduce these issues on x86 with a simpler test
case I can share.

Best regards,

--
DoubleF

--- a/src/lparser.c
+++ b/src/lparser.c
@@ -1977,6 +1977,7 @@ LClosure *luaY_parser (lua_State *L, ZIO *z, Mbuffer *buff,
   sethvalue2s(L, L->top, lexstate.h);  /* anchor it */
   luaD_inctop(L);
   funcstate.f = cl->p = luaF_newproto(L);
+  luaC_objbarrier(L, cl, cl->p);
   funcstate.f->source = luaS_new(L, name);  /* create and anchor TString */
   luaC_objbarrier(L, funcstate.f, funcstate.f->source);
   lexstate.buff = buff;
--- a/src/lundump.c
+++ b/src/lundump.c
@@ -191,6 +191,7 @@ static void loadProtos (LoadState *S, Proto *f) {
     f->p[i] = NULL;
   for (i = 0; i < n; i++) {
     f->p[i] = luaF_newproto(S->L);
+    luaC_objbarrier(S->L, f, f->p[i]);
     loadFunction(S, f->p[i], f->source);
   }
 }
@@ -310,6 +311,7 @@ LClosure *luaU_undump(lua_State *L, ZIO *Z, const char *name) {
   setclLvalue2s(L, L->top, cl);
   luaD_inctop(L);
   cl->p = luaF_newproto(L);
+  luaC_objbarrier(L, cl, cl->p);
   loadFunction(&S, cl->p, NULL);
   lua_assert(cl->nupvalues == cl->p->sizeupvalues);
   luai_verifycode(L, buff, cl->p);