rrthomas / lrexlib

A Lua (5.1 and later) binding of various regex library APIs (POSIX, PCRE, PCRE2, GNU, Oniguruma and TRE)
Other
161 stars 29 forks source link

Crash on windows build with pcre Version 8.32! #3

Closed zhaozg closed 11 years ago

zhaozg commented 11 years ago

Can't pass runtest.lua,

May be in pcre_study error.

zhaozg commented 11 years ago

More Info.

I use luajit-2.0, if I replace (single file) common.c with with version 2.6.0, It will work fine.

It just diff std alloc or lua alloc.

rrthomas commented 11 years ago

Thanks very much for this bug report: I am calling the allocation function incorrectly, with the Lua state pointer as the first argument instead of the allocator's userdata. I will issue a fix ASAP. If you want to fix it yourself quickly, make sure the first argument passed to "lalloc" is always "ud".

rrthomas commented 11 years ago

Memo to self: Lua 5.2 has changed the interface too, and requires that I pass LUA_TUSERDATA instead of 0 as the third argument when allocating.

rrthomas commented 11 years ago

I have pushed a fix to git that should fix things for you; please let me know if it works (it's just a new version of common.c).

The change for Lua 5.2 doesn't actually matter with the default allocator, but I will fix it anyway and make a new release. (Also, this issue should not affect LuaJIT, which uses the Lua 5.1 API.)

zhaozg commented 11 years ago

clear, quickly, thanks very much.,

I think below need fix too.

lpcre.c(223): ud->match = (int ) Lmalloc (L, (ALG_NSUB(ud) + 1) * 3 \ sizeof (int)); Not match. lpcre.c(340): free (ud->match);

lpcre.c(280): free (buf); lpcre.c(284): free (buf); Not match with buf = (int) Lmalloc (L, (argE.ovecsize + argE.wscount) \ sizeof(int)); at line 263.


void *Lfree(lua_State *L, void *p) {
  void *ud;
  lua_Alloc lalloc = lua_getallocf(L, &ud);
  p = lalloc(ud, p, LUA_TNIL, LUA_TNIL);
  if(p != NULL)
    luaL_error(L, "malloc failed");
  return p;
}
rrthomas commented 11 years ago

Thanks for that, that was another brown-paper-bag bug where I'd failed to use the lua_Alloc function to free memory everywhere (there were problems in other modules too).

However, in the process of fixing it I found and fixed two other bugs, so it's not all bad!

Thanks very much again for the bug report; I'll make a new release as soon as I can.

zhaozg commented 11 years ago

https://github.com/rrthomas/lrexlib/commit/cdd65f33ec817c31820e2a7673c33a63df72c12c Sorry, Can't pass, Fail.

rrthomas commented 11 years ago

I'll need more information than that, all the tests pass for me now with LuaJIT or Lua.

zhaozg commented 11 years ago
    rex_pcre.dll!Lpcre_gc(lua_State * L)  行 340 + 0xa 字节  C
luajit.dll!59b8f6d2()   
luajit.dll!59b537d7()   
rex_pcre.dll!buffer_pushresult(tagBuffer * buf)  行 145 + 0x14 字节  C
rex_pcre.dll!algf_gsub(lua_State * L)  行 420  C
luajit.dll!59b8f6d2()   

Lmalloc will call lalloc which get by lua_getallocf, and implement in shared library.(On windows It's as a dll and with NDEBUG), but rex_pcre, complied with DEBUG. That means, a same mem pointer not same management. So it creash. I think must keep malloc and free as a pair, can't assume all thing pass. Sorry for my Pool Language

rrthomas commented 11 years ago

[Don't worry about your English, I'm very grateful you are working in a language I speak!]

I need to understand better. Do you mean that if I allocate a pointer with lua_Alloc function (in Lua or LuaJIT DLL), the pointer is not valid in PCRE DLL? But this already happens, because strings, allocated with LuaJIT's allocator (which is not the system one, unless you compile LuaJIT with LUAJIT_USE_SYSMALLOC) are also passed to PCRE, but apparently they do not cause this problem.

zhaozg commented 11 years ago

Do you mean that if I allocate a pointer with lua_Alloc function (in Lua or LuaJIT DLL), the pointer is not valid in PCRE DLL?

Not this, Please Think this, I compile luajit with -DNDEBUG, but compile rex_pcre with -D_DEBUG, When alloc, It will use a malloc function, but when free, because _DEBUG, free as a macro for debug API, So alloc with a way, free with another way, different memory management mechanism. Cause crash.

persist to keep same memory management mechanism, can you understand my means. Please

Build it on windows with msvc2010.

rrthomas commented 11 years ago

Thank you for explaining, I understand better now.

But in lrexlib, every allocation is free'd with the allocator that malloc'd it.

So for example, if allocated with "malloc", then freed with "free". If allocated with lua_Alloc, then freed with lua_Alloc.

zhaozg commented 11 years ago

Yes, because can be changed by user, It will match with free on some conditions. I often use rex_pcre, It's great, Thanks.

rrthomas commented 11 years ago

Sorry, what can be changed by user?

zhaozg commented 11 years ago

Sorry, what can be changed by user?

lua_State lua_newstate (lua_Alloc f, void ud);

Creates a new, independent state. Returns NULL if cannot create the state (due to lack of memory). The argument f is the allocator function; Lua does all memory allocation for this state through this function. The second argument, ud, is an opaque pointer that Lua simply passes to the allocator in every call.

If I write a lua_Alloc, to cache all memory, free will cause my app fail!

rrthomas commented 11 years ago

OK, I understand that the user can choose an allocator.

But, in lrexlib, always the same memory management mechanism is used for each block of memory, just as you ask.

rrthomas commented 11 years ago

So, for example, if you write lua_Alloc to cache all memory, that is OK: free is never called on a block created with lua_Alloc.

rrthomas commented 11 years ago

Let me be precise: in lpcre.c, lua_Alloc is used to allocate memory in two places:

    Line 223:   ud->match = (int *) Lmalloc (L, (ALG_NSUB(ud) + 1) * 3 * sizeof (int));
    Line 267:  buf = (int*) Lmalloc (L, bufsize);

These blocks are also freed with Lfree, at lines 286, 290 and 346. They are never freed in PCRE.

rrthomas commented 11 years ago

Similarly, all memory allocated by PCRE is freed with pcre_free.

rrthomas commented 11 years ago

I am sorry, I do not use Windows, and I cannot find the mistake. If you can find the mistake, I am happy to fix it.

Otherwise, you can choose the allocator to be the same always.

zhaozg commented 11 years ago

Thanks very much.

All test pass.

rrthomas commented 11 years ago

I don't understand, I didn't change anything!

Do you mean that you find the current code does work correctly for you?

zhaozg commented 11 years ago

Sorry, I fetch last master version from git, So all test pass.

rrthomas commented 11 years ago

That's very good news, thanks for your help!

zhaozg commented 11 years ago

https://github.com/rrthomas/lrexlib/commit/fadff3495c6973eac77bffd115cde3007668a206 fix all my problems

Faint, If I check last code, will save your two hours time, So sorry.