Bug in cpcall

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

Bug in cpcall

Patrick Donnelly-3
I believe cpcall does some improper memory handling somewhere because
it attempts to free an invalid pointer. The attached c code provides a
reproducible case. The basic idea is to limit the memory usage of a
lua_State. (Looking at the code) In my application, I don't actually
have to call cpcall multiple times (it actually happens the first
time).

It should compile as:
gcc -shared -o test.so test_alloc.c
And run as:
batrick@waterdeep:~/lua$ lua -e "require'test'"
Segmentation fault

This may happen in lua_pcall() as well (I haven't checked). There is a
bug in LuaTask using pcalls that cause a segfault and I get the
feeling this may be it.

-- 
-Patrick Donnelly

"One of the lessons of history is that nothing is often a good thing
to do and always a clever thing to say."

-Will Durant
#include <lua.h>
#include <lauxlib.h>
#include <stdio.h>

static size_t limit = 10000;
static size_t mem_used = 0;

static void *my_Alloc(void *ud, void *ptr, size_t osize, size_t nsize)
{
  (void) ud;
    if (nsize == 0)
    {
      mem_used -= osize;
      free(ptr);
      return NULL;
    }
    else if (nsize + mem_used > limit)
      return NULL;
    else
    {
      mem_used += nsize;
      return realloc(ptr, nsize);
    }
}

static int some_func(lua_State *L)
{
  return 0;
}

static int do_stuff(lua_State *L)
{
  lua_createtable(L, 0, 1);
  lua_pushcfunction(L, some_func);
  lua_setfield(L, -2, "__gc");
  lua_setfield(L, LUA_REGISTRYINDEX, "thread_value_gc_m");
  lua_createtable(L, 0, 100);
  lua_createtable(L, 0, 1);
  lua_pushstring(L, "__mode");
  lua_pushstring(L, "k");
  lua_settable(L, -3);
  lua_setmetatable(L, -2);
  lua_setfield(L, LUA_REGISTRYINDEX, "other_worldly_references");
  lua_createtable(L, 0, 100);
  lua_setfield(L, LUA_REGISTRYINDEX, "my_worldly_references");
  return 0;
}

int luaopen_test(lua_State *L)
{
  int i;
  int status;
  for (i = 0; i < 20000; i++)
  {
    int j;
    lua_State *LL;
    limit += 1;
    mem_used = 0;
    LL = lua_newstate(my_Alloc, NULL);
    if (LL == NULL)
      continue;
    for (j = 1; j<1000;j++)
     status = lua_cpcall(LL, do_stuff, NULL); /* does not panic */
    switch(status)
    {
      case 0:
        printf("success!\n");
        break;
      case LUA_ERRRUN:
      case LUA_ERRERR:
      case LUA_ERRMEM:
        printf("error = %d\n", status);
    }
    lua_close(LL);
  }
  return 0;
}
Reply | Threaded
Open this post in threaded view
|

Re: Bug in cpcall

Alexander Gladysh
Hi!

    for (j = 1; j<1000;j++)
     status = lua_cpcall(LL, do_stuff, NULL); /* does not panic */
    switch(status)
    {
 /* ...*/
    }

I believe you meant either

    for (j = 1; j<1000;j++)
    {
      status = lua_cpcall(LL, do_stuff, NULL); /* does not panic */
      switch(status)
      {
      /* ...*/
      }
    }

Or

    for (j = 1; j<1000;j++)
    {
     status = lua_cpcall(LL, do_stuff, NULL); /* does not panic */
     if (status != 0)
       break;
    }

    switch(status)
    {
 /* ...*/
    }

Before segfault your code fails a number of times with LUA_ERRMEM.

HTH,
Alexander.

Reply | Threaded
Open this post in threaded view
|

Re: Bug in cpcall

Patrick Donnelly-3
While keeping the switch in the for loop is probably more proper. It's
not really what I meant. cpcall should continue returning LUA_ERRMEM
every time I call it. Eventually it will segfault. Just to be clear,
in my application (this is just a test program) I call cpcall once and
I get a segfault. For some reason it became necessary for me to call
cpcall repeatedly in this test program to reproduce this (I think it
has to do with building error messages on the stack).

-- 
-Patrick Donnelly

"One of the lessons of history is that nothing is often a good thing
to do and always a clever thing to say."

-Will Durant

Reply | Threaded
Open this post in threaded view
|

Re: Bug in cpcall

Roberto Ierusalimschy
In reply to this post by Patrick Donnelly-3
> I believe cpcall does some improper memory handling somewhere because
> it attempts to free an invalid pointer. The attached c code provides a
> reproducible case. The basic idea is to limit the memory usage of a
> lua_State. (Looking at the code) In my application, I don't actually
> have to call cpcall multiple times (it actually happens the first
> time).

I did not test your program yet, but I think these is a small problem
with 'my_Alloc': it may fail even when reducing the size of a block.
("Lua assumes that the allocator never fails when osize >= nsize.")
This behavior may crash the GC.


-- Roberto

Reply | Threaded
Open this post in threaded view
|

Re: Bug in cpcall

Doug Rogers-4
In reply to this post by Patrick Donnelly-3
Patrick Donnelly wrote:
> static void *my_Alloc(void *ud, void *ptr, size_t osize, size_t nsize)
> {
>   (void) ud;
>     if (nsize == 0)
>     {
>       mem_used -= osize;
>       free(ptr);
>       return NULL;
>     }
>     else if (nsize + mem_used > limit)
>       return NULL;
>     else
>     {
>       mem_used += nsize;
>       return realloc(ptr, nsize);
>     }
> }

Don't you want

  mem_used += (nsize - osize);

in the last else block?

Doug

-- 
Innovative Concepts, Inc. www.innocon.com 703-893-2007 x220


Reply | Threaded
Open this post in threaded view
|

Re: Bug in cpcall

Roberto Ierusalimschy
In reply to this post by Patrick Donnelly-3
> I believe cpcall does some improper memory handling somewhere because
> it attempts to free an invalid pointer. The attached c code provides a
> reproducible case. The basic idea is to limit the memory usage of a
> lua_State. (Looking at the code) In my application, I don't actually
> have to call cpcall multiple times (it actually happens the first
> time).

In the code that you sent, it seems that the segfault is caused by a
stack overflow. Each cpcall leaves at the stack the error message; if
you do not pop it, they accumulate and overflow the stack. Once you
add a lua_pop inside the loop, the code seems to run without problems.

-- Roberto

Reply | Threaded
Open this post in threaded view
|

Re: Bug in cpcall

Patrick Donnelly-3
>Don't you want
>
> mem_used += (nsize - osize);
>
>in the last else block?
>
>Doug

Yes, thanks for pointing that out!

Changing my_Alloc to:
static void *my_Alloc(void *ud, void *ptr, size_t osize, size_t nsize)
{
  (void) ud;
    if (nsize == 0)
    {
      mem_used -= osize;
      free(ptr);
      return NULL;
    }
    else if ((nsize - osize) + mem_used > limit)
      return NULL;
    else
    {
      mem_used += (nsize - osize);
      return realloc(ptr, nsize);
    }
}

Does not seem to fix the problem however.

>In the code that you sent, it seems that the segfault is caused by a
>stack overflow. Each cpcall leaves at the stack the error message; if
>you do not pop it, they accumulate and overflow the stack. Once you
>add a lua_pop inside the loop, the code seems to run without problems.

That's what I suspected, but I believe there is a different problem
because my application gets this segfault in _one_ call to cpcall and
right after the Lua state is made. The segfault always occurs in the
call to free().

-- 
-Patrick Donnelly

"One of the lessons of history is that nothing is often a good thing
to do and always a clever thing to say."

-Will Durant

Reply | Threaded
Open this post in threaded view
|

Table length operator

Eva Schmidt
In reply to this post by Patrick Donnelly-3
Hello guys,

I've got a question on the Lua table structure:

In case of numerical table indices I can use the length operator # to get the number of fields. With one restriction: indices must be incremental without any gaps because the definition is:
"... any integer index n such that t[n] is not nil and t[n+1] is nil ..."

For tables with string indices there is simply no length operator :-(

So far, the only way to get a reliable information about the length of a table is to iterate it by using for i,v in pairs (tbl) and count the field by myself.

For me this is very dissatisfying. :-(

Is there any way to find a length operator for tables independent of the type of its keys?

Cheers,
Eva


Reply | Threaded
Open this post in threaded view
|

RE: Table length operator

Jerome Vuarand-2
Eva Schmidt wrote:
> For tables with string indices there is simply no length operator :-(
> 
> So far, the only way to get a reliable information about the length
> of a table is to iterate it by using for i,v in pairs (tbl) and count
> the field by myself.  
> 
> For me this is very dissatisfying. :-(
> 
> Is there any way to find a length operator for tables independent of
> the type of its keys? 

You're right, there is no way to get the total number of key/value pairs
in a table.

However I can't see any use for that information which doesn't involve
building an array anyway (the size of which can be queried afterward).
Can you tell us in what situation you would need to have that
information ?


Reply | Threaded
Open this post in threaded view
|

Re: Table length operator

Jim Whitehead II
In reply to this post by Eva Schmidt
On Wed, Mar 12, 2008 at 3:23 PM, Eva Schmidt <[hidden email]> wrote:
> Hello guys,
>
>  I've got a question on the Lua table structure:
>
>  In case of numerical table indices I can use the length operator # to get the
>  number of fields. With one restriction: indices must be incremental without any
>  gaps because the definition is:
>  "... any integer index n such that t[n] is not nil and t[n+1] is nil ..."
>
>  For tables with string indices there is simply no length operator :-(
>
>  So far, the only way to get a reliable information about the length of a table
>  is to iterate it by using for i,v in pairs (tbl) and count the field by myself.
>
>  For me this is very dissatisfying. :-(
>
>  Is there any way to find a length operator for tables independent of the type of
>  its keys?

If you find yourself needing this, you have two options:

1.) Wrap all assignment to the table (including setting to nil) in a
function call that updates an internal counter.  Then your inserts and
removes have extra overhead but you can query the length of the table
with virtually no cost.

2.) Iterate the table using pairs() and update the count when you need
it.  This is expensive when you have a large table, but saves you from
the insert/remove overhead.

There is no native way to cheaply determine the number of key/value
pairs in a Lua table.

- Jim

Reply | Threaded
Open this post in threaded view
|

Re: Table length operator

David Jones-2
In reply to this post by Eva Schmidt

On 12 Mar 2008, at 15:23, Eva Schmidt wrote:

So far, the only way to get a reliable information about the length of a table is to iterate it by using for i,v in pairs (tbl) and count the field by myself.

Correct.  But often it is sufficient to know if a table is empty or not:

next(tbl) == nil

will tell you that.

drj


Reply | Threaded
Open this post in threaded view
|

Re: Table length operator

Eva Schmidt
Hi,


Correct.  But often it is sufficient to know if a table is empty or not:

next(tbl) == nil

Yes, in my case this will work. I didn't thought of that!

Cheery,
Eva