[lld] [lld-macho] Fix unchecked Error crash for thin archive missing child (PR #97169)

[Daniel Bertalan via llvm-commits]

https://github.com/BertalanD updated https://github.com/llvm/llvm-project/pull/97169

>From 059c08241e6af264242fe0c3fffe23ac19f34021 Mon Sep 17 00:00:00 2001
From: Daniel Bertalan <dani at danielbertalan.dev>
Date: Sat, 29 Jun 2024 18:47:06 +0200
Subject: [PATCH 1/3] [lld-macho] Fix unchecked Error crash for thin archive
 missing child

The repro file in #94716 contains a few thin archives without their
member object files present. I couldn't figure out why this happened
(issue in repro tarball generation?), but this caused an "unhandled
Error" crash on `LLVM_ENABLE_ABI_BREAKING_CHECKS` builds.
---
 lld/MachO/Driver.cpp | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 9dddabcf3680c..eb64e764100b2 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -349,7 +349,12 @@ static InputFile *addFile(StringRef path, LoadType loadType,
         Error e = Error::success();
         for (const object::Archive::Child &c : file->getArchive().children(e)) {
           Expected<MemoryBufferRef> mb = c.getMemoryBufferRef();
-          if (!mb || !hasObjCSection(*mb))
+          if (!mb) {
+            llvm::consumeError(mb.takeError());
+            continue;
+          }
+
+          if (!hasObjCSection(*mb))
             continue;
           if (Error e = file->fetch(c, "-ObjC"))
             error(toString(file) + ": -ObjC failed to load archive member: " +

>From dc53f8259a5075788b2643a137689919ba01459e Mon Sep 17 00:00:00 2001
From: Daniel Bertalan <dani at danielbertalan.dev>
Date: Fri, 5 Jul 2024 19:03:40 +0200
Subject: [PATCH 2/3] Add test

---
 lld/test/MachO/reproduce-thin-archive-objc.s | 22 ++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 lld/test/MachO/reproduce-thin-archive-objc.s

diff --git a/lld/test/MachO/reproduce-thin-archive-objc.s b/lld/test/MachO/reproduce-thin-archive-objc.s
new file mode 100644
index 0000000000000..665411b012b3a
--- /dev/null
+++ b/lld/test/MachO/reproduce-thin-archive-objc.s
@@ -0,0 +1,22 @@
+# REQUIRES: x86
+
+## We only save those archive members from thin archives in the repro file that are actually used
+## during linking. This means that if we just iterate members, some of them might not exist.
+## Test that that we handle missing members correctly and don't assert on an unchecked Error.
+# FIXME: Should we eagerly save all members instead?
+
+# RUN: rm -rf %t; mkdir %t
+# RUN: sed s/SYM/_main/   %s | llvm-mc -filetype=obj -triple=x86_64-apple-macos -o %t/main.o
+# RUN: sed s/SYM/_unused/ %s | llvm-mc -filetype=obj -triple=x86_64-apple-macos -o %t/unused.o
+
+# RUN: cd %t; llvm-ar rcsT unused.a unused.o
+## FIXME: Absolute paths don't end up relativized in the repro file.
+
+# RUN: %lld %t/main.o %t/unused.a -ObjC --reproduce=%t/repro.tar -o /dev/null
+# RUN: tar xf %t/repro.tar -C %t
+# RUN: cd %t/repro; %lld @response.txt
+
+.text
+.globl SYM
+SYM:
+    ret

>From 8e29cd648b91d4dff0decc7f90c2dbbb81327342 Mon Sep 17 00:00:00 2001
From: Daniel Bertalan <dani at danielbertalan.dev>
Date: Fri, 5 Jul 2024 19:09:19 +0200
Subject: [PATCH 3/3] Add explanation

---
 lld/MachO/Driver.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index eb64e764100b2..2e01d65017cc4 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -350,6 +350,8 @@ static InputFile *addFile(StringRef path, LoadType loadType,
         for (const object::Archive::Child &c : file->getArchive().children(e)) {
           Expected<MemoryBufferRef> mb = c.getMemoryBufferRef();
           if (!mb) {
+            // Thin archives from repro tarballs can contain missing members
+            // if the member was not loaded later during the initial link.
             llvm::consumeError(mb.takeError());
             continue;
           }