[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

7 ↗

(On Diff #260840)

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

7 ↗

(On Diff #260840)

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 (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 + ": broken fat file");
+    return MemoryBufferRef(StringRef(buf + offset, size), path);
+  }
+
+  warn("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/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 %t %t.fat.o
+
+# RUN: llvm-lipo %t.i386.o -create -o %t.noarch.o
+# RUN: not lld -flavor darwinnew -arch x86_64 -o %t %t.noarch.o 2>&1 | \
+# RUN:    FileCheck %s -DFILE=%t.noarch.o
+# CHECK: Unable to find matching architecture in [[FILE]]
+# CHECK: undefined symbol: _main
+
+.text
+.global _main
+_main:
+  mov $0, %eax
+  ret