Lua 5.2 beta: trap with lua_getglobal

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

Lua 5.2 beta: trap with lua_getglobal

Patrick Rapin
I just had a crash with Lua 5.2 and it took me a little to understand why.
The problem is with the following instruction:

  lua_getglobal(L, lua_tostring(L, -1));

With Lua 5.1, this is no problem. The macro expands to

  lua_getfield(L, LUA_GLOBALSINDEX,  lua_tostring(L, -1))

But on Lua 5.2, the macro now expands to:

  (lua_pushglobaltable(L), lua_getfield(L, -1, ( lua_tostring(L,
-1))), lua_remove(L, -2))

And when the lua_tostring is evaluated, the stack has changed !

I propose to rewrite the macro with something like:

#define lua_getglobal(L,s) \
        do { const char* S=s; lua_pushglobaltable(L); lua_getfield(L, -1, S);
lua_remove(L, -2); } while(0)

Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.2 beta: trap with lua_getglobal

liam mail
On 22 October 2011 12:55, Patrick Rapin <[hidden email]> wrote:

> I just had a crash with Lua 5.2 and it took me a little to understand why.
> The problem is with the following instruction:
>
>  lua_getglobal(L, lua_tostring(L, -1));
>
> With Lua 5.1, this is no problem. The macro expands to
>
>  lua_getfield(L, LUA_GLOBALSINDEX,  lua_tostring(L, -1))
>
> But on Lua 5.2, the macro now expands to:
>
>  (lua_pushglobaltable(L), lua_getfield(L, -1, ( lua_tostring(L,
> -1))), lua_remove(L, -2))
>
> And when the lua_tostring is evaluated, the stack has changed !
>
> I propose to rewrite the macro with something like:
>
> #define lua_getglobal(L,s) \
>        do { const char* S=s; lua_pushglobaltable(L); lua_getfield(L, -1, S);
> lua_remove(L, -2); } while(0)
>
>

Yes this is correct as you are using a relative rather than absolute
address and your proposed solution looks good for relative indexes,
although I would question the do while block. The do while block scope
is recommended by numerous FAQs, books et al, yet with a high enough
compiler warning level and the wrong compiler it will generate user
warnings in code. *

* The warning will be something like conditional expression is
constant IIRC and you can use a compiler pragma or empty for loop to
remove the warnings yet it looks ugly as sin.

Liam

Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.2 beta: trap with lua_getglobal

Ingo van Lil
On 22/10/2011 15:03, liam mail wrote:

> I propose to rewrite the macro with something like:
>
> #define lua_getglobal(L,s) \
>         do { const char* S=s; lua_pushglobaltable(L); lua_getfield(L, -1, S);
> lua_remove(L, -2); } while(0)
>
>
> Yes this is correct as you are using a relative rather than absolute
> address and your proposed solution looks good for relative indexes,
> although I would question the do while block. The do while block scope
> is recommended by numerous FAQs, books et al, yet with a high enough
> compiler warning level and the wrong compiler it will generate user
> warnings in code. *

The 'do {} while(0)' trick is not just about limiting scope, its primary
purpose is to guarantee that you can safely use the macro like a regular
function in all contexts. Without the dummy loop you'd get unexpected
results e.g. when using:

if (foo)
     lua_getglobal(L, x);

I'd also expect the trick to be common enough to not generate a warning
in most compilers, but that's only a guess. There is however another
potential source of warnings: The variable 'S' might shadow another
identifier declared outside. It should at least be changed to a less
commonly used name.

I guess the only safe solution is to make lua_getglobal() a real function.

Regards,
Ingo

Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.2 beta: trap with lua_getglobal

Patrick Donnelly
In reply to this post by Patrick Rapin
On Sat, Oct 22, 2011 at 7:55 AM, Patrick Rapin <[hidden email]> wrote:

> I just had a crash with Lua 5.2 and it took me a little to understand why.
> The problem is with the following instruction:
>
>  lua_getglobal(L, lua_tostring(L, -1));
>
> With Lua 5.1, this is no problem. The macro expands to
>
>  lua_getfield(L, LUA_GLOBALSINDEX,  lua_tostring(L, -1))
>
> But on Lua 5.2, the macro now expands to:
>
>  (lua_pushglobaltable(L), lua_getfield(L, -1, ( lua_tostring(L,
> -1))), lua_remove(L, -2))
>
> And when the lua_tostring is evaluated, the stack has changed !
>
> I propose to rewrite the macro with something like:
>
> #define lua_getglobal(L,s) \
>        do { const char* S=s; lua_pushglobaltable(L); lua_getfield(L, -1, S);
> lua_remove(L, -2); } while(0)

There is another problem here as well I think. Since lua_getglobal
pushes 2 elements on the stack instead of just one, we could have a
stack overflow. Unlike the recently brought up auxiliary library [1],
lua_* C API functions can't blindly use "extra" stack space?

[1] http://lua-users.org/lists/lua-l/2011-10/msg00909.html

--
- Patrick Donnelly

Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.2 beta: trap with lua_getglobal

Luiz Henrique de Figueiredo
In reply to this post by Patrick Rapin
Thanks for catching this.

Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.2 beta: trap with lua_getglobal

Patrick Rapin
Just to be complete, in case you didn't notice, the macro
lua_setglobal does have to same problem.