<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Hi, Sergey</p>
<p>thanks for review! Please see my comment below.</p>
<p><br>
</p>
<div class="moz-cite-prefix">On 08.04.2024 18:01, Sergey Kaplun
wrote:<br>
</div>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">Hi, Sergey!
Thanks for the patch!
I'll proceed with the version from the branch.
Please consider my comments below.
Please rebase your branch to the tarantool/master and solve the
conflicts.
On 26.03.24, Sergey Bronnikov wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Subject: [PATCH 2/3] OSX/iOS/ARM64: Fix generation of Mach-O object files.
Thanks to Carlo Cabrera.
(cherry picked from commit 3065c910ad6027031aabe2dfd3c26a3d0f014b4f)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
<snipped>
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
Part of tarantool/tarantool#9595
---
.github/workflows/exotic-builds-testing.yml | 6 +-
src/jit/bcsave.lua | 6 +-
test/LuaJIT-tests/CMakeLists.txt | 9 +
test/LuaJIT-tests/lib/ffi/index | 2 +-
...generation_of_mach-o_object_files.test.lua | 343 ++++++++++++++++++
5 files changed, 361 insertions(+), 5 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 859603bd..9cace0bc 100644
--- a/.github/workflows/exotic-builds-testing.yml
+++ b/.github/workflows/exotic-builds-testing.yml
@@ -34,7 +34,7 @@ jobs:
BUILDTYPE: [Debug, Release]
ARCH: [ARM64, x86_64]
GC64: [ON, OFF]
- FLAVOR: [checkhook, dualnum, gdbjit, nojit, nounwind]
+ FLAVOR: [checkhook, dualnum, gdbjit, nojit, nounwind, avx512]
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Please sort entries alphabetically.
</pre>
</blockquote>
<p>Done.</p>
<p><br>
</p>
<p>--- a/.github/workflows/exotic-builds-testing.yml<br>
+++ b/.github/workflows/exotic-builds-testing.yml<br>
@@ -34,7 +34,7 @@ jobs:<br>
BUILDTYPE: [Debug, Release]<br>
ARCH: [ARM64, x86_64]<br>
GC64: [ON, OFF]<br>
- FLAVOR: [checkhook, dualnum, gdbjit, nojit, nounwind,
avx512]<br>
+ FLAVOR: [ avx512, checkhook, dualnum, gdbjit, nojit,
nounwind ]<br>
include:<br>
- BUILDTYPE: Debug<br>
CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug
-DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON<br>
</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> include:
- BUILDTYPE: Debug
CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
@@ -50,12 +50,16 @@ jobs:
FLAVORFLAGS: -DLUAJIT_USE_GDBJIT=ON
- FLAVOR: nounwind
FLAVORFLAGS: -DLUAJIT_NO_UNWIND=ON
+ - FLAVOR: avx512
+ CMAKEFLAGS: -DCMAKE_C_FLAGS=-march=skylake-avx512 -DCMAKE_C_COMPILER=gcc
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Is there any guarantee that the given runner has avx512 support?
If not, our tests will fail due to illegal instruction error.
</pre>
</blockquote>
<p>I'll add a check for AVX support and condition that will skip job
if there is no AVX support.</p>
<p><br>
</p>
<span style="white-space: pre-wrap">
</span>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> exclude:
- ARCH: ARM64
GC64: OFF
# DUALNUM is default for ARM64, no need for additional testing.
- FLAVOR: dualnum
ARCH: ARM64
+ - FLAVOR: avx512
+ ARCH: ARM64
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Please sort entries alphabetically.
</pre>
</blockquote>
<br>
<p>Fixed:</p>
<p>--- a/.github/workflows/exotic-builds-testing.yml<br>
+++ b/.github/workflows/exotic-builds-testing.yml<br>
@@ -34,12 +34,14 @@ jobs:<br>
BUILDTYPE: [Debug, Release]<br>
ARCH: [ARM64, x86_64]<br>
GC64: [ON, OFF]<br>
- FLAVOR: [checkhook, dualnum, gdbjit, nojit, nounwind,
avx512]<br>
+ FLAVOR: [ avx512, checkhook, dualnum, gdbjit, nojit,
nounwind ]<br>
include:<br>
- BUILDTYPE: Debug<br>
CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug
-DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON<br>
- BUILDTYPE: Release<br>
CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo<br>
+ - FLAVOR: avx512<br>
+ CMAKEFLAGS: -DCMAKE_C_FLAGS=-march=skylake-avx512
-DCMAKE_C_COMPILER=gcc<br>
- FLAVOR: dualnum<br>
FLAVORFLAGS: -DLUAJIT_NUMMODE=2<br>
- FLAVOR: checkhook<br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap=""></pre>
</blockquote>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> runs-on: [self-hosted, regular, Linux, '${{ matrix.ARCH }}']
name: >
LuaJIT ${{ matrix.FLAVOR }}
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/LuaJIT-tests/CMakeLists.txt b/test/LuaJIT-tests/CMakeLists.txt
index a0fb5440..29146113 100644
--- a/test/LuaJIT-tests/CMakeLists.txt
+++ b/test/LuaJIT-tests/CMakeLists.txt
@@ -64,6 +64,15 @@ if(LUAJIT_NO_UNWIND)
set(LUAJIT_TEST_TAGS_EXTRA +internal_unwinder)
endif()
+if(CMAKE_C_FLAGS MATCHES "-march=skylake-avx512")
+ # Test <bit64.lua> verifies bitwise operations on numbers.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Maybe we should add FIXME: tag here. Fill free to ignore.
</pre>
</blockquote>
<p>Added.</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ # There is a known issue - bitop doesn't working in LuaJIT
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/working/work/</pre>
</blockquote>
<p>Fixed.</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ # built with enabled AVX512 instruction set,
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/enabled/the enabled/</pre>
</blockquote>
<p>Fixed.</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ # see <a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/issues/6787">https://github.com/tarantool/tarantool/issues/6787</a>.
+ # Hence, skip this when "skylake-avx512" is passed.
+ set(LUAJIT_TEST_TAGS_EXTRA +avx512)
+endif()
+
add_custom_command(TARGET LuaJIT-tests
COMMENT "Running LuaJIT-tests"
COMMAND
diff --git a/test/LuaJIT-tests/lib/ffi/index b/test/LuaJIT-tests/lib/ffi/index
index e3a34e30..e7c168c2 100644
--- a/test/LuaJIT-tests/lib/ffi/index
+++ b/test/LuaJIT-tests/lib/ffi/index
@@ -1,4 +1,4 @@
-bit64.lua +luajit>=2.1
+bit64.lua +luajit>=2.1 -avx512
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Do we need to skip all tests instead of only failed one?
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> cdata_var.lua
copy_fill.lua
err.lua
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
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Please, use "-" instead of "_" as a separator, according to other test
names.
Also, the test name is too long. I suggest something like the
following:
| lj-865-cross-generation-mach-o-file.test.lua
Don't forget to update the testname below as well.
</pre>
</blockquote>
<p>Fixed.</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">new file mode 100644
index 00000000..40bd1c74
--- /dev/null
+++ b/test/tarantool-tests/lj-865-fix_generation_of_mach-o_object_files.test.lua
@@ -0,0 +1,343 @@
+local tap = require('tap')
+local test = tap.test('lj-865-fix_generation_of_mach-o_object_files')
+
+test:plan(4)
+
+-- The test creates an object file in Mach-O format with LuaJIT
+-- bytecode and checks the validness of the object file fields.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo? s/validness/validity/
IINM, validness is the obsolete form.
</pre>
</blockquote>
<p>Fixed.</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+--
+-- The original problem is reproduced with LuaJIT that built with
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/LuaJIT that built/LuaJIT, which is built/
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- 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 CPU codename.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/CPU codename/a CPU codename/
</pre>
</blockquote>
Fixed<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- Please consult for the available model architecture on
+-- GCC Online Documentation [1] for available CPU codenames.
+-- and Wikipedia for CPUs with AVX-512 support [2].
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
I suggest the following rewording:
| Please consult the GCC Online Documentation [1] for available CPU
| codenames. Also, see the Wikipedia for CPUs with AVX-512 support [2].
Or the follwing:
| 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].
Feel free to reword it like you want.</pre>
</blockquote>
<p>What's wrong with provided text?</p>
<p>Why should I change it?<span style="white-space: pre-wrap">
</span></p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- To detect CPU codename execute:
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/CPU codename/the CPU codename/</pre>
</blockquote>
<p>Fixed.</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- `gcc -march=native -Q --help=target | grep march`.
+--
+-- 1. <a class="moz-txt-link-freetext" href="https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html">https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html</a>
+-- 2. <a class="moz-txt-link-freetext" href="https://en.wikipedia.org/wiki/AVX-512#CPUs_with_AVX-512">https://en.wikipedia.org/wiki/AVX-512#CPUs_with_AVX-512</a>
+--
+-- Manual steps for reproducing are the following:
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
I suppose you mean "from the original issue". Or we can use
| -DCMAKE_C_FLAGS="-march=skylake-avx512"
</pre>
</blockquote>
These steps works in our fork too.<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+--
+-- $ make CC=gcc TARGET_CFLAGS='skylake-avx512' -f Makefile.original
+-- $ 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')
+
+-- LuaJIT can generate so called Universal Binary with Lua bytetcode.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Comment width is more than 66 symbols.</pre>
</blockquote>
Fixed.<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- The Universal Binary format is a format for executable files
+-- that run natively on hardware platforms with different hardware
+-- architectures. This concept is more generally known as a fat binary.
+--
+-- Format of the Mach-O is described in the document
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/Format/The format/</pre>
</blockquote>
Fixed.<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- "OS X ABI Mach-O File Format Reference", published by Apple company.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo? s/Reference",/Reference,"/</pre>
</blockquote>
Don't get it. Why comma should be inside quotes?<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- Copy of the (now removed) official documentation can be found
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/Copy/The copy/</pre>
</blockquote>
Fixed.<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- here [1]. Yet another source of thruth are XNU headers,
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/thruth/truth/</pre>
</blockquote>
<p>Fixed.</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- see definition of C-structures in: [2] (`nlist_64`),
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/definition/the definition/</pre>
</blockquote>
Fixed.<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- [3] (`fat_arch` and `fat_header`).
+--
+-- There are a good visual representation of Universal Binary
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/are/is/</pre>
</blockquote>
<p>Fixed.</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- in "Mac OS X Internals" book (pages 67-68) [5] and in [6].
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/in/in the/g</pre>
</blockquote>
<p>Fixed.</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- Below is schematic structure of Universal Binary that
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/schematic/the schematic/
Typo: s/Binary that/Binary, which/</pre>
</blockquote>
<p><br>
</p>
<p>Fixed.<br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- includes two executables for PowerPC and Intel i386 (omitted):
+--
+-- 0x0000000 ---------------------------------------
+-- |
+-- struct | 0xcafebabe FAT_MAGIC magic
+-- fat_header | -------------------------------------
+-- | 0x00000003 nfat_arch
+-- ---------------------------------------
+-- | 0x00000012 CPU_TYPE_POWERPC cputype
+-- | -------------------------------------
+-- | 0x00000000 CPU_SUBTYPE_POWERPC_ALL cpusubtype
+-- struct | -------------------------------------
+-- fat_arch | 0x00001000 4096 bytes offset
+-- | -------------------------------------
+-- | 0x00004224 16932 bytes size
+-- | -------------------------------------
+-- | 0x0000000c 2^12 = 4096 bytes align
+-- ---------------------------------------
+-- ---------------------------------------
+-- | 0x00000007 CPU_TYPE_I386 cputype
+-- | -------------------------------------
+-- | 0x00000003 CPU_SUBTYPE_I386_ALL cpusubtype
+-- struct | -------------------------------------
+-- fat_arch | 0x00006000 24576 bytes offset
+-- | -------------------------------------
+-- | 0x0000292c 10540 bytes size
+-- | -------------------------------------
+-- | 0x0000000c 2^12 = 4096 bytes align
+-- ---------------------------------------
+-- Unused
+-- 0x00001000 ---------------------------------------
+-- | 0xfeedface MH_MAGIC magic
+-- | ------------------------------------
+-- | 0x00000012 CPU_TYPE_POWERPC cputype
+-- | ------------------------------------
+-- struct | 0x00000000 CPU_SUBTYPE_POWERPC_ALL cpusubtype
+-- mach_header | ------------------------------------
+-- | 0x00000002 MH_EXECUTE filetype
+-- | ------------------------------------
+-- | 0x0000000b 10 load commands ncmds
+-- | ------------------------------------
+-- | 0x00000574 1396 bytes sizeofcmds
+-- | ------------------------------------
+-- | 0x00000085 DYLDLINK TWOLEVEL flags
+-- --------------------------------------
+-- Load commands
+-- ---------------------------------------
+-- Data
+-- ---------------------------------------
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Side note: This diagramm is amazing thanks!
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+--
+-- < x86 executable >
+--
+--
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Nit: excess empty line.
</pre>
</blockquote>
<p>Removed.</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- 1. <a class="moz-txt-link-freetext" href="https://github.com/aidansteele/osx-abi-macho-file-format-reference">https://github.com/aidansteele/osx-abi-macho-file-format-reference</a>
+-- 2. <a class="moz-txt-link-freetext" href="https://github.com/apple-oss-distributions/xnu/blob/xnu-10002.1.13/EXTERNAL_HEADERS/mach-o/nlist.h">https://github.com/apple-oss-distributions/xnu/blob/xnu-10002.1.13/EXTERNAL_HEADERS/mach-o/nlist.h</a>
+-- 3. <a class="moz-txt-link-freetext" href="https://github.com/apple-oss-distributions/xnu/blob/xnu-10002.1.13/EXTERNAL_HEADERS/mach-o/fat.h">https://github.com/apple-oss-distributions/xnu/blob/xnu-10002.1.13/EXTERNAL_HEADERS/mach-o/fat.h</a>
+-- 4. <a class="moz-txt-link-freetext" href="https://developer.apple.com/documentation/apple-silicon/addressing-architectural-differences-in-your-macos-code">https://developer.apple.com/documentation/apple-silicon/addressing-architectural-differences-in-your-macos-code</a>
+-- 5. <a class="moz-txt-link-freetext" href="https://reverseengineering.stackexchange.com/a/6357/46029">https://reverseengineering.stackexchange.com/a/6357/46029</a>
+-- 6. <a class="moz-txt-link-freetext" href="http://formats.kaitai.io/mach_o/index.html">http://formats.kaitai.io/mach_o/index.html</a>
+--
+-- Using the same declarations as defined in <src/jit/bcsave.lua>.
+ffi.cdef[[
+typedef struct
+{
+ uint32_t magic, cputype, cpusubtype, filetype, ncmds, sizeofcmds, flags;
+} mach_header;
+
+typedef struct
+{
+ mach_header; uint32_t reserved;
+} mach_header_64;
+
+typedef struct {
+ uint32_t cmd, cmdsize;
+ char segname[16];
+ uint32_t vmaddr, vmsize, fileoff, filesize;
+ uint32_t maxprot, initprot, nsects, flags;
+} mach_segment_command;
+
+typedef struct {
+ uint32_t cmd, cmdsize;
+ char segname[16];
+ uint64_t vmaddr, vmsize, fileoff, filesize;
+ uint32_t maxprot, initprot, nsects, flags;
+} mach_segment_command_64;
+
+typedef struct {
+ char sectname[16], segname[16];
+ uint32_t addr, size;
+ uint32_t offset, align, reloff, nreloc, flags;
+ uint32_t reserved1, reserved2;
+} mach_section;
+
+typedef struct {
+ char sectname[16], segname[16];
+ uint64_t addr, size;
+ uint32_t offset, align, reloff, nreloc, flags;
+ uint32_t reserved1, reserved2, reserved3;
+} mach_section_64;
+
+typedef struct {
+ uint32_t cmd, cmdsize, symoff, nsyms, stroff, strsize;
+} mach_symtab_command;
+
+typedef struct {
+ int32_t strx;
+ uint8_t type, sect;
+ int16_t desc;
+ uint32_t value;
+} mach_nlist;
+
+typedef struct {
+ uint32_t strx;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Should we use here `int32_t` as it is after the changes?</pre>
</blockquote>
Yes, fixed.<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ uint8_t type, sect;
+ uint16_t desc;
+ uint64_t value;
+} mach_nlist_64;
+
+typedef struct
+{
+ uint32_t magic, nfat_arch;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Should we use here `int32_t` as it is after the changes?</pre>
</blockquote>
Fixed.<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+} mach_fat_header;
+
+typedef struct
+{
+ uint32_t cputype, cpusubtype, offset, size, align;
</pre>
</blockquote>
</blockquote>
<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
Should we use here `int32_t` as it is after the changes?</pre>
</blockquote>
<p>Fixed.</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+} mach_fat_arch;
+
+typedef struct {
+ mach_fat_header fat;
+ mach_fat_arch fat_arch[2];
+ struct {
+ mach_header hdr;
+ mach_segment_command seg;
+ mach_section sec;
+ mach_symtab_command sym;
+ } arch[2];
+ 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;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Should this part be defined in the next patch, since there is no such
cdef at the moment?
OTOH, it makes test more arch-agnostic. So, the adding of the other case
is more simple.
</pre>
</blockquote>
Moved to the third patch.<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+]]
+
+local function create_obj_file(name, arch)
+ local mach_o_path = os.tmpname() .. '.o'
+ local lua_path = os.getenv('LUA_PATH')
+ local lua_bin = require('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)
+ 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
+-- the FAT magic number and `nfat_arch`.
+local function read_mach_o(buf, hw_arch)
+ local res = {
+ header = {
+ magic = 0,
+ nfat_arch = 0,
+ },
+ fat_arch = {},
+ }
+
+ local is64 = hw_arch == 'arm64'
+
+ -- Mach-O FAT object.
+ local mach_fat_obj_type = ffi.typeof(is64 and
+ 'mach_fat_obj_64 *' or 'mach_fat_obj *')
+ local obj = ffi.cast(mach_fat_obj_type, buf)
+
+ -- Mach-O FAT object header.
+ local mach_fat_header_type = ffi.typeof('mach_fat_header *')
+ local mach_fat_header = ffi.cast(mach_fat_header_type, obj.fat)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Looks like this cast is excess:
| src/luajit -e '
| local ffi = require"ffi"
| ffi.cdef[[
| struct test {int a; int b;};
| struct test_outer { struct test t; int c;};
| ]]
| local s = ffi.new("struct test_outer", {{0}, 0})
| print(s.t.a)
| '
| 0</pre>
</blockquote>
<p>Code in the test casts cdata to ffi type, you snippet demonstrate
how to create cdata using ffi.new.</p>
<p>It is similar cases?<br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ local be32 = bit.bswap -- Mach-O FAT is BE, target arch is LE.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Please, use the separate line for the comment.</pre>
</blockquote>
Moved to a new line.<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ res.header.magic = be32(mach_fat_header.magic)
+ res.header.nfat_arch = be32(mach_fat_header.nfat_arch)
+
+ -- Mach-O FAT object arches.
+ local mach_fat_arch_type = ffi.typeof('mach_fat_arch *')
+ for i = 0, res.header.nfat_arch - 1 do
+ local fat_arch = ffi.cast(mach_fat_arch_type, obj.fat_arch[i])
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Looks like this cast is excess.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ local arch = {
+ cputype = be32(fat_arch.cputype),
+ cpusubtype = be32(fat_arch.cpusubtype),
+ }
+ table.insert(res.fat_arch, arch)
+ end
+
+ return res
+end
+
+-- Universal Binary can contain executables for more than one
+-- CPU architecture. For simplicity the test compares
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/For simplicity/For simplicity,/</pre>
</blockquote>
Fixed.<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
Nit: The comment line is "underfilled".</pre>
</blockquote>
Fixed.<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- sum of CPU types and CPU subtypes.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/sum/the sum/</pre>
</blockquote>
Fixed.<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- Numbers below are defined in <src/jit/bcsave.lua:bcsave_machobj>.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/Numbers/The numbers/</pre>
</blockquote>
<p>There is no space for "<src/jit/bcsave.lua:bcsave_machob>"
on the line</p>
<p>after adding "The"as you requested. And line become underfileld
if</p>
<p>"<src/jit/bcsave.lua:bcsave_machob>" moved to the next
line. How do it better:</p>
<p>leave a free space on the line or left "Number" without article?<br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- Original source is XNU source code, <osfmk/mach/machine.h> [1].
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/Original/The original/
Typo: s/XNU/the XNU/</pre>
</blockquote>
the same as above, with added articles line looks underfilled and
therefore it looks ugly.<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+--
+-- 1. <a class="moz-txt-link-freetext" href="https://opensource.apple.com/source/xnu/xnu-4570.41.2/osfmk/mach/machine.h.auto.html">https://opensource.apple.com/source/xnu/xnu-4570.41.2/osfmk/mach/machine.h.auto.html</a>
+local sum_cputype = {
+ x86 = 7,
+ x64 = 0x01000007,
+ arm = 7 + 12,
+ arm64 = 0x01000007 + 0x0100000c,
+}
+local sum_cpusubtype = {
+ x86 = 3,
+ x64 = 3,
+ arm = 3 + 9,
+ arm64 = 3 + 0,
+}
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
I suggest the following for the clarification:
| local cputype = {
| x86 = 7,
| x64 = 0x01000007,
| arm = 7,
| arm64 = 0x0100000c,
| }
| local cpusubtype = {
| x86 = 3,
| x64 = 3,
| arm = 9,
| arm64 = 0,
| }
And declare next:
| local sum_cputype = {
| arm = cputype.arm + cputype.x86,
| arm64 = cputype.arm64 + cputype.x64,
| }
| local sum_cpusubtype = {
| arm = cpusubtype.arm + cpusubtype.x86,
| arm64 = cpusubtype.arm64 + cpusubtype.x64,
| }</pre>
</blockquote>
<p>You proposed changes breaks the test.</p>
<p>See original dictionaries</p>
<p><a class="moz-txt-link-freetext" href="https://github.com/tarantool/luajit/blob/ecc45358ddab52c2d0100c74da8efddef2aaf365/src/jit/bcsave.lua#L512-L513">https://github.com/tarantool/luajit/blob/ecc45358ddab52c2d0100c74da8efddef2aaf365/src/jit/bcsave.lua#L512-L513</a></p>
<p> local cputype = ({ x86={7}, x64={0x01000007}, arm={7,12},
arm64={0x01000007,0x0100000c} })[ctx.arch]</p>
<p><br>
</p>
<p>converted to sum_cputype = { x86 = 7, x64 = 0x01000007, arm = 7 +
12, arm64 = 0x01000007 + 0x0100000c}<br>
</p>
<p><br>
</p>
<p> local cpusubtype = ({ x86={3}, x64={3}, arm={3,9}, arm64={3,0}
})[ctx.arch]</p>
<p><br>
</p>
<p>converted to sum_cpusubtype = { x86 = 3, x64 = 3, arm = 3 + 9,
arm64 = 3 + 0 }<br>
</p>
<p><br>
</p>
<p>I don't know how to make this place in the test more clear. See
comment that explains these arrays:</p>
<p><br>
</p>
<p> -- Universal Binary can contain executables for more than
one <br>
-- CPU architecture. For simplicity, the test compares the sum
of <br>
-- CPU types and CPU
subtypes. <br>
-- Numbers below are defined in
<src/jit/bcsave.lua:bcsave_machobj>. <br>
-- The original source is the XNU source
code, <br>
-- <osfmk/mach/machine.h>
[1]. <br>
-- <br>
</p>
<p><br>
</p>
<p>Ignored.<br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
Also, it is preferable to use upper case since values are constants.</pre>
</blockquote>
<p><br>
</p>
<p>Made everything uppercase.<br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+
+-- The function builds Mach-O FAT object file and retrieves
+-- its header fields (magic and nfat_arch)
+-- and fields of the each arch (cputype, cpusubtype).
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/the each/each/</pre>
</blockquote>
Fixed. Also line above was underfilled, fixed that.<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+--
+-- Mach-O FAT object header can be retrieved with `otool` on macOS:
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/Mach-O FAT object/A Mach-O FAT object/</pre>
</blockquote>
Fixed to "The Mach-O FAT object".<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
Typo: s/header/headers/
</pre>
</blockquote>
single header, why "s"?<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+--
+-- $ otool -f empty.o
+-- Fat headers
+-- fat_magic 0xcafebabe
+-- nfat_arch 2
+-- <snipped>
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Minor: it is better to separate quoting in the comment like the
following to simplify reading:
| $ otool -f empty.o
| Fat headers
| fat_magic 0xcafebabe
| nfat_arch 2
| ...
Feel free to ignore.</pre>
</blockquote>
Ignored.<br>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+--
+-- CPU type and subtype can be retrieved with `lipo` on macOS:
+--
+-- $ luajit -b -o osx -a arm empty.lua empty.o
+-- $ lipo -archs empty.o
+-- i386 armv7
+-- $ luajit -b -o osx -a arm64 empty.lua empty.o
+-- $ lipo -archs empty.o
+-- x86_64 arm64
+local function build_and_check_mach_o(hw_arch)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Maybe it is better to use a subtest here:
The number of tests is fixed for this function, and it becomes more
clear when we add the additional arch for testing.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ assert(hw_arch == 'arm' or hw_arch == 'arm64')
+
+ -- FAT_MAGIC is an integer containing the value 0xCAFEBABE in
+ -- big-endian byte order format. On a big-endian host CPU,
+ -- this can be validated using the constant FAT_MAGIC;
+ -- on a little-endian host CPU, it can be validated using
+ -- the constant FAT_CIGAM.
+ --
+ -- FAT_NARCH is an integer specifying the number of fat_arch
+ -- data structures that follow. This is the number of
+ -- architectures contained in this binary.
+ --
+ -- See aforementioned "OS X ABI Mach-O File Format Reference".
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/aforementioned/the aforementioned/
</pre>
</blockquote>
<p>With added article I need to move name of the reference to a next
line,</p>
<p>and line looks under-filled and ugly.<br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ --
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Excess empty line.</pre>
</blockquote>
<p>It is a visual splitter.</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ local FAT_MAGIC = '0xffffffffcafebabe'
+ local FAT_NARCH = 2
+
+ local MODULE_NAME = 'lango_team'
+
+ local mach_o_obj_path = create_obj_file(MODULE_NAME, hw_arch)
+ local mach_o_buf = require('utils').tools.read_file(mach_o_obj_path)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Please move `require` of the necessary tool to the start of the file to
be consistent with the code style in tests.</pre>
</blockquote>
<p>moved.</p>
<p><br>
</p>
<p>I found that 20 tests in our repository are not consistent too:<br>
<br>
test/tarantool-tests/lj-946-print-errors-from-gc-fin-default.test.lua:local
script = require('utils').exec.makecmd(arg, { redirect =
'2>&1' })<br>
test/tarantool-tests/unit-jit-parse.test.lua:local jparse =
require('utils').jit.parse<br>
test/tarantool-tests/lj-981-folding-0.test.lua:local jparse =
require('utils').jit.parse<br>
test/tarantool-tests/lj-366-strtab-correct-size.test.lua: local
lua_bin = require('utils').exec.luacmd(arg):match('%S+')<br>
test/tarantool-tests/lj-366-strtab-correct-size.test.lua:local
elf_content = require('utils').tools.read_file(elf_filename)<br>
test/tarantool-tests/fix-argv2ctype-cts-L-init.test.lua:local
script = require('utils').exec.makecmd(arg)<br>
grep:
test/tarantool-tests/.lj-736-BC_UCLO-triggers-infinite-loop.lua.swp:
binary file matches<br>
test/tarantool-tests/fix-mips64-spare-side-exit-patching.test.lua:local
generators = require('utils').jit.generators<br>
test/tarantool-tests/fix-mips64-spare-side-exit-patching.test.lua:local
frontend = require('utils').frontend<br>
test/tarantool-tests/lj-1128-table-new-opt-tnew.test.lua:local
jparse = require('utils').jit.parse<br>
test/tarantool-tests/gh-6098-fix-side-exit-patching-on-arm64.test.lua:local
generators = require('utils').jit.generators<br>
test/tarantool-tests/gh-6098-fix-side-exit-patching-on-arm64.test.lua:local
frontend = require('utils').frontend<br>
test/tarantool-tests/lj-351-print-tostring-number.test.lua:local
script = require('utils').exec.makecmd(arg)<br>
test/tarantool-tests/lj-flush-on-trace.test.lua:local script =
require('utils').exec.makecmd(arg, {<br>
test/tarantool-tests/lj-366-strtab-correct-size.test.lua_: local
lua_bin = require('utils').exec.luacmd(arg):match('%S+')<br>
test/tarantool-tests/lj-366-strtab-correct-size.test.lua_:local
elf_content = require('utils').tools.read_file(elf_filename)<br>
test/tarantool-tests/fix-gc-setupvalue.test.lua:local gcisblack =
require('utils').gc.isblack<br>
test/tarantool-tests/misclib-getmetrics-lapi.test.lua:local
MAXNINS = require('utils').jit.const.maxnins<br>
test/tarantool-tests/gh-4427-ffi-sandwich.test.lua:local script =
require('utils').exec.makecmd(arg, {<br>
test/tarantool-tests/fix-jit-dump-ir-conv.test.lua:local jparse =
require('utils').jit.parse<br>
test/tarantool-tests/lj-802-panic-at-mcode-protfail.test.lua:local
script = require('utils').exec.makecmd(arg, {<br>
test/tarantool-tests/lj-962-stack-overflow-report.test.lua:local
LUABIN = require('utils').exec.luabin(arg)<br>
</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ assert(mach_o_buf == nil or #mach_o_buf ~= 0, 'cannot read an object file')
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Should it be:
| mach_o_buf ~= nil and #mach_o_buf ~= 0</pre>
</blockquote>
<p><br>
</p>
<p>Fixed to " assert(mach_o_buf ~= nil and #mach_o_buf ~= 0, 'cannot
read an object file')".<br>
</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+
+ local mach_o = read_mach_o(mach_o_buf, hw_arch)
+
+ -- Teardown.
+ local retcode = os.remove(mach_o_obj_path)
+ assert(retcode == true, 'remove an object file')
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Minor: I suggest refactoring this as the following (`retcode` isn't used
anywhere else):
| assert(os.remove(mach_o_obj_path), 'remove an object file')</pre>
</blockquote>
<p><br>
</p>
<p>Updated.<br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+
+ local magic_str = string.format('0x%02x', mach_o.header.magic)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Why do we need 02 in the format string?
Also, you may use '%#x' for the same result.</pre>
</blockquote>
<p>Fixed.</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ test:is(magic_str, FAT_MAGIC,
+ 'fat_magic is correct in Mach-O, ' .. hw_arch)
+ test:is(mach_o.header.nfat_arch, FAT_NARCH,
+ 'nfat_arch is correct in Mach-O, ' .. hw_arch)
+
+ local total_cputype = 0
+ local total_cpusubtype = 0
+ for i = 1, mach_o.header.nfat_arch do
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Minor: I suggest using `FAT_NARCH` here since we've checked their
equality above.</pre>
</blockquote>
<p>Updated.</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZhQGxWQMYxSKgUB5@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ total_cputype = total_cputype + mach_o.fat_arch[i].cputype
+ total_cpusubtype = total_cpusubtype + mach_o.fat_arch[i].cpusubtype
+ end
+ test:is(total_cputype, sum_cputype[hw_arch],
+ 'cputype is correct in Mach-O, ' .. hw_arch)
+ test:is(total_cpusubtype, sum_cpusubtype[hw_arch],
+ 'cpusubtype is correct in Mach-O, ' .. hw_arch)
+end
+
+build_and_check_mach_o('arm')
+
+test:done(true)
--
2.44.0
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
</pre>
</blockquote>
</body>
</html>