[lld-macho] Support reading of universal binaries

[int3]

update

[int3]

output to /dev/null

[int3]

rebase

[ruiu]

LGTM

[MaskRay]

LGTM, with nits.

lld/test/MachO/fat-arch.s

11

Add more context the the diagnostics, probably error:

It is also a convention to not capitalize the diagnostics, see clang, ld.lld and binutils/LLVM binary utilities' output.

[smeenai]

lld/MachO/InputFiles.cpp

90

We may wanna add some bounds checking here to handle malformed fat files, e.g. if nfat_arch is an absurdly large value and we go over bounds. Idk if yaml2obj is capable of generating a malformed fat file of that sort, but we should at least have some checks.

[smeenai]

lld/test/MachO/fat-arch.s

11

I believe ld64 treats this scenario as a warning, not an error. I wouldn't mind us treating this as an error (at least until we get more experience with the new linker and whether that's reasonable behavior), because in my experience, a mistake like this usually leads to undefined symbols in your link anyway, and it's less confusing if you get an error about the issue right away.

[int3]

address comments

lld/test/MachO/fat-arch.s

11

Add more context the the diagnostics, probably error:

lld/Common/ErrorHandler.h always prints out ANSI color markers around its warning: and error: prefixes. We could use regexes to ignore the colors I guess but it's kind of ugly...

I'll make it an error.

[int3]

copy the file path when returning a MemoryBufferRef

[int3]

rebase

[int3]

rebase

[int3]

fix ASAN issue, add dep on llvm-lipo

[int3]

I realize I'd neglected to add llvm-lipo as a test dependency. Just fixed that, but I've added it as a dependency for all lld test suites (including ELF and COFF); not sure if it's possible to add it for just check-lld-macho.

[MaskRay]

lld/test/MachO/invalid-fat-narch.s

8

You may omit --- and ...

[MaskRay]

lld/test/MachO/fat-arch.s

11

No. Color code sequences are not emitted when stderr is connected to a regular file/pipe instead of a pty.

[int3]

lld/test/MachO/fat-arch.s

11

I agree that's the way it *should* work but I don't think that has been implemented... just from a glance at ErrorHandler.cpp anyway. It is definitely still producing the color markers when I run it under llvm-lit locally though

lld/test/MachO/invalid-fat-narch.s

8

ah thanks, wasn't too familiar with YAML syntax

[int3]

remove redundant yaml stuff

[MaskRay]

lld/test/MachO/fat-arch.s

11

Most test/ELF/* error/warning tests include error: or warning: .

[int3]

lld/test/MachO/fat-arch.s

11

Aha, we weren't setting up the error logger properly in lld-macho's Driver.cpp. I'll put up a diff to fix that. Thanks!

[int3]

add error: prefix

[int3]

rebase

lld/MachO/InputFiles.cpp

Optional<MemoryBufferRef> macho::readFile(StringRef path) {
   make<std::unique_ptr<MemoryBuffer>>(std::move(mb)); // take mb ownership
 
   // If this is a regular non-fat file, return it.
   const char *buf = mbref.getBufferStart();
   auto *hdr = reinterpret_cast<const MachO::fat_header *>(buf);
   if (read32be(&hdr->magic) != MachO::FAT_MAGIC)
     return mbref;
 
-  error("TODO: Add support for universal binaries");
+  // Object files and archive files may be fat files, which contains
+  // multiple real files for different CPU ISAs. Here, we search for a
+  // file that matches with the current link target and returns it as
+  // a MemoryBufferRef.
+  auto *arch = reinterpret_cast<const MachO::fat_arch *>(buf + sizeof(*hdr));
+
+  for (uint32_t i = 0, n = read32be(&hdr->nfat_arch); i < n; ++i) {
+    if (reinterpret_cast<const char *>(arch + i + 1) >
+        buf + mbref.getBufferSize()) {
+      error(path + ": fat_arch struct extends beyond end of file");
+      return None;
+    }
+
+    if (read32be(&arch[i].cputype) != target->cpuType ||
+        read32be(&arch[i].cpusubtype) != target->cpuSubtype)
+      continue;
+
+    uint32_t offset = read32be(&arch[i].offset);
+    uint32_t size = read32be(&arch[i].size);
+    if (offset + size > mbref.getBufferSize())
+      error(path + ": slice extends beyond end of file");
+    return MemoryBufferRef(StringRef(buf + offset, size), path.copy(bAlloc));
+  }
+
+  error("unable to find matching architecture in " + path);
   return None;
 }
 
 static const load_command *findCommand(const mach_header_64 *hdr,
                                        uint32_t type) {
   const uint8_t *p =
       reinterpret_cast<const uint8_t *>(hdr) + sizeof(mach_header_64);
 

lld/test/CMakeLists.txt

configure_lit_site_cfg(
   MAIN_CONFIG
   ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.cfg.py
   )
 
 set(LLD_TEST_DEPS lld)
 if (NOT LLD_BUILT_STANDALONE)
   list(APPEND LLD_TEST_DEPS
     FileCheck count llc llvm-ar llvm-as llvm-bcanalyzer llvm-config llvm-cvtres
-    llvm-dis llvm-dwarfdump llvm-lib llvm-mc llvm-nm llvm-objcopy llvm-objdump
-    llvm-pdbutil llvm-readelf llvm-readobj not obj2yaml opt yaml2obj
+    llvm-dis llvm-dwarfdump llvm-lib llvm-lipo llvm-mc llvm-nm llvm-objcopy
+    llvm-objdump llvm-pdbutil llvm-readelf llvm-readobj not obj2yaml opt
+    yaml2obj
     )
 endif()
 
 if (LLVM_INCLUDE_TESTS)
   list(APPEND LLD_TEST_DEPS LLDUnitTests)
 endif()
 
 add_lit_testsuite(check-lld "Running lld test suite"

lld/test/MachO/fat-arch.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=i386-apple-darwin %s -o %t.i386.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.x86_64.o
+# RUN: llvm-lipo %t.i386.o %t.x86_64.o -create -o %t.fat.o
+# RUN: lld -flavor darwinnew -arch x86_64 -o /dev/null %t.fat.o
+
+# RUN: llvm-lipo %t.i386.o -create -o %t.noarch.o
+# RUN: not lld -flavor darwinnew -arch x86_64 -o /dev/null %t.noarch.o 2>&1 | \
+# RUN:    FileCheck %s -DFILE=%t.noarch.o
+# CHECK: error: unable to find matching architecture in [[FILE]]
+
+.text
+.global _main
+_main:
+  mov $0, %eax
+  ret

lld/test/MachO/invalid-fat-narch.s

+# REQUIRES: x86
+# RUN: yaml2obj %s -o %t.o
+# RUN: not lld -flavor darwinnew -arch x86_64 -o /dev/null %t.o 2>&1 | \
+# RUN:    FileCheck %s -DFILE=%t.o
+# CHECK: error: [[FILE]]: fat_arch struct extends beyond end of file
+
+!fat-mach-o
+FatHeader:
+  magic:           0xCAFEBABE
+  nfat_arch:       2
+FatArchs:
+Slices:

lld/test/MachO/invalid-fat-offset.s

+# REQUIRES: x86
+# RUN: yaml2obj %s -o %t.o
+# RUN: not lld -flavor darwinnew -arch x86_64 -o /dev/null %t.o 2>&1 | \
+# RUN:    FileCheck %s -DFILE=%t.o
+# CHECK: error: [[FILE]]: slice extends beyond end of file
+
+!fat-mach-o
+FatHeader:
+  magic:           0xCAFEBABE
+  nfat_arch:       2
+FatArchs:
+  - cputype:         0x01000007
+    cpusubtype:      0x00000003
+    offset:          0x0000000000001000
+    size:            0
+    align:           12
+  - cputype:         0x00000007
+    cpusubtype:      0x00000003
+    offset:          0x000000000000B000
+    size:            0
+    align:           12
+Slices: