Page MenuHomePhabricator

mediawiki-php-luasandbox produces broken build artifact for HHVM on CentOS
Closed, InvalidPublic

Description

mediawiki-php-luasandbox compiles improperly on CentOS 7 64-bit. While it does produce a build, running hhvm-test.sh fails to load the extension.

The first thing wrong appears to be a lack of an extern "C" { ... } guard around the Lua headers. This would be compatible with C if done with the standard practice of enclosing the opening and closing each like so

#ifdef __cplusplus
extern "C" {
#endif
...
#ifdef __cplusplus
}
#endif

...but not so fast:
The secondary (and related) issue is that the FindLua51cpp.cmake file expects to find a Lua library with a C++ linkage. Which isn't necessarily unreasonable on its face, because HHVM is C++ as are its extensions. It appears to search lib, but not lib64. When I added lib64, 'liblua' and 'lua' to the names, and /usr to the paths, it did find the library†.

† However, even with these hacks, the test failed to run, though the link error did go away.

The wrinkle here is that so far as I can tell, there are no readily available, packaged distributions of Lua with C++ linkage, which led to the initial diagnosis of the problem as a simple linkage mismatch.

However, the rabbit hole gets a little deeper:
Some searching around surfaced http://stackoverflow.com/a/9803812/321041, from which I followed the link to http://lua-users.org/wiki/BuildingLua.

The relevant quote:

It could be argued that if you are distributing a pre-packaged binaries of the libraries, then you have compiled the lua core as either C (most likely) or as C++, and if you compiled lua as C, you should modify the lua headers to indicate this. However, using prebuilt libraries for lua isn't recommended by the authors, they recommend directly incorporating the lua source into your application. See BuildingModules for a discussion (the end of the page).

By default if lua 5.1 or later is compiled as C++, it will use C++ exceptions to unwind the stack rather than longjmp/setjmp, though this is configurable (at compile time). See luaconf.h near LUAI_THROW/LUAI_TRY for a discussion of this.

So, long-story short, the convention is to embed Lua sources so they share linkage with the C/C++ environment that compiled them, the linkage takes advantage of C++ exceptions, a meaningful difference which causes behavioral differences in error handling , and CentOS or otherwise Red Hat platforms do not appear to offer C++ binary packages (perhaps due to different viable compiler toolchains).


Some additional context: I was aiming to run Scribunto on my MediaWiki installation to take advantage of some Wikipedia templates. However, the only other Lua engine provided is LuaStandalone, which embeds executables of perhaps known provenance, but limited verifiability.

Event Timeline

When I build for HHVM under Debian (sid) using the following commands

export CXX=g++-5   # HHVM doesn't compile right with g++-6, which is the default on my system
hphpize &&
	cmake . \
		-DCMAKE_INSTALL_PREFIX=/usr \
		-DCMAKE_VERBOSE_MAKEFILE=ON \
		-DCMAKE_BUILD_TYPE=None \
		-DLUA_USE_CPP=1 &&
	make && (
		phpize # for run-tests.php
		NO_INTERACTION=1 ./hhvm-test.sh run-tests.php
	)

it compiles and runs fine.

Debian's liblua5.1-0 package (for amd64) installs both C (liblua5.1.so) and C++ (liblua5.1-c++.so) versions of the library and installs them in appropriate locations so that they can be found without issue. It sounds like whatever package you're using for CentOS 7 does not set things up as well, and whatever hacks you're making to FindLua51cpp.cmake are causing it to find a C-linked version of the library. Last I heard HHVM requires the C++ linkage to avoid issues with longjmp/setjmp (a quick search turns up this blog post that mentions such an issue), so hearing that it doesn't work with a C-linked version isn't surprising.

Right, I noted that I didn't see a package with C++ linkage on CentOS or Red Hat platforms.. I think it's possible that this was deliberate as different compilers may do name mangling in incompatible ways, and the officially endorsed (as far as I could tell) Lua practice is to embed the language's source code itself alongside whatever embeds it.

Embedding all of Lua in the PHP extension would complicate updates if a vulnerability is found in Lua 5.1, such as CVE-2014-5461, but I'll leave the final decision on that to @tstarling.

Debian's package sidesteps the potential name mangling incompatibility by patching luaconf.h to define LUA_API as extern "C" instead of just extern when compiling for C++. But I doubt name mangling incompatibility is actually very much of an issue in practice since every other C++ shared library in the distro would have the same problem.

Yes, you need to configure Lua to use throw/catch instead of setjmp/longjmp.

I think bundling Lua is a reasonable solution. Note that Lua 5.1 does have a known DoS vulnerability, which they couldn't be bothered patching, that's why we distribute the patch ourselves in luasandbox_lstrlib.patch .

Lua's upstream repository doesn't include CMake scripts, and https://github.com/LuaDist/luajit is hopelessly out of date.

https://github.com/torch/luajit-rocks looks to be a plausible candidate, as it bundles lua, luajit, and CMake scripts for both.

MaxSem subscribed.

Moot now that we've dropped HHVM support.