From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id CC5FDB85E13; Tue, 26 Mar 2024 16:53:41 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org CC5FDB85E13 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1711461221; bh=LSeVlUV0caq2a/3cZRrYdBPUVE9VEBxDnm2gBN6R3n4=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=q2jP6wt1SI7HfYn7dJGxqjWA0KP7L0Ks6wMFquKrHZxMG5VbbDayFvZtVMhQnbJgj Gbru6fk8Zly3odWz4ht/nEzPyLRva7whpSJZUDMNGvcm5u5Na+WksdXc+pAQWTHMdO IHp1CPga6Qnss0QVjMOITmoMaDlwNptyIYrtjckE= Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [95.163.41.81]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 3F0E9B85E00 for ; Tue, 26 Mar 2024 16:53:40 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 3F0E9B85E00 Received: by smtp40.i.mail.ru with esmtpa (envelope-from ) id 1rp7FD-00000000PyQ-1Qls; Tue, 26 Mar 2024 16:53:39 +0300 Message-ID: Date: Tue, 26 Mar 2024 16:53:38 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Maxim Kokryashkin References: <9a71bf0765acd6ab019de4ae9f491a6c7bcb463d.1710416150.git.sergeyb@tarantool.org> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD905284D235AF2CB53F8B0B20111956DEEEBA3154EBA97A4AB182A05F53808504016C08CA1E4FC658B3DE06ABAFEAF67054005471F37267B6D1746037C3D0EF958F9D66BFDEA673F01 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE72E4E5201E1C2E308EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006371D26D2A8652661258638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D899D8E693B5C957098CE6F358E7D4572A1A9C4588F4DFFB34CC7F00164DA146DAFE8445B8C89999728AA50765F7900637CAEE156C82D3D7D9389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC81D471462564A2E19F6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947CF254576263B31EA96E0066C2D8992A164AD6D5ED66289B523666184CF4C3C14F6136E347CC761E07725E5C173C3A84C319CDA2F3EEE8CC63BA3038C0950A5D36B5C8C57E37DE458B330BD67F2E7D9AF16D1867E19FE14079C09775C1D3CA48CF4964A708C60C975A1DD303D21008E298D5E8D9A59859A8B6B372FE9A2E580EFC725E5C173C3A84C3515BD7C68D321F4835872C767BF85DA2F004C90652538430E4A6367B16DE6309 X-C1DE0DAB: 0D63561A33F958A5BA1A22F94DE6D5315002B1117B3ED6962CFC0DFE3A5ACF78B91D2EB2DEE3878C823CB91A9FED034534781492E4B8EEAD3CCD70CEBBF18A22BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF013DB1EC40080A7C5B1ED535A4F7043CEF278EC5BD926AE43B21E8AE30DE5E38A161E968D9DBB349F212F37558E724660342DBB3A2EE39B19683AA59743FBA5C1AECED992EB63D3EC226CC413062362A913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojhu0AwaaKnHo5+v12f0s2lQ== X-Mailru-Sender: 520A125C2F17F0B1A9638AD358559B5950E971867008FEBC3DE06ABAFEAF67054005471F37267B6DB7CBEF92542CD7C8795FA72BAB74744FC77752E0C033A69EA16A481184E8BB1C9B38E6EA4F046BE03A5DB60FBEB33A8A0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 1/2] OSX/iOS/ARM64: Fix generation of Mach-O object files. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Cc: Sergey Bronnikov , tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Max, changes force-pushed. On 3/19/24 19:28, Maxim Kokryashkin wrote: > Hi, Sergey! > Thanks for the fixes! > Please consider my answers below. > > On Tue, Mar 19, 2024 at 11:19:19AM +0300, Sergey Bronnikov wrote: >> Max, >> >> thanks for review! see comments below >> >> >> On 3/18/24 15:53, Maxim Kokryashkin wrote: >>> Hi, Sergey, thanks for the patch! >>> Please consider my comments below. >>> On Thu, Mar 14, 2024 at 02:39:49PM +0300, Sergey Bronnikov wrote: > >>>> .github/workflows/exotic-builds-testing.yml | 5 +- >>>> src/jit/bcsave.lua | 6 +- >>>> .../lj-366-strtab-correct-size.test.lua | 10 +- >>>> ...generation_of_mach-o_object_files.test.lua | 271 ++++++++++++++++++ >>>> test/tarantool-tests/utils/tools.lua | 8 + >>>> 5 files changed, 287 insertions(+), 13 deletions(-) >>>> create mode 100644 test/tarantool-tests/lj-865-fix_generation_of_mach-o_object_files.test.lua >>>> >>>> diff --git a/.github/workflows/exotic-builds-testing.yml b/.github/workflows/exotic-builds-testing.yml >>>> index a9ba5fd5..df4bc2e9 100644 >>>> --- a/.github/workflows/exotic-builds-testing.yml >>>> +++ b/.github/workflows/exotic-builds-testing.yml >>>> @@ -32,6 +32,7 @@ jobs: >>>> fail-fast: false >>>> matrix: >>>> BUILDTYPE: [Debug, Release] >>>> + OS: [Linux, macOS] >>>> ARCH: [ARM64, x86_64] >>>> GC64: [ON, OFF] >>>> FLAVOR: [checkhook, dualnum, gdbjit, nojit, nounwind] >>>> @@ -50,13 +51,15 @@ jobs: >>>> FLAVORFLAGS: -DLUAJIT_USE_GDBJIT=ON >>> https://github.com/tarantool/luajit/actions/runs/8279362128 >>>> - FLAVOR: nounwind >>>> FLAVORFLAGS: -DLUAJIT_NO_UNWIND=ON >>>> + - FLAVOR: avx512 >>>> + CMAKEFLAGS: -DCMAKE_C_FLAGS=skylake-avx512 -DCMAKE_C_COMPILER=gcc >>>> exclude: >>>> - ARCH: ARM64 >>>> GC64: OFF >>>> # DUALNUM is default for ARM64, no need for additional testing. >>>> - FLAVOR: dualnum >>>> ARCH: ARM64 >>>> - runs-on: [self-hosted, regular, Linux, '${{ matrix.ARCH }}'] >>>> + runs-on: [self-hosted, regular, Linux, '${{ matrix.ARCH }}', '${ matrix.OS }'] >>> The matrix.OS variable should be wrapped with double curly braces, >>> instead of singular ones. So, it should be like this: >>> | '${{ matrix.OS }}' >> Fixed, thanks! >> >> >>> Currently, exotic build testing fails to start because of this mistake. >>> https://github.com/tarantool/luajit/actions/runs/8279362128 > That's unfortunate, but the workflow run still fails to start. It seems > like double quotation marks are required around the computed properties. > At least, they are mentioned specifically in the documentation: > https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idruns-on > > So both variables should actually be: > | "${{ matrix.OS }}", "${{ matrix.ARCH }}" Fixed. Patch with fix is below: --- a/.github/workflows/exotic-builds-testing.yml +++ b/.github/workflows/exotic-builds-testing.yml @@ -35,7 +35,7 @@ jobs:          OS: [Linux, macOS]          ARCH: [ARM64, x86_64]          GC64: [ON, OFF] -        FLAVOR: [checkhook, dualnum, gdbjit, nojit, nounwind] +        FLAVOR: [checkhook, dualnum, gdbjit, nojit, nounwind, avx512]          include:            - BUILDTYPE: Debug              CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON >>>> name: > >>>> LuaJIT ${{ matrix.FLAVOR }} >>>> (Linux/${{ matrix.ARCH }}) >>>> diff --git a/src/jit/bcsave.lua b/src/jit/bcsave.lua >>>> index a287d675..7aec1555 100644 >>>> --- a/src/jit/bcsave.lua >>>> +++ b/src/jit/bcsave.lua >>>> @@ -446,18 +446,18 @@ typedef struct { >>>> uint32_t value; >>>> } mach_nlist; >>>> typedef struct { >>>> - uint32_t strx; >>>> + int32_t strx; >>>> uint8_t type, sect; >>>> uint16_t desc; >>>> uint64_t value; >>>> } mach_nlist_64; >>>> typedef struct >>>> { >>>> - uint32_t magic, nfat_arch; >>>> + int32_t magic, nfat_arch; >>>> } mach_fat_header; >>>> typedef struct >>>> { >>>> - uint32_t cputype, cpusubtype, offset, size, align; >>>> + int32_t cputype, cpusubtype, offset, size, align; >>>> } mach_fat_arch; >>>> typedef struct { >>>> struct { >>>> diff --git a/test/tarantool-tests/lj-366-strtab-correct-size.test.lua b/test/tarantool-tests/lj-366-strtab-correct-size.test.lua >>>> index 8a97a441..0bb92da6 100644 >>>> --- a/test/tarantool-tests/lj-366-strtab-correct-size.test.lua >>>> +++ b/test/tarantool-tests/lj-366-strtab-correct-size.test.lua >>> Let's move this update to a separate patch alongside with the added >>> utility function. Currently, this change in unrelated test file is >>> a bit confusing. >> >> Moved to a separate patch: >> >> >> commit 138bcda850ed4aad4bbbae4d30315d9a0934cb26 >> Author: Sergey Bronnikov >> Date:   Tue Mar 19 09:56:51 2024 +0300 >> >>     test: introduce a helper read_file >> >>     The test `lj-366-strtab-correct-size.test.lua` has a test helper >>     `read_file` that read a file's content and returns it. > Typo: s/read/reads/ Fixed. >>     This helper will be useful for a test upcoming in the next commit, >>     so it is moved to a test tools. > Typo: s/to a/to/ Fixed. >> diff --git a/test/tarantool-tests/lj-366-strtab-correct-size.test.lua >> b/test/tarantool-tests/lj-366-strtab-correct-size.test.lua >> index 8a97a441..0bb92da6 100644 >> --- a/test/tarantool-tests/lj-366-strtab-correct-size.test.lua >> +++ b/test/tarantool-tests/lj-366-strtab-correct-size.test.lua >> @@ -138,14 +138,6 @@ local function create_obj_file(name) >>    return elf_filename >>  end >> >> --- Reads a file located in a specified path and returns its content. >> -local function read_file(path) >> -  local file = assert(io.open(path), 'cannot open an object file') >> -  local content = file:read('*a') >> -  file:close() >> -  return content >> -end >> - >>  -- Parses a buffer in an ELF format and returns an offset and a size of >> strtab >>  -- and symtab sections. >>  local function read_elf(elf_content) >> @@ -172,7 +164,7 @@ end >>  test:plan(3) >> >>  local elf_filename = create_obj_file(MODULE_NAME) >> -local elf_content = read_file(elf_filename) >> +local elf_content = require('utils').tools.read_file(elf_filename) >>  assert(#elf_content ~= 0, 'cannot read an object file') >> >>  local strtab, symtab = read_elf(elf_content) >> diff --git a/test/tarantool-tests/utils/tools.lua >> b/test/tarantool-tests/utils/tools.lua >> index f35c6922..26b8c08d 100644 >> --- a/test/tarantool-tests/utils/tools.lua >> +++ b/test/tarantool-tests/utils/tools.lua >> @@ -12,4 +12,12 @@ function M.profilename(name) >>    return (arg[0]:gsub('^(.+)/([^/]+)%.test%.lua$', replacepattern)) >>  end >> >> +-- Reads a file located in a specified path and returns its content. >> +function M.read_file(path) >> +  local file = assert(io.open(path), 'cannot open an object file') >> +  local content = file:read('*a') >> +  file:close() >> +  return content >> +end >> + >>  return M >> >> >>>> @@ -138,14 +138,6 @@ local function create_obj_file(name) >>>> return elf_filename >>>> end >>>> >>>> --- Reads a file located in a specified path and returns its content. >>>> -local function read_file(path) >>>> - local file = assert(io.open(path), 'cannot open an object file') >>>> - local content = file:read('*a') >>>> - file:close() >>>> - return content >>>> -end >>>> - >>>> -- Parses a buffer in an ELF format and returns an offset and a size of strtab >>>> -- and symtab sections. >>>> local function read_elf(elf_content) >>>> @@ -172,7 +164,7 @@ end >>>> test:plan(3) >>>> >>>> local elf_filename = create_obj_file(MODULE_NAME) >>>> -local elf_content = read_file(elf_filename) >>>> +local elf_content = require('utils').tools.read_file(elf_filename) >>>> assert(#elf_content ~= 0, 'cannot read an object file') >>>> >>>> local strtab, symtab = read_elf(elf_content) > The newly added patch LGTM. > >>>> 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 >>>> new file mode 100644 >>>> index 00000000..0519e134 >>>> --- /dev/null >>>> +++ b/test/tarantool-tests/lj-865-fix_generation_of_mach-o_object_files.test.lua >>>> @@ -0,0 +1,271 @@ > >>> Side note: I feel like the comments for the sections are not elaborate >>> enough for unprepared reader. I think you should briefly desribe the >>> basic structure of a FAT object (FAT header, then array of per-segment >>> headers, then object files) >> Added a short description and an ASCII scheme. > The scheme is very nice and informative, thanks! > > > > Regards, > Maxim Kokryashkin