Re: [Tarantool-patches] [PATCH 2/2] Followup fix for embedded bytecode loader.

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.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
>>>