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

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Max,

thanks for review. See my comments below.

Changes force-pushed.

On 3/18/24 16:44, Maxim Kokryashkin wrote:
> 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/
Fixed.
>> format because FFI structure used for generation was wrong:
> Typo: s/FFI/The FFI/
Fixed.
>> `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/
Fixed.
>> 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.
Why comment above is not sufficient?
>>   test:done(true)
>> --
>> 2.34.1
>>