[lld] 2d873d5 - [lld-macho]Rework error-checking in peeking at first-member in archive to avoid segfault.

[Vy Nguyen via llvm-commits]

Author: Vy Nguyen
Date: 2023-08-09T09:54:32-04:00
New Revision: 2d873d5aa4f75053a9b74610bc96d7f7e65ee3ad

URL: https://github.com/llvm/llvm-project/commit/2d873d5aa4f75053a9b74610bc96d7f7e65ee3ad
DIFF: https://github.com/llvm/llvm-project/commit/2d873d5aa4f75053a9b74610bc96d7f7e65ee3ad.diff

LOG: [lld-macho]Rework error-checking in peeking at first-member in archive to avoid segfault.

Details:
calling getMemoryBufferRef() on an empty archive can trigger segfault so the code should check before calling this.
this seems like a bug in the Archive API but that can be fixed separately.

P.S: follow up to D156468

Differential Revision: https://reviews.llvm.org/D157300

Added: 
    

Modified: 
    lld/MachO/InputFiles.cpp
    lld/test/MachO/force-load.s

Removed: 
    


################################################################################
diff  --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 15902d0e3eefd2..470b590c5a473f 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -2106,28 +2106,34 @@ ArchiveFile::ArchiveFile(std::unique_ptr<object::Archive> &&f, bool forceHidden)
       forceHidden(forceHidden) {}
 
 void ArchiveFile::addLazySymbols() {
-  Error err = Error::success();
-  Expected<MemoryBufferRef> mbOrErr =
-      this->file->child_begin(err)->getMemoryBufferRef();
+  // Avoid calling getMemoryBufferRef() on zero-symbol archive
+  // since that crashes.
+  if (file->isEmpty() || file->getNumberOfSymbols() == 0)
+    return;
 
+  Error err = Error::success();
+  auto child = file->child_begin(err);
   // Ignore the I/O error here - will be reported later.
-  if (!mbOrErr) {
-    llvm::consumeError(mbOrErr.takeError());
-  } else if (!err) {
-    if (identify_magic(mbOrErr->getBuffer()) == file_magic::macho_object) {
-      if (target->wordSize == 8)
-        compatArch = compatWithTargetArch(
-            this, reinterpret_cast<const LP64::mach_header *>(
-                      mbOrErr->getBufferStart()));
-      else
-        compatArch = compatWithTargetArch(
-            this, reinterpret_cast<const ILP32::mach_header *>(
-                      mbOrErr->getBufferStart()));
-
-      if (!compatArch)
-        return;
+  if (!err) {
+    Expected<MemoryBufferRef> mbOrErr = child->getMemoryBufferRef();
+    if (!mbOrErr) {
+      llvm::consumeError(mbOrErr.takeError());
+    } else {
+      if (identify_magic(mbOrErr->getBuffer()) == file_magic::macho_object) {
+        if (target->wordSize == 8)
+          compatArch = compatWithTargetArch(
+              this, reinterpret_cast<const LP64::mach_header *>(
+                        mbOrErr->getBufferStart()));
+        else
+          compatArch = compatWithTargetArch(
+              this, reinterpret_cast<const ILP32::mach_header *>(
+                        mbOrErr->getBufferStart()));
+        if (!compatArch)
+          return;
+      }
     }
   }
+
   for (const object::Archive::Symbol &sym : file->symbols())
     symtab->addLazyArchive(sym.getName(), this, sym);
 }

diff  --git a/lld/test/MachO/force-load.s b/lld/test/MachO/force-load.s
index 0ae13dc4d00d9e..d973a65e00e4e6 100644
--- a/lld/test/MachO/force-load.s
+++ b/lld/test/MachO/force-load.s
@@ -42,6 +42,12 @@
 # TWICE-DAG: __TEXT,archive _bar
 # TWICE-DAG: __TEXT,archive _baz
 
+## Loading an empty-archive should not crash.
+# RUN: llvm-ar --format=darwin rcs %t/libEmpty.a
+# RUN: %lld -lSystem %t/test.o -force_load %t/libEmpty.a -o %t/loadEmpty.out
+# RUN: llvm-objdump --macho --syms %t/loadEmpty.out | FileCheck %s --check-prefix=EMPTY
+# EMPTY: F __TEXT,__text _main
+
 #--- archive-foo.s
 .section __TEXT,archive
 .globl _foo, _bar