bit implementation behaves differently from LuaJIT's on Mac

[jpd236]

(As a disclaimer, I'm a bit inexperienced with wxLua and Lua in general; I've been working on fixing bugs in a project which is using wxLua. I was unable to build wxLua in isolation locally to try to reproduce outside this project. Please let me know if this seems likely to be a project-specific issue rather than one in the library itself, or if anything here is off base).

I have found that the md5 library available at https://github.com/kikito/md5.lua is returning incorrect values in my application which uses wxLua on 64-bit Mac, but is working as expected on 32-bit Windows. What I've found is that the "bit" operations that this script depends on behave differently than the script expects.

If I just run the script using LuaJIT (2.0.4), it uses LuaJIT's built-in "bit" module. This module always returns 32-bit signed integers (http://bitop.luajit.org/semantics.html).

wxLua appears to replace this module with its own implementation, defined here:

https://github.com/pkulchenko/wxlua/blob/3a23bcbf3ecba91874fc5d77c1e0504d247b458d/wxLua/modules/wxlua/wxlstate.cpp#L2522

In my 64-bit Mac application, this replacement returns unsigned integers. This appears to be unexpected by the library, which thus returns invalid md5 sums.

If I either:

• Update the md5 script to ignore 'bit' and use 'bit32' instead, or
• Replace the implementation of luaopen_bit linked above with just "return 0"

Then the script works as expected.

Is the script making an assumption about how 'bit' works that is not generally safe to make, and thus this is a bug there? Or is this assumption fair and wxLua is unexpectedly breaking it?

[pkulchenko]

@jpd236, thank you for the report. Do you have a test case I can use to see the problem?

[jpd236]

I was able to build and run the wxLua app so I can confirm this isn't related to our app's integration.

In the wxLua sample app, I ran the following program:

local bit = require 'bit'
print(bit.band(4023233417, 2562383102))

which outputs 2290649224. In contrast, when I run the same program in the LuaJIT interpreter, I get -2004318072. These are both 0x88888888 - just unsigned vs. signed interpretations.

As a broader example of the issue this causes, download the md5 module at https://raw.githubusercontent.com/kikito/md5.lua/master/md5.lua and run:

package.path='/path/to/md5.lua'
local md5 = require 'md5'
print(md5.sumhexa('test'))

This outputs "0c07ed8baeb13b874903ab859a6d640f". But the md5 hash of "test" is "098f6bcd4621d373cade4e832627b4f6" (as can be seen at http://www.miraclesalad.com/webtools/md5.php or any other md5 tool).

Does that help convey the issue more clearly?

[pkulchenko]

print(bit.band(4023233417, 2562383102)) Does that help convey the issue more clearly?

Yes, it does. wxlua implementation uses bitlib release 25 and I haven't had a chance to figure out why it happens, but I suspect it may have something to do with UMAX being used in truncation:

#define BIT_TRUNCATE(i)                         \
  ((i) & BIT_UMAX)

[jpd236]

I definitely can't say for sure that this is the right way to handle this, but in case this makes sense to you, I sent a pull request (https://github.com/pkulchenko/wxlua/pull/44) which moves from bitlib to Lua BitOp. This makes some sense to me given that bitlib is deprecated in favor of Lua BitOp, and since Lua BitOp appears to fix this problem and provides consistency with the same module on LuaJIT.

I tested on my Mac with an external LuaJIT and the built-in Lua 5.1 and 5.2 distributions. For 5.2, I had to make a small tweak to the library to always use luaL_register instead of luaL_newlib; the latter wasn't working without modifications to the Lua code itself, and I assumed we'd want this to work even if an external Lua distribution were being used.

Note that there's still a second copy of bitlib in lbitlib.c... this one is used for "bit32" which I suppose provides backwards compatibility since it was backported from Lua 5.2. I've left that one untouched.

Let me know what you think! This meets my needs but I wouldn't be surprised if it had subtle issues somewhere I wasn't looking.

[pkulchenko]

@jpd236, thank you for the patch! This indeed looks like a better option. I should be able to review and merge in a few days.

[jpd236]

Thanks for merging the patch!

[pkulchenko]

@jpd236, I updated bit.c to work with Lua 5.3+ (commit 463112e5), which could have affected your patch. I expect it will continue to work (as it only worked previously when LUA_COMPAT_MODULE was enabled), but if you run into any issues with the latest code, let me know.