Re: [Tarantool-patches] [PATCH luajit 2/2] OSX/iOS/ARM64: Fix bytecode embedding in Mach-O object file.

[Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Sergey!
Thanks for the patch!
Please consider my comments below.
On Thu, Mar 14, 2024 at 02:39:51PM +0300, Sergey Bronnikov wrote:
> From: sergeyb@tarantool.org
>
> Thanks to Carlo Cabrera.
>
> (cherry picked from commit b98b37231bd2dcb79e10b0f974cefd91eb0d7b3a)
>
> Mach-O FAR object files generated by LuaJIT for arm64 had an incorrect
Typo: s/FAR/FAT/
Typo: s/arm64/ARM64/
> format because FFI structure used for generation was wrong:
Typo: s/FFI/The FFI/
> `mach_fat_obj` instead of `mach_fat_obj_64`.
>
> Sergey Bronnikov:
> * added the description and a test for the problem
Typo: s/a test/the test/
>
> Part of tarantool/tarantool#9595
> ---
>  src/jit/bcsave.lua                                 | 14 +++++++++++++-
>  ...-fix_generation_of_mach-o_object_files.test.lua |  4 +++-
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/src/jit/bcsave.lua b/src/jit/bcsave.lua
> index 7aec1555..069cf1a3 100644
> --- a/src/jit/bcsave.lua
> +++ b/src/jit/bcsave.lua
> @@ -491,6 +491,18 @@ typedef struct {
>    mach_nlist sym_entry;
>    uint8_t space[4096];
>  } mach_fat_obj;
> +typedef struct {
> +  mach_fat_header fat;
> +  mach_fat_arch fat_arch[2];
> +  struct {
> +    mach_header_64 hdr;
> +    mach_segment_command_64 seg;
> +    mach_section_64 sec;
> +    mach_symtab_command sym;
> +  } arch[2];
> +  mach_nlist_64 sym_entry;
> +  uint8_t space[4096];
> +} mach_fat_obj_64;
>  ]]
>    local symname = '_'..LJBC_PREFIX..ctx.modname
>    local isfat, is64, align, mobj = false, false, 4, "mach_obj"
> @@ -499,7 +511,7 @@ typedef struct {
>    elseif ctx.arch == "arm" then
>      isfat, mobj = true, "mach_fat_obj"
>    elseif ctx.arch == "arm64" then
> -    is64, align, isfat, mobj = true, 8, true, "mach_fat_obj"
> +    is64, align, isfat, mobj = true, 8, true, "mach_fat_obj_64"
>    else
>      check(ctx.arch == "x86", "unsupported architecture for OSX")
>    end
> diff --git a/test/tarantool-tests/lj-865-fix_generation_of_mach-o_object_files.test.lua b/test/tarantool-tests/lj-865-fix_generation_of_mach-o_object_files.test.lua
> index 0519e134..0a11f163 100644
> --- a/test/tarantool-tests/lj-865-fix_generation_of_mach-o_object_files.test.lua
> +++ b/test/tarantool-tests/lj-865-fix_generation_of_mach-o_object_files.test.lua
> @@ -7,7 +7,7 @@ local test = tap.test('lj-865-fix_generation_of_mach-o_object_files'):skipcond({
>    ['Test uses exotic type of loaders (see #9671)'] = _TARANTOOL,
>  })
>
> -test:plan(4)
> +test:plan(8)
>
>  -- Test creates an object file in Mach-O format with LuaJIT bytecode
>  -- and checks validness of the object file fields.
> @@ -267,5 +267,7 @@ end
>
>  -- ARM
>  build_and_check_mach_o(false)
> +-- ARM64
> +build_and_check_mach_o(true)
These `true/false` should be explained as platform toggle.
An even better solution would be to pass the platform name
explicitly.
>
>  test:done(true)
> --
> 2.34.1
>