tolua 4.0a: critical bug + patch

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

tolua 4.0a: critical bug + patch

Christian Vogler-2
The attached patch fixes a bug in tolua 4.0a that can cause bogus
object references to appear. These are to objects on the C stack that
have already been destroyed. Under certain circumstances - when the
C stack patterns are just so - these can cause a VERY difficult to find
and to reproduce crash in the lua GC.

I think it may be better to rewrite the functions in tolua_lb.c to use
the lua API directly, instead of creating tolua bindings for
them. With the new lua API, using tolua pkg files to bind functions
that manipulate the lua stack feels distinctly awkward and is
error-prone, as this bug demonstrates.

- Christian




Index: src/lib/tolua_gp.c
===================================================================
RCS file: /home3/cvogler/src/cvs/3rdParty/tolua/src/lib/tolua_gp.c,v
retrieving revision 1.1.1.1.4.1
diff -u -r1.1.1.1.4.1 tolua_gp.c
--- src/lib/tolua_gp.c  2001/03/06 03:50:09     1.1.1.1.4.1
+++ src/lib/tolua_gp.c  2001/03/11 23:10:33
@@ -40,7 +40,13 @@
 
 int tolua_getvalue (lua_State* L, int narg, int def)
 {
- return lua_gettop(L)<abs(narg) ? def : narg;
+ if (lua_gettop(L)<abs(narg))
+ {
+  lua_pushnil(L);
+  return lua_gettop(L);
+ }
+ else
+  return narg;
 }
 
 int tolua_getbool (lua_State* L, int narg, int def)
Reply | Threaded
Open this post in threaded view
|

Re: tolua 4.0a: critical bug + patch

Waldemar Celes-3
sorry about that. the constant LUA_NIL, that should be used as
a default value, is part of my TODO list, as documented in the tolua.h file.
your fix works fine assuming the default value is always nil.

-- waldemar

> The attached patch fixes a bug in tolua 4.0a that can cause bogus
> object references to appear. These are to objects on the C stack that
> have already been destroyed. Under certain circumstances - when the
> C stack patterns are just so - these can cause a VERY difficult to find
> and to reproduce crash in the lua GC.
>
> I think it may be better to rewrite the functions in tolua_lb.c to use
> the lua API directly, instead of creating tolua bindings for
> them. With the new lua API, using tolua pkg files to bind functions
> that manipulate the lua stack feels distinctly awkward and is
> error-prone, as this bug demonstrates.
>
> - Christian
>


Reply | Threaded
Open this post in threaded view
|

Re: tolua 4.0a: critical bug + patch

Christian Vogler-2
On Mon, Mar 12, 2001 at 04:38:56PM -0300, Waldemar Celes wrote:
> sorry about that. the constant LUA_NIL, that should be used as
> a default value, is part of my TODO list, as documented in the tolua.h file.
> your fix works fine assuming the default value is always nil.

I did a grep on this function in the tolua source code before applying
this fix, and currently the only usage of this function assumes nil as
a default value. So, at least in this version of tolua it should work
as intended. 

How exactly would it be possible to specify an object as a default
value that is not on the stack? By using lua_getref()?

- Christian