Re: Is this a bug?

[Mark Feldman <mfeldman@...>]

> Some days ago, our program crash. I found the crash in lua code. So I 
check lua code, found the stack overflow.

I can confirm on x86 Windows running Lua 5.1.4, it also threw an 
exception in my C# KopiLua port which implements a functionally 
indentical GC.

Looks like a crash bug alright, the adjust_varargs() function contains a 
call to luaC_checkGC() which can reduce the size of the current stack 
when luaD_precall() is in the middle of messing around with stack 
itself. The end result is that when ci->top is restored the stack has 
shrunk and can no longer accommadate p->maxstacksize entries. I think 
the real problem is that checkstacksizes() only takes L->top into 
account, which isn't really valid at the point this code is executed 
since it's half-way through initializing a function call. I guess an 
extra check could be added in luaD_precall, but it seems a bit ugly to 
let the GC erroneously shorten the stack and then have to go back and 
fix it up afterwards.

Personally my gut tells me that letting the GC mess with the stack when 
luaD_precall is in the middle of initializing a function call probably 
isn't a good idea so I'd suggest fixing it by simply removing the call 
to luaC_checkGC() in adjust_varargs(). Sample code below, patch file 
appears after that.

Mark Feldman

=== repro code ===

function foo(p1, p2, p3, p4, p5, p6)
   return 0
end

function bar(p1, p2, p3, p4, p5, p6, p7, p8, p9, p10, p11, p12, ...)
   return {
       0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
       0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
       0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
       0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
       0, 0, 0
   }
end

function baz()
   bar(foo(0,0,0,0,0,0), foo(0,0,0,0,0,0) )
   bar(foo(0,0,0,0,0,0), foo(0,0,0,0,0,0) )
   bar(foo(0,0,0,0,0,0), foo(0,0,0,0,0,0) )
   bar(foo(0,0,0,0,0,0), foo(0,0,0,0,0,0) )
   bar(foo(0,0,0,0,0,0), foo(0,0,0,0,0,0) )
   bar(foo(0,0,0,0,0,0), foo(0,0,0,0,0,0) )
   bar(foo(0,0,0,0,0,0), foo(0,0,0,0,0,0) )
   bar(foo(0,0,0,0,0,0), foo(0,0,0,0,0,0) )
   bar(foo(0,0,0,0,0,0), foo(0,0,0,0,0,0) )
   bar(foo(0,0,0,0,0,0), foo(0,0,0,0,0,0) )
   
end

baz()
foo(0,0,0,0,0,0)    --> crash!

=== patch ===

diff -ur c:\temp\lua-5.1.4/src/ldo.c lua-5.1.4/src/ldo.c
--- c:\temp\lua-5.1.4/src/ldo.c    2008-01-19 09:31:22.000000000 +1100
+++ lua-5.1.4/src/ldo.c    2010-05-10 14:34:27.093614600 +1000
@@ -216,7 +216,6 @@
  if (p->is_vararg & VARARG_NEEDSARG) { /* compat. with old-style 
vararg? */
    int nvar = actual - nfixargs;  /* number of extra arguments */
    lua_assert(p->is_vararg & VARARG_HASARG);
-    luaC_checkGC(L);
    htab = luaH_new(L, nvar, 1);  /* create `arg' table */
    for (i=0; i<nvar; i++)  /* put extra arguments into `arg' table */
      setobj2n(L, luaH_setnum(L, htab, i+1), L->top - nvar + i);


This message and its attachments may contain legally privileged or confidential information. This message is intended for the use of the individual or entity to which it is addressed. If you are not the addressee indicated in this message, or the employee or agent responsible for delivering the message to the intended recipient, you may not copy or deliver this message or its attachments to anyone. Rather, you should permanently delete this message and its attachments and kindly notify the sender by reply e-mail. Any content of this message and its attachments, which does not relate to the official business of the sending company must be taken not to have been sent or endorsed by the sending company or any of its related entities. No warranty is made that the e-mail or attachment(s) are free from computer virus or other defect.