Closed hishamhm closed 3 years ago
I pushed a candidate fix to the dont-redefine
branch, but I'm not sure if it working. Could you please test it on your computer, to see if you get the same result I'm getting?
I tested on a couple of benchmarks and they seemed to work. The tl
compiler also started to build successfully, without the "multiple definition" error. However, when I try to run the standalone executable I get an error.
$ binary/build/tl -h
tl: [string "/home/hugo/test/tl/_binary/src/argparse.lua"]:119: bad property 'name' (string expected, got boolean)
stack traceback:
[C]: in function 'error'
[string "/home/hugo/test/tl/_binary/src/argparse.lua"]:119: in upvalue 'typecheck'
[string "/home/hugo/test/tl/_binary/src/argparse.lua"]:128: in upvalue 'callback'
[string "/home/hugo/test/tl/_binary/src/argparse.lua"]:57: in field '?'
[string "/home/hugo/test/tl/_binary/src/argparse.lua"]:88: in function <[string "/home/hugo/test/tl/_binary/src/argparse.lua"]:67>
(...tail calls...)
[string "/home/hugo/test/tl/_binary/src/argparse.lua"]:649: in method 'option'
[string "/home/hugo/test/tl/_binary/src/argparse.lua"]:655: in method 'flag'
[string "/home/hugo/test/tl/_binary/src/argparse.lua"]:230: in upvalue 'callback'
[string "/home/hugo/test/tl/_binary/src/argparse.lua"]:57: in method 'add_help'
[string "/home/hugo/test/tl/_binary/src/argparse.lua"]:2097: in upvalue 'argparse'
[string "tl"]:815: in local 'get_args_parser'
[string "tl"]:1304: in main chunk
[C]: in ?
I can reproduce the same successful build and crash at runtime using tl -h
you described above.
If I build it using stock Lua, that works. I pushed an alternative gist containing extras/binary.sh
, which uses stock Lua, so you can have a baseline reference:
git clone https://github.com/teal-language/tl
cd tl
mkdir extras
cd extras
wget https://gist.githubusercontent.com/hishamhm/1ab3c1e214c012b0aca4526d79942650/raw/d85b98480136142251294952dca98a21e678d697/binary.sh
chmod +x binary.sh
cd ..
./extras/binary.sh
./_binary/build/tl -h
I noticed that the ${CC}
line in extras/binary-aot.sh
didn't have -O2
, so I added it, but it didn't change the behavior, tl -h
still crashes.
Another data point, here's a "binary-noaot.sh" script, which builds liblua.a from this repository (using the dont-redefine
branch), but does not aot-compile tl.lua, and instead bundles the plain .lua file like the original binary.sh does:
It still produces the same crash with tl -h
.
I don't think I'm doing anything strange in binary-aot.sh or binary-noaot.sh, here's the diff between plain binary.sh and binary-noaot.sh; its seems to just pick Lua from the different repo:
--- extras/binary.sh 2021-09-04 11:39:31.245485932 -0300
+++ extras/binary-noaot.sh 2021-09-04 11:50:48.958458140 -0300
@@ -125,7 +125,6 @@
(
cd "${root}/downloads"
- download "https://www.lua.org/ftp/lua-${lua_version}.tar.gz"
download "https://github.com/luarocks/argparse/archive/${argparse_version}.tar.gz" "argparse-${argparse_version}.tar.gz"
download "https://github.com/kikito/inspect.lua/archive/v${inspect_version}.tar.gz" "inspect.lua-${inspect_version}.tar.gz"
download "https://github.com/keplerproject/luafilesystem/archive/v${luafilesystem_version//./_}.tar.gz" "luafilesystem-${luafilesystem_version}.tar.gz"
@@ -135,7 +134,8 @@
(
cd "${root}/deps"
- tar zxvpf "../downloads/lua-${lua_version}.tar.gz"
+ git clone "https://github.com/hugomg/lua-aot-5.4"
+ ( cd lua-aot-5.4; git checkout dont-redefine )
tar zxvpf "../downloads/argparse-${argparse_version}.tar.gz"
tar zxvpf "../downloads/inspect.lua-${inspect_version}.tar.gz"
tar zxvpf "../downloads/luafilesystem-${luafilesystem_version}.tar.gz"
@@ -154,14 +154,14 @@
}
(
- cd "${root}/deps/lua-${lua_version}"
+ cd "${root}/deps/lua-aot-5.4"
"${MAKE}" -C "src" LUA_A="liblua.a" CC="${CC}" AR="${AR} rcu" RANLIB="${RANLIB}" SYSCFLAGS= SYSLIBS= SYSLDFLAGS= "liblua.a"
check "src/liblua.a"
)
(
cd "${root}/deps/luafilesystem-${luafilesystem_version}"
- "${CC}" -c -o "lfs.o" -I "../lua-${lua_version}/src" "src/lfs.c"
+ "${CC}" -c -o "lfs.o" -I "../lua-aot-5.4/src" "src/lfs.c"
"${AR}" rcu -o "lfs.a" "lfs.o"
check "lfs.a"
)
@@ -176,7 +176,7 @@
check "inspect.lua"
)
-LIBLUA_A="${root}/deps/lua-${lua_version}/src/liblua.a"
+LIBLUA_A="${root}/deps/lua-aot-5.4/src/liblua.a"
LFS_A="${root}/deps/luafilesystem-${luafilesystem_version}/lfs.a"
ARGPARSE_LUA="${root}/deps/argparse-${argparse_version}/src/argparse.lua"
INSPECT_LUA="${root}/deps/inspect.lua-${inspect_version}/inspect.lua"
@@ -510,7 +510,7 @@
exe_pathname="${root}/build/${executable}"
-${CC} -o "$exe_pathname" -I"${root}/deps/lua-${lua_version}/src" "${root}/src/tl.c" "${LIBLUA_A}" "${LFS_A}" $MYCFLAGS
+${CC} -o "$exe_pathname" -I"${root}/deps/lua-aot-5.4/src" "${root}/src/tl.c" "${LIBLUA_A}" "${LFS_A}" $MYCFLAGS
set +x
So it seems something is going on with this repo's liblua.a
? Does it pass the Lua test suite?
One thing that adds an extra layer of weirdness is that in one of my proof-of-concept experiments (editing the C code by hand) managed to run tl without errors. But that proof of concept was also obviously wrong in some other places. I'm baffled that the obviously wrong version worked but the theoretically improved one did not...
I'm considering taking a step back to create a proper test suite, just to be more confident that the issue isn't something else.
I'm not 100% sure if the current version passes the Lua test suite. Because my tests weren't automated, I didn't always rerun all of them...
I'm not 100% sure if the current version passes the Lua test suite. Because my tests weren't automated, I didn't always rerun all of them...
Since this is meant to be a drop-in replacement for Lua, I suppose it shouldn't be too hard to enable Github Actions in this repo and trigger at least the stock Lua test suite in CI. I've never used Actions myself (been using Travis and Appveyor on my past projects), but my GSoC student @Deepak123bharat has investigated it recently and it seems pretty straightforward: https://github.com/Deepak123bharat/rocks-sysdetect/commit/42ec9b25a8060bffad115771770f9d8474ff2b2f
One thing that adds an extra layer of weirdness is that in one of my proof-of-concept experiments (editing the C code by hand) managed to run tl without errors.
One relevant point is that using binary-noaot.sh
we get the same error even without aot-compiling tl.lua to generate C code, so the misbehavior in the patched Lua VM happens when running plain Lua code. I'd try to diff the current code with your experiment's branch and see what else is different outside of the C code generator.
Pallene is using Github Actions now, so I remember the basics. (The one thing I never managed to figure out was caching dependencies to speed it up...). But before setting up the CI, I need to set up the scripts to run it locally :)
One relevant point is that using binary-noaot.sh we get the same error even without aot-compiling tl.lua to generate C code, so the misbehavior in the patched Lua VM happens when running plain Lua code
Good point! Most of the modified code should be inside #if LUAOT
blocks, although to be safe it might be better to look at the full diff.
With help from the full Lua test suite, I found two bugs in my modifications to the OP_CALL in the lvm.c:
At this point, there are two conceivable directions to take:
I'm going to make another attempt at option 2. If we can pull that off, it should avoid this whole category of bugs and be more efficient to boot. I'll post updates here on github when I make more progress.
I figured out a way to remove the OP_CALL modifications. The Lua test suite is passing now and I think that standalone executables also work.
Before we close this issue maybe it would be nice if we added some documentation telling how to build a standalone executable.
Another question I have: what is the cleanest replacement for dofile("file.lua")
for AOT compiled code.
Before we close this issue maybe it would be nice if we added some documentation telling how to build a standalone executable.
As long as liblua.a works, I think there is nothing special now regarding LuaAOT on how to create standalone executables (i.e. you can use the same methods used for regular Lua, such as luapak or my custom script).
One thing to note, though, is that I'm not AOT-compiling my main chunk (the tl
script at the top-level of the tl repo); that's still a plain Lua string. If you wanted to make it possible to AOT-compile main chunks easily, then I think it would make sense to have some explicit support in luaot
which creates the necessary scaffolding similar to the one that my script creates (which was in turn inspired by the code of luac
and other executable packers). But the route of least-effort now would be to document that for now you need a plain Lua main chunk and that all AOT-code should be compiled as modules, and then you can use luapak and friends.
Another question I have: what is the cleanest replacement for dofile("file.lua")for AOT compiled code.
I assume that if one is using dofile
they are explicitly loading plaintext Lua code dynamically, so that means it is not meant to be AOT-compiled. So I don't think nothing needs to be done about that?
I think there is nothing special now regarding LuaAOT on how to create standalone executables (i.e. you can use the same methods used for regular Lua, such as luapak or my custom script).
Indeed. However, I always forget what are the right incantations to create a standalone executable. Maybe we should talk about it in the readme. I expect this will be a common question.
I didn't know about luapak, but if it works for luaot it might be a good thing to point people to. Thanks for the tip!
I assume that if one is using dofile they are explicitly loading plaintext Lua code dynamically
The thing I'm thinking about is that main chunk thing you just mentioned. Currently the ergonomics is awkward.
Perhaps I should have said lua file.lua
instead of dofile("file.lua")
when I asked my question. Currently, to run test cases and benchmarks I'm doing lua -e 'require("file_aot")'
but that doesn't feel right... It also means that we can't name the C file with the same name as the Lua file.
Maybe we can use package.loadlib instead of require? Do you know why I have to add a a ./
at the start? Just doing package.loadlib("foo.so"
doesn't seem to work.
assert(package.loadlib("./foo.so", 'luaopen_foo'))()
But the route of least-effort now would be to document that for now you need a plain Lua main chunk and that all AOT-code should be compiled as modules
Agreed. But for the test suite it would be nice if we had AOT-compiled main chunks. Do you think using package.loadlib would do the trick?
Indeed. However, I always forget what are the right incantations to create a standalone executable.
The incantations aren't trivial: look at my script, it includes a Lua script which does bin2c of .lua files, prepares arg
(which is also available as ...
for the main chunk!), handles C libraries that are bundled together). luapak does all that; I ended up using my own standalone script because I originally made it for LuaRocks which has its own class of chicken-and-egg problems and reused it for Teal.
I didn't know about luapak, but if it works for luaot it might be a good thing to point people to. Thanks for the tip!
Yes, that's probably the way to go for luaot! (→ btw, it's a bit confusing that the repo is lua-aot, the project is LuaAOT and the binary is luaot :laughing: )
Currently, to run test cases and benchmarks I'm doing lua -e 'require("file_aot")' but that doesn't feel right...
I think that's acceptable. For me as a seasoned Lua user, it's a lot easier to understand that the Lua module is compiled into a C module and then I get all the expected behaviors and usage patterns I get from C modules, rather than having to learn any LuaAOT-specific behind-the-scenes magic (and any time we add magic to the module loading process we end up adding gotchas... I sorta regret every single one I did for LuaRocks; I should have just told users "learning how to use package.(c)path and LUA_(C)PATH(_5_x) is part of learning Lua")
In short I think the lua -e 'require("file_aot")'
is fine given it's a C module.
It also means that we can't name the C file with the same name as the Lua file.
Yes, but that's the standard Lua behavior — and there are modules that rely on it: LuaSec has ssl.lua
which is the main module and ssl.so
which is a bundle library that contain the other ssl.*
modules (the C package loader does look for ssl.foo
inside ssl.so
)
Do you know why I have to add a a ./ at the start?
The manual says package.loadlib bypasses the package searching mechanisms — maybe the tests have changed the cwd?
Do you think using package.loadlib would do the trick?
If require works, I'd just keep things simple and continue using that. requiring a module does run the module's main chunk, after all.
btw, it's a bit confusing that the repo is lua-aot, the project is LuaAOT and the binary is luaot
The binary used to be called "luaaot" but I changed it after mistyping countless times XD. I'll consider renaming the repository though.
and any time we add magic to the module loading process we end up adding gotchas... I sorta regret every single one I did for LuaRocks
Fair enough. That's a strong argument against complicating the module loading.
The manual says package.loadlib bypasses the package searching mechanisms — maybe the tests have changed the cwd?
I'm testing by hand in the Lua REPL. I'm confused because "aaa.so" gives "No such file or directory" but "./aaa.so" works fine. Shouldn't "aaa.so" and "./aaa.so" always mean the same thing on Linux? 🤷
Well, I guess that's one more reason to be distrustful of complicating the loading.
Shouldn't "aaa.so" and "./aaa.so" always mean the same thing on Linux? shrug
It should. Running it with strace might give some hints on what Lua is doing.
Hi Hugo! I tried to build a stand-alone executable using this project; I hit an issue with exported symbols:
This should allow you to reproduce it, it fetches the repo and compilation script I used: