Hi, Sergey

Hi, Sergey!
Thanks for the fixes!
LGTM after fixing a few minor nits below.

On 11.04.24, Sergey Bronnikov wrote:

From: Mike Pall <mike>

<snipped>

---
 src/jit/bcsave.lua                            |   6 +-
 test/LuaJIT-tests/CMakeLists.txt              |   9 +
 ...-865-cross-generation-mach-o-file.test.lua | 300 ++++++++++++++++++
 3 files changed, 312 insertions(+), 3 deletions(-)
 create mode 100644 test/tarantool-tests/lj-865-cross-generation-mach-o-file.test.lua

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

<snipped>

diff --git a/test/LuaJIT-tests/CMakeLists.txt b/test/LuaJIT-tests/CMakeLists.txt
index b8e4dfc4..6d073700 100644
--- a/test/LuaJIT-tests/CMakeLists.txt
+++ b/test/LuaJIT-tests/CMakeLists.txt
@@ -52,6 +52,15 @@ if(LUAJIT_NO_UNWIND)
   set(LUAJIT_TEST_TAGS_EXTRA +internal_unwinder)
 endif()
 
+if(CMAKE_C_FLAGS MATCHES "-march=skylake-avx512")
+  # FIXME: Test <bit64.lua> verifies bitwise operations on numbers.

Nit: comment line width is more than 66 symbols.

Fixed.

+  # There is a known issue - bitop doesn't work in LuaJIT built
+  # with the enabled AVX512 instruction set, see
+  # https://github.com/tarantool/tarantool/issues/6787.
+  # Hence, skip this when "skylake-avx512" is passed.
+  set(LUAJIT_TEST_TAGS_EXTRA +avx512)
+endif()
+

Should this be a part of the first commit?

Moved to the first commit.

 set(TEST_SUITE_NAME "LuaJIT-tests")
 
 # XXX: The call produces both test and target <LuaJIT-tests-deps>
diff --git a/test/tarantool-tests/lj-865-cross-generation-mach-o-file.test.lua b/test/tarantool-tests/lj-865-cross-generation-mach-o-file.test.lua
new file mode 100644
index 00000000..04fb5495
--- /dev/null
+++ b/test/tarantool-tests/lj-865-cross-generation-mach-o-file.test.lua
@@ -0,0 +1,300 @@
+local tap = require('tap')
+local test = tap.test('lj-865-cross-generation-mach-o-file')
+local utils = require('utils')
+
+test:plan(1)
+
+-- The test creates an object file in Mach-O format with LuaJIT
+-- bytecode and checks the validity of the object file fields.
+--
+-- The original problem is reproduced with LuaJIT that built with

Typo: s/LuaJIT that built/LuaJIT, which is built/

+-- enabled AVX512F instructions. The support for AVX512F could be
+-- checked in `/proc/cpuinfo` on Linux and
+-- `sysctl hw.optional.avx512f` on Mac. AVX512F must be
+-- implicitly enabled in a C compiler by passing a CPU codename.
+-- Please take a look at the GCC Online Documentation [1] for
+-- available CPU codenames. Also, see the Wikipedia for CPUs with
+-- AVX-512 support [2].
+-- To detect the CPU codename execute:

Typo: s/codename/codename,/

+-- `gcc -march=native -Q --help=target | grep march`.
+--
+-- 1. https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html
+-- 2. https://en.wikipedia.org/wiki/AVX-512#CPUs_with_AVX-512
+--
+-- Manual steps for reproducing are the following:
+--
+-- $ CC=gcc TARGET_CFLAGS='skylake-avx512' cmake -S . -B build
+-- $ cmake --build build --parallel
+-- $ echo > test.lua
+-- $ LUA_PATH="src/?.lua;;" luajit -b -o osx -a arm test.lua test.o
+-- $ file test.o
+-- empty.o: DOS executable (block device driver)
+
+local ffi = require('ffi')

Nit: Why do we require the ffi here alongside from others requires?

+
+-- LuaJIT can generate so called Universal Binary with Lua

<snipped>

+--
+-- There are a good visual representation of Universal Binary

Typo: s/are/is/

+-- in "Mac OS X Internals" book (pages 67-68) [5] and in the [6].
+-- Below is the schematic structure of Universal Binary, which
+-- includes two executables for PowerPC and Intel i386 (omitted):
+--
+--   0x0000000 ---------------------------------------

<snipped>

+local function create_obj_file(name, arch)
+  local mach_o_path = os.tmpname() .. '.o'
+  local lua_path = os.getenv('LUA_PATH')
+  local lua_bin = utils.exec.luacmd(arg):match('%S+')
+  local cmd_fmt = 'LUA_PATH="%s" %s -b -n "%s" -o osx -a %s -e "print()" %s'
+  local cmd = (cmd_fmt):format(lua_path, lua_bin, name, arch, mach_o_path)

Nit: Typo: s/(cmd_fmt)/cmd_fmt/

+  local ret = os.execute(cmd)
+  assert(ret == 0, 'cannot create an object file')
+  return mach_o_path
+end
+
+-- Parses a buffer in the Mach-O format and returns

Nit: The comment line looks underfilled.

+-- the FAT magic number and `nfat_arch`.
+local function read_mach_o(buf)

<snipped>

+local SUM_CPUTYPE = {

Minor: It will be nice to add the comment:
|  -- x86 + arm.

+  arm = 7 + 12,
+}
+local SUM_CPUSUBTYPE = {

Minor: It will be nice to add the comment:
|  -- x86 + arm.

+  arm = 3 + 9,
+}
+

<snipped>

+local function build_and_check_mach_o(subtest, hw_arch)
+  assert(hw_arch == 'arm')
+
+  subtest:plan(4)

<snipped>

+test:test('arm', build_and_check_mach_o, 'arm')

Minor: we can use `subtest.name` as the definition of the `hw_arch` in the
`build_and_check_mach_o()`, so it helps to avoid duplication of arch
usage.

Matter of taste.
Feel free to ignore.

+
+test:done(true)
-- 
2.34.1