From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Maxim Kokryashkin <m.kokryashkin@tarantool.org>, Sergey Bronnikov <estetus@gmail.com> Cc: max.kokryashkin@gmail.com, tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH luajit 1/2][v2] Fix embedded bytecode loader. Date: Mon, 4 Sep 2023 12:31:21 +0300 [thread overview] Message-ID: <1011cc5a-a628-450d-8c29-25972489bf0e@tarantool.org> (raw) In-Reply-To: <ukoja5u3qk7rg3hn2j7gckacvcv7mwrxaejzzlm7ccto4btu7t@uza5lsm3eqij> Hi, Maxim On 9/1/23 12:42, Maxim Kokryashkin via Tarantool-patches wrote: <snipped> >> diff --git a/test/tarantool-tests/lj-549-bytecode-loader.test.lua b/test/tarantool-tests/lj-549-bytecode-loader.test.lua >> new file mode 100644 >> index 00000000..889be80a >> --- /dev/null >> +++ b/test/tarantool-tests/lj-549-bytecode-loader.test.lua >> @@ -0,0 +1,96 @@ >> +local tap = require('tap') >> +local ffi = require('ffi') >> +local utils = require('utils') >> +local test = tap.test('lj-549-bytecode-loader'):skipcond({ >> + -- ['Test requires GC64 mode enabled'] = not require('ffi').abi('gc64'), > Why this skipcond is commented out? it is not needed anymore, so I removed it. Iterative patch is below: --- a/test/tarantool-tests/lj-549-bytecode-loader.test.lua +++ b/test/tarantool-tests/lj-549-bytecode-loader.test.lua @@ -1,9 +1,7 @@ local tap = require('tap') local ffi = require('ffi') local utils = require('utils') -local test = tap.test('lj-549-bytecode-loader'):skipcond({ - -- ['Test requires GC64 mode enabled'] = not require('ffi').abi('gc64'), -}) +local test = tap.test('lj-549-bytecode-loader') test:plan(1) >> +}) >> + >> +test:plan(1) >> + >> +-- Test creates a shared library with LuaJIT bytecode, >> +-- loads shared library as a Lua module and checks, >> +-- that no crashes eliminated. >> +-- >> +-- $ make HOST_CC='gcc -m32' TARGET_CFLAGS='-m32' \ >> +-- TARGET_LDFLAGS='-m32' \ >> +-- TARGET_SHLDFLAGS='-m32' \ >> +-- -f Makefile.original >> +-- $ echo 'print("test")' > a.lua >> +-- $ LUA_PATH="src/?.lua;;" luajit -b a.lua a.c >> +-- $ gcc -m32 -fPIC -shared a.c -o a.so >> +-- $ luajit -e "require('a')" >> +-- Program received signal SIGBUS, Bus error >> + >> +local function file_exists(fname) >> + return io.open(fname, 'r') or true and false >> +end >> + >> +local function get_file_name(file) >> + return file:match("[^/]*$") >> +end >> + >> +local stdout_msg = 'Lango team' >> +local lua_code = ('print(%q)'):format(stdout_msg) >> +local fpath = os.tmpname() >> +local path_lua = ('%s.lua'):format(fpath) >> +local path_c = ('%s.c'):format(fpath) >> +local path_so = ('%s.so'):format(fpath) >> + >> +-- Create a file with a minimal Lua code. >> +local fh = assert(io.open(path_lua, 'w')) >> +fh:write(lua_code) >> +fh:close() >> + >> +local module_name = assert(get_file_name(fpath)) >> + >> +local basedir = function(path) >> + local sep = '/' >> + return path:match('(.*' .. sep .. ')') or './' >> +end >> + >> +-- Create a C file with LuaJIT bytecode. >> +-- We cannot use utils.makecmd, because command-line generated >> +-- by `makecmd` contains `-e` that is incompatible with option `-b`. >> +local function create_c_file(pathlua, pathc) >> + local lua_path = os.getenv('LUA_PATH') >> + local lua_bin = require('utils').exec.luacmd(arg):match('%S+') >> + local cmd_fmt = 'LUA_PATH="%s" %s -b %s %s' >> + local cmd = (cmd_fmt):format(lua_path, lua_bin, pathlua, pathc) >> + local ret = os.execute(cmd) >> + assert(ret == 0, 'create a C file with bytecode') >> +end >> + >> +create_c_file(path_lua, path_c) >> +assert(file_exists(path_c)) >> + >> +-- Compile C source code with LuaJIT bytecode to a shared library. >> +-- `-m64` is not available on ARM64, see >> +-- "3.18.1 AArch64 Options in the manual", >> +-- https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html >> +local cflags_64 = jit.arch == 'arm64' and '-march=armv8-a' or '-m64' >> +local cflags = ffi.abi('32bit') and '-m32' or cflags_64 >> +local cc_cmd = ('cc %s -fPIC -shared %s -o %s'):format(cflags, path_c, path_so) >> +local ph = io.popen(cc_cmd) >> +ph:close() > I suggest using the os.execute and checking the exit code. > Popen is excessive here. replaced, iterative patch is below --- a/test/tarantool-tests/lj-549-bytecode-loader.test.lua +++ b/test/tarantool-tests/lj-549-bytecode-loader.test.lua @@ -68,8 +68,8 @@ assert(file_exists(path_c)) local cflags_64 = jit.arch == 'arm64' and '-march=armv8-a' or '-m64' local cflags = ffi.abi('32bit') and '-m32' or cflags_64 local cc_cmd = ('cc %s -fPIC -shared %s -o %s'):format(cflags, path_c, path_so) -local ph = io.popen(cc_cmd) -ph:close() +local rc = os.execute(cc_cmd) +assert(rc == 0) assert(file_exists(path_so)) -- Load shared library as a Lua module. > >> +assert(file_exists(path_so)) >> + >> +-- Load shared library as a Lua module. >> +local lua_cpath = ('"/tmp/?.so;"'):format(basedir(fpath)) >> +assert(file_exists(path_so)) >> +local cmd = utils.exec.makecmd(arg, { >> + script = ('-e "require([[%s]])"'):format(module_name), >> + env = { >> + LUA_CPATH = lua_cpath, >> + -- It is required to cleanup LUA_PATH, otherwise >> + -- LuaJIT loads Lua module, see tarantool-tests/utils/init.lua. >> + LUA_PATH = '', >> + }, >> +}) >> +local res = cmd() >> +test:ok(res == stdout_msg, 'bytecode loader works') >> + >> +os.remove(path_lua) >> +os.remove(path_c) >> +os.remove(path_so) >> + >> +os.exit(test:check() and 0 or 1) > `test:done` should be used instead. Fixed. @@ -91,4 +91,4 @@ os.remove(path_lua) os.remove(path_c) os.remove(path_so) -os.exit(test:check() and 0 or 1) +test:done(true) >> -- >> 2.34.1 >>
next prev parent reply other threads:[~2023-09-04 9:31 UTC|newest] Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-08-31 11:29 [Tarantool-patches] [PATCH luajit 0/2][v2] " Sergey Bronnikov via Tarantool-patches 2023-08-31 11:30 ` [Tarantool-patches] [PATCH luajit 1/2][v2] " Sergey Bronnikov via Tarantool-patches 2023-08-31 11:49 ` Sergey Bronnikov via Tarantool-patches 2023-09-01 9:42 ` Maxim Kokryashkin via Tarantool-patches 2023-09-04 9:31 ` Sergey Bronnikov via Tarantool-patches [this message] 2023-09-05 6:34 ` Maxim Kokryashkin via Tarantool-patches 2023-09-05 14:10 ` Sergey Kaplun via Tarantool-patches 2023-09-07 15:21 ` Sergey Bronnikov via Tarantool-patches 2023-09-11 8:45 ` Sergey Kaplun via Tarantool-patches 2023-09-12 10:20 ` Sergey Bronnikov via Tarantool-patches 2023-10-31 11:30 ` Sergey Kaplun via Tarantool-patches 2023-09-05 14:12 ` Sergey Kaplun via Tarantool-patches 2023-09-07 7:06 ` Sergey Bronnikov via Tarantool-patches 2023-08-31 11:32 ` [Tarantool-patches] [PATCH luajit 2/2][v2] Followup fix for " Sergey Bronnikov via Tarantool-patches 2023-09-01 10:05 ` Maxim Kokryashkin via Tarantool-patches 2023-09-04 16:34 ` Sergey Bronnikov via Tarantool-patches 2023-09-05 6:45 ` Maxim Kokryashkin via Tarantool-patches 2023-09-05 12:55 ` Sergey Kaplun via Tarantool-patches 2023-09-07 7:04 ` Sergey Bronnikov via Tarantool-patches 2023-09-11 9:26 ` Sergey Kaplun via Tarantool-patches 2023-09-12 10:30 ` Sergey Bronnikov via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1011cc5a-a628-450d-8c29-25972489bf0e@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=estetus@gmail.com \ --cc=m.kokryashkin@tarantool.org \ --cc=max.kokryashkin@gmail.com \ --cc=sergeyb@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH luajit 1/2][v2] Fix embedded bytecode loader.' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox