From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH 2/2] Followup fix for embedded bytecode loader. Date: Mon, 14 Aug 2023 11:14:44 +0300 [thread overview] Message-ID: <fb6b062d-b2d0-6c1d-d581-1ca7c419e74c@tarantool.org> (raw) In-Reply-To: <d3389ce1-dbd1-9a4d-0b80-fed78c35d5f9@tarantool.org> Hi, Max On 8/1/23 12:53, Sergey Bronnikov via Tarantool-patches wrote: <snipped> >>> + * platforms. >>> + */ >>> + return skip_all("Enabled on Linux/x86_64 with disabled GC64"); >> We prefer to run tests like that on all paltforms, just to be sure. >> We usually >> exclude tests from platforms only if those are completely irrelevant >> to the test, >> or there is some kind of major issue. > > Yes, for this test the rule was ignored because bytecode is not portable > > and following the rule will make maintenance difficult. Therefore > supported platforms limited by x86_64 only. > Test has been updated: bytecode is not needed and now test runs on all platforms. See patch below: diff --git a/test/tarantool-c-tests/lj-549-lua_load.test.c b/test/tarantool-c-tests/lj-549-lua_load.test.c index fd1e20ed..669b6b27 100644 --- a/test/tarantool-c-tests/lj-549-lua_load.test.c +++ b/test/tarantool-c-tests/lj-549-lua_load.test.c @@ -19,20 +19,11 @@ #define LJ_MAX_MEM32 0x7fffff00 /* Max. 32 bit memory allocation. */ #define LJ_MAX_BUF LJ_MAX_MEM32 /* Max. buffer length. */ -#define UNUSED(x) ((void)(x)) +/* Defined in lua.h. */ +/* mark for precompiled code (`<esc>Lua') */ +#define LUA_SIGNATURE "\033Lua" -/** - * Array with bytecode was generated using commands below: - * - * cat << EOF > a.lua - * local a = 1 - * EOF - * luajit -b a.lua a.h - */ -#define luaJIT_BC_x86_64_sample_SIZE 22 -static const unsigned char luaJIT_BC_x86_64_sample[] = { - 27, 76, 74, 2, 2, 15, 2, 0, 1, 0, 0, 0, 2, 41, 0, 1, 0, 75, 0, 1, 0, 0 -}; +#define UNUSED(x) ((void)(x)) /** * Function generates a huge chunk of "bytecode" with a size bigger than @@ -50,15 +41,14 @@ bc_reader_with_endmark(lua_State *L, void *data, size_t *size) assert(bc_chunk != NULL); /** - * Put a chunk a valid bytecode at the beginning of the allocated - * region. `lua_load` automatically detects whether the chunk is text - * or binary, and loads it accordingly. We need a trace for bytecode - * input, so it is necessary to deceive a check in lj_lex_setup, that + * `lua_load` automatically detects whether the chunk is text or binary, + * and loads it accordingly. We need a trace for bytecode input, + * so it is necessary to deceive a check in lj_lex_setup, that * makes a sanity check and detects whether input is bytecode or text - * by the first char. Strictly speaking, it is enough to put - * LUA_SIGNATURE[0] as the first symbol in the produced chunk. + * by the first char. Put LUA_SIGNATURE[0] at the beginning of the + * allocated region. */ - memcpy(bc_chunk, luaJIT_BC_x86_64_sample, luaJIT_BC_x86_64_sample_SIZE); + bc_chunk[0] = LUA_SIGNATURE[0]; *size = bc_chunk_size; @@ -105,9 +95,10 @@ bc_reader_with_eof(lua_State *L, void *data, size_t *size) return NULL; } - char *bc_chunk = malloc(luaJIT_BC_x86_64_sample_SIZE); - memcpy(bc_chunk, luaJIT_BC_x86_64_sample, luaJIT_BC_x86_64_sample_SIZE); - *size = luaJIT_BC_x86_64_sample_SIZE; + size_t sz = 10; + char *bc_chunk = malloc(sz); + bc_chunk[0] = LUA_SIGNATURE[0]; + *size = sz; return bc_chunk; } @@ -128,15 +119,6 @@ static int bc_loader_with_eof(void *test_state) int main(void) { - if (LJ_GC64 || !LUAJIT_ARCH_X64 || !LJ_TARGET_LINUX) - /** - * `lua_load` source code is common on all platforms, - * when bytecode is not portable. - * So the test runs on Linux/x86_64 only and is skipped on other - * platforms. - */ - return skip_all("Enabled on Linux/x86_64 with disabled GC64"); - lua_State *L = utils_lua_init(); const struct test_unit tgroup[] = { test_unit_def(bc_loader_with_endmark), >>> + >>> + lua_State *L = utils_lua_init(); >>> + const struct test_unit tgroup[] = { >>> + test_unit_def(bc_loader_with_endmark), >>> + test_unit_def(bc_loader_with_eof) >>> + }; >>> + >>> + const int test_result = test_run_group(tgroup, L); >>> + utils_lua_close(L); >>> + return test_result; >>> +} >>> -- >>> 2.34.1 >>>
prev parent reply other threads:[~2023-08-14 8:14 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-07-25 16:34 [Tarantool-patches] [PATCH 0/2] Fix " Sergey Bronnikov via Tarantool-patches 2023-07-25 16:36 ` [Tarantool-patches] [PATCH 1/2] " Sergey Bronnikov via Tarantool-patches 2023-07-31 12:01 ` Maxim Kokryashkin via Tarantool-patches 2023-08-01 9:56 ` Sergey Bronnikov via Tarantool-patches 2023-08-06 11:09 ` Sergey Kaplun via Tarantool-patches 2023-08-15 8:51 ` Maxim Kokryashkin via Tarantool-patches 2023-07-25 16:37 ` [Tarantool-patches] [PATCH 2/2] Followup fix for " Sergey Bronnikov via Tarantool-patches 2023-07-31 12:20 ` Maxim Kokryashkin via Tarantool-patches 2023-08-01 9:53 ` Sergey Bronnikov via Tarantool-patches 2023-08-14 8:14 ` Sergey Bronnikov via Tarantool-patches [this message]
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=fb6b062d-b2d0-6c1d-d581-1ca7c419e74c@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=sergeyb@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 2/2] Followup fix for 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