Finish cleaning up most of the error handling in libObject’s MachOUniversalBinary

and its clients to use the new llvm::Error model for error handling.

Changed getAsArchive() from ErrorOr<...> to Expected<...> so now all
interfaces there use the new llvm::Error model for return values.

In the two places it had if (!Parent) this is actually a program error so changed
from returning errorCodeToError(object_error::parse_failed) to calling
report_fatal_error() with a message.

In getObjectForArch() added error messages to its two llvm::Error return values
instead of returning errorCodeToError(object_error::arch_not_found) with no
error message.

For the llvm-obdump, llvm-nm and llvm-size clients since the only binary files in
Mach-O Universal Binaries that are supported are Mach-O files or archives with
Mach-O objects, updated their logic to generate an error when a slice contains
something like an ELF binary instead of ignoring it. And added a test case for
that.

The last error stuff to be cleaned up for libObject’s MachOUniversalBinary is
the use of errorOrToExpected(Archive::create(ObjBuffer)) which needs
Archive::create() to be changed from ErrorOr<...> to Expected<...> first,
which I’ll work on next.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@274079 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/Object/MachOUniversal.h b/include/llvm/Object/MachOUniversal.h
index f4c2eacf181cc725ee9118316a7538e8dca4d26f..7eb2af944f3dc58ba181f7aea64c1cce3afb8508 100644 (file)
--- a/include/llvm/Object/MachOUniversal.h
+++ b/include/llvm/Object/MachOUniversal.h
@@ -19,7 +19,6 @@
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/MachO.h"
-#include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/MachO.h"
 
 namespace llvm {
@@ -105,7 +104,7 @@ public:
 
     Expected<std::unique_ptr<MachOObjectFile>> getAsObjectFile() const;
 
-    ErrorOr<std::unique_ptr<Archive>> getAsArchive() const;
+    Expected<std::unique_ptr<Archive>> getAsArchive() const;
   };
 
   class object_iterator {

diff --git a/lib/Object/MachOUniversal.cpp b/lib/Object/MachOUniversal.cpp
index e6880587cde5ef06b6009d72395cec832b0bcb30..5c8501010c4154c6991075fe7d47e4cc3d77d46f 100644 (file)
--- a/lib/Object/MachOUniversal.cpp
+++ b/lib/Object/MachOUniversal.cpp
@@ -68,7 +68,8 @@ MachOUniversalBinary::ObjectForArch::ObjectForArch(
 Expected<std::unique_ptr<MachOObjectFile>>
 MachOUniversalBinary::ObjectForArch::getAsObjectFile() const {
   if (!Parent)
-    return errorCodeToError(object_error::parse_failed);
+    report_fatal_error("MachOUniversalBinary::ObjectForArch::getAsObjectFile() "
+                       "called when Parent is a nullptr");
 
   StringRef ParentData = Parent->getData();
   StringRef ObjectData;
@@ -81,10 +82,11 @@ MachOUniversalBinary::ObjectForArch::getAsObjectFile() const {
   return ObjectFile::createMachOObjectFile(ObjBuffer);
 }
 
-ErrorOr<std::unique_ptr<Archive>>
+Expected<std::unique_ptr<Archive>>
 MachOUniversalBinary::ObjectForArch::getAsArchive() const {
   if (!Parent)
-    return object_error::parse_failed;
+    report_fatal_error("MachOUniversalBinary::ObjectForArch::getAsArchive() "
+                       "called when Parent is a nullptr");
 
   StringRef ParentData = Parent->getData();
   StringRef ObjectData;
@@ -94,7 +96,7 @@ MachOUniversalBinary::ObjectForArch::getAsArchive() const {
     ObjectData = ParentData.substr(Header64.offset, Header64.size);
   StringRef ObjectName = Parent->getFileName();
   MemoryBufferRef ObjBuffer(ObjectData, ObjectName);
-  return Archive::create(ObjBuffer);
+  return errorOrToExpected(Archive::create(ObjBuffer));
 }
 
 void MachOUniversalBinary::anchor() { }
@@ -145,11 +147,15 @@ MachOUniversalBinary::MachOUniversalBinary(MemoryBufferRef Source, Error &Err)
 Expected<std::unique_ptr<MachOObjectFile>>
 MachOUniversalBinary::getObjectForArch(StringRef ArchName) const {
   if (Triple(ArchName).getArch() == Triple::ArchType::UnknownArch)
-    return errorCodeToError(object_error::arch_not_found);
+    return make_error<GenericBinaryError>(std::move("Unknown architecture "
+                                                    "named: " + ArchName),
+                                          object_error::arch_not_found);
 
   for (object_iterator I = begin_objects(), E = end_objects(); I != E; ++I) {
     if (I->getArchTypeName() == ArchName)
       return I->getAsObjectFile();
   }
-  return errorCodeToError(object_error::arch_not_found);
+  return make_error<GenericBinaryError>(std::move("fat file does not "
+                                                  "contain " + ArchName),
+                                        object_error::arch_not_found);
 }

diff --git a/test/Object/Inputs/macho-invalid-fat.obj.elf-x86_64 b/test/Object/Inputs/macho-invalid-fat.obj.elf-x86_64
new file mode 100644 (file)
index 0000000..4fa5afb
Binary files /dev/null and b/test/Object/Inputs/macho-invalid-fat.obj.elf-x86_64 differ

diff --git a/test/Object/macho-invalid.test b/test/Object/macho-invalid.test
index 4c66f5c402e6d7a5b9b2c484c42143ac9fa8fce9..0be522eb49b3f7859846ea109fb0cb3d67f05871 100644 (file)
--- a/test/Object/macho-invalid.test
+++ b/test/Object/macho-invalid.test
@@ -97,3 +97,6 @@ INCOMPLETE-SEGMENT-LOADC-FAT-ARCHIVE: macho-universal-archive-bad2.x86_64.i386(m
 
 RUN: not llvm-objdump -macho -universal-headers %p/Inputs/macho-invalid-fat 2>&1 | FileCheck -check-prefix INVALID-FAT %s
 INVALID-FAT: truncated or malformed fat file (fat_arch_64 structs would extend past the end of the file)
+
+RUN: not llvm-objdump -macho -private-headers -arch all %p/Inputs/macho-invalid-fat.obj.elf-x86_64 2>&1 | FileCheck -check-prefix INVALID-FAT-ELF %s
+INVALID-FAT-ELF: Mach-O universal file: {{.*}}/macho-invalid-fat.obj.elf-x86_64 for architecture x86_64 is not a Mach-O file or an archive file

diff --git a/tools/llvm-nm/llvm-nm.cpp b/tools/llvm-nm/llvm-nm.cpp
index 9661f8e2020439a211b4d5477ae8db391006d66a..2cd0ede4a5371bf490214e055b8a497c5ace9a6b 100644 (file)
--- a/tools/llvm-nm/llvm-nm.cpp
+++ b/tools/llvm-nm/llvm-nm.cpp
@@ -1171,7 +1171,7 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
               error(std::move(E), Filename, ArchFlags.size() > 1 ?
                     StringRef(I->getArchTypeName()) : StringRef());
               continue;
-            } else if (ErrorOr<std::unique_ptr<Archive>> AOrErr =
+            } else if (Expected<std::unique_ptr<Archive>> AOrErr =
                            I->getAsArchive()) {
               std::unique_ptr<Archive> &A = *AOrErr;
               for (Archive::child_iterator AI = A->child_begin(),
@@ -1209,6 +1209,12 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
                                             ArchitectureName);
                 }
               }
+            } else {
+              consumeError(AOrErr.takeError());
+              error(Filename + " for architecture " +
+                    StringRef(I->getArchTypeName()) +
+                    " is not a Mach-O file or an archive file",
+                    "Mach-O universal file");
             }
           }
         }
@@ -1238,7 +1244,7 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
                      ObjOrErr.takeError())) {
             error(std::move(E), Filename);
             return;
-          } else if (ErrorOr<std::unique_ptr<Archive>> AOrErr =
+          } else if (Expected<std::unique_ptr<Archive>> AOrErr =
                          I->getAsArchive()) {
             std::unique_ptr<Archive> &A = *AOrErr;
             for (Archive::child_iterator AI = A->child_begin(),
@@ -1266,6 +1272,12 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
                 dumpSymbolNamesFromObject(*O, false, ArchiveName);
               }
             }
+          } else {
+            consumeError(AOrErr.takeError());
+            error(Filename + " for architecture " +
+                  StringRef(I->getArchTypeName()) +
+                  " is not a Mach-O file or an archive file",
+                  "Mach-O universal file");
           }
           return;
         }
@@ -1301,7 +1313,8 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
         error(std::move(E), Filename, moreThanOneArch ?
               StringRef(I->getArchTypeName()) : StringRef());
         continue;
-      } else if (ErrorOr<std::unique_ptr<Archive>> AOrErr = I->getAsArchive()) {
+      } else if (Expected<std::unique_ptr<Archive>> AOrErr =
+                  I->getAsArchive()) {
         std::unique_ptr<Archive> &A = *AOrErr;
         for (Archive::child_iterator AI = A->child_begin(), AE = A->child_end();
              AI != AE; ++AI) {
@@ -1336,6 +1349,12 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
             dumpSymbolNamesFromObject(*O, false, ArchiveName, ArchitectureName);
           }
         }
+      } else {
+        consumeError(AOrErr.takeError());
+        error(Filename + " for architecture " +
+              StringRef(I->getArchTypeName()) +
+              " is not a Mach-O file or an archive file",
+              "Mach-O universal file");
       }
     }
     return;

diff --git a/tools/llvm-objdump/MachODump.cpp b/tools/llvm-objdump/MachODump.cpp
index b92599a02f8f477252a92e6b7f5f760a4d749bb9..82e7f8a258b6b0fab3b6071c832d05d50f47a279 100644 (file)
--- a/tools/llvm-objdump/MachODump.cpp
+++ b/tools/llvm-objdump/MachODump.cpp
@@ -1621,7 +1621,7 @@ void llvm::ParseInputMachO(StringRef Filename) {
               report_error(Filename, StringRef(), std::move(E),
                            ArchitectureName);
               continue;
-            } else if (ErrorOr<std::unique_ptr<Archive>> AOrErr =
+            } else if (Expected<std::unique_ptr<Archive>> AOrErr =
                            I->getAsArchive()) {
               std::unique_ptr<Archive> &A = *AOrErr;
               outs() << "Archive : " << Filename;
@@ -1646,6 +1646,11 @@ void llvm::ParseInputMachO(StringRef Filename) {
                         dyn_cast<MachOObjectFile>(&*ChildOrErr.get()))
                   ProcessMachO(Filename, O, O->getFileName(), ArchitectureName);
               }
+            } else {
+              consumeError(AOrErr.takeError());
+              error("Mach-O universal file: " + Filename + " for " +
+                    "architecture " + StringRef(I->getArchTypeName()) +
+                    " is not a Mach-O file or an archive file");
             }
           }
         }
@@ -1676,7 +1681,7 @@ void llvm::ParseInputMachO(StringRef Filename) {
                      ObjOrErr.takeError())) {
             report_error(Filename, std::move(E));
             continue;
-          } else if (ErrorOr<std::unique_ptr<Archive>> AOrErr =
+          } else if (Expected<std::unique_ptr<Archive>> AOrErr =
                          I->getAsArchive()) {
             std::unique_ptr<Archive> &A = *AOrErr;
             outs() << "Archive : " << Filename << "\n";
@@ -1698,6 +1703,11 @@ void llvm::ParseInputMachO(StringRef Filename) {
                       dyn_cast<MachOObjectFile>(&*ChildOrErr.get()))
                 ProcessMachO(Filename, O, O->getFileName());
             }
+          } else {
+            consumeError(AOrErr.takeError());
+            error("Mach-O universal file: " + Filename + " for architecture " +
+                  StringRef(I->getArchTypeName()) +
+                  " is not a Mach-O file or an archive file");
           }
           return;
         }
@@ -1721,7 +1731,8 @@ void llvm::ParseInputMachO(StringRef Filename) {
                  ObjOrErr.takeError())) {
         report_error(StringRef(), Filename, std::move(E), ArchitectureName);
         continue;
-      } else if (ErrorOr<std::unique_ptr<Archive>> AOrErr = I->getAsArchive()) {
+      } else if (Expected<std::unique_ptr<Archive>> AOrErr =
+                   I->getAsArchive()) {
         std::unique_ptr<Archive> &A = *AOrErr;
         outs() << "Archive : " << Filename;
         if (!ArchitectureName.empty())
@@ -1747,6 +1758,11 @@ void llvm::ParseInputMachO(StringRef Filename) {
                            ArchitectureName);
           }
         }
+      } else {
+        consumeError(AOrErr.takeError());
+        error("Mach-O universal file: " + Filename + " for architecture " +
+              StringRef(I->getArchTypeName()) +
+              " is not a Mach-O file or an archive file");
       }
     }
     return;

diff --git a/tools/llvm-objdump/llvm-objdump.cpp b/tools/llvm-objdump/llvm-objdump.cpp
index 3c536af5713952482877a29f55f5be022b7f8572..bb171ec3fc3183763994ea851daadaac80d262ec 100644 (file)
--- a/tools/llvm-objdump/llvm-objdump.cpp
+++ b/tools/llvm-objdump/llvm-objdump.cpp
@@ -264,6 +264,12 @@ void llvm::error(std::error_code EC) {
   exit(1);
 }
 
+LLVM_ATTRIBUTE_NORETURN void llvm::error(Twine Message) {
+  errs() << ToolName << ": " << Message << ".\n";
+  errs().flush();
+  exit(1);
+}
+
 LLVM_ATTRIBUTE_NORETURN void llvm::report_error(StringRef File,
                                                 std::error_code EC) {
   assert(EC);

diff --git a/tools/llvm-objdump/llvm-objdump.h b/tools/llvm-objdump/llvm-objdump.h
index 439018963cb7163b1c0d7911a95696f380ed3451..5b10ee87ca86381e9d71e33fddeab086648b5e0e 100644 (file)
--- a/tools/llvm-objdump/llvm-objdump.h
+++ b/tools/llvm-objdump/llvm-objdump.h
@@ -88,6 +88,7 @@ void PrintSectionHeaders(const object::ObjectFile *o);
 void PrintSectionContents(const object::ObjectFile *o);
 void PrintSymbolTable(const object::ObjectFile *o, StringRef ArchiveName,
                       StringRef ArchitectureName = StringRef());
+LLVM_ATTRIBUTE_NORETURN void error(Twine Message);
 LLVM_ATTRIBUTE_NORETURN void report_error(StringRef File, std::error_code EC);
 LLVM_ATTRIBUTE_NORETURN void report_error(StringRef File, llvm::Error E);
 LLVM_ATTRIBUTE_NORETURN void report_error(StringRef FileName,

diff --git a/tools/llvm-readobj/llvm-readobj.cpp b/tools/llvm-readobj/llvm-readobj.cpp
index 06d5e058edf94ed3c667b47adcaf06c3a2326caf..ad97dea2d0219646d8d8da6cf891814ea368d706 100644 (file)
--- a/tools/llvm-readobj/llvm-readobj.cpp
+++ b/tools/llvm-readobj/llvm-readobj.cpp
@@ -459,7 +459,7 @@ static void dumpMachOUniversalBinary(const MachOUniversalBinary *UBinary) {
       OS.flush();
       reportError(UBinary->getFileName(), Buf);
     }
-    else if (ErrorOr<std::unique_ptr<Archive>> AOrErr = Obj.getAsArchive())
+    else if (Expected<std::unique_ptr<Archive>> AOrErr = Obj.getAsArchive())
       dumpArchive(&*AOrErr.get());
   }
 }

diff --git a/tools/llvm-size/llvm-size.cpp b/tools/llvm-size/llvm-size.cpp
index 544eb104f5f1810d8b16b42f0d17d881418db5eb..ecc0a0eac3cac212a6fbeee27ec113122a11fc07 100644 (file)
--- a/tools/llvm-size/llvm-size.cpp
+++ b/tools/llvm-size/llvm-size.cpp
@@ -99,6 +99,13 @@ static bool error(std::error_code ec) {
   return true;
 }
 
+static bool error(Twine Message) {
+  HadError = true;
+  errs() << ToolName << ": " << Message << ".\n";
+  errs().flush();
+  return true;
+}
+
 // This version of error() prints the archive name and member name, for example:
 // "libx.a(foo.o)" after the ToolName before the error message.  It sets
 // HadError but returns allowing the code to move on to other archive members. 
@@ -585,7 +592,7 @@ static void printFileSectionSizes(StringRef file) {
               error(std::move(E), file, ArchFlags.size() > 1 ?
                     StringRef(I->getArchTypeName()) : StringRef());
               return;
-            } else if (ErrorOr<std::unique_ptr<Archive>> AOrErr =
+            } else if (Expected<std::unique_ptr<Archive>> AOrErr =
                            I->getAsArchive()) {
               std::unique_ptr<Archive> &UA = *AOrErr;
               // This is an archive. Iterate over each member and display its
@@ -630,6 +637,11 @@ static void printFileSectionSizes(StringRef file) {
                   }
                 }
               }
+            } else {
+              consumeError(AOrErr.takeError());
+              error("Mach-O universal file: " + file + " for architecture " +
+                    StringRef(I->getArchTypeName()) +
+                    " is not a Mach-O file or an archive file");
             }
           }
         }
@@ -671,7 +683,7 @@ static void printFileSectionSizes(StringRef file) {
           } else if (auto E = isNotObjectErrorInvalidFileType(UO.takeError())) {
             error(std::move(E), file);
             return;
-          } else if (ErrorOr<std::unique_ptr<Archive>> AOrErr =
+          } else if (Expected<std::unique_ptr<Archive>> AOrErr =
                          I->getAsArchive()) {
             std::unique_ptr<Archive> &UA = *AOrErr;
             // This is an archive. Iterate over each member and display its
@@ -709,6 +721,11 @@ static void printFileSectionSizes(StringRef file) {
                 }
               }
             }
+          } else {
+            consumeError(AOrErr.takeError());
+            error("Mach-O universal file: " + file + " for architecture " +
+                   StringRef(I->getArchTypeName()) +
+                   " is not a Mach-O file or an archive file");
           }
           return;
         }
@@ -744,7 +761,7 @@ static void printFileSectionSizes(StringRef file) {
         error(std::move(E), file, MoreThanOneArch ?
               StringRef(I->getArchTypeName()) : StringRef());
         return;
-      } else if (ErrorOr<std::unique_ptr<Archive>> AOrErr =
+      } else if (Expected<std::unique_ptr<Archive>> AOrErr =
                          I->getAsArchive()) {
         std::unique_ptr<Archive> &UA = *AOrErr;
         // This is an archive. Iterate over each member and display its sizes.
@@ -781,6 +798,11 @@ static void printFileSectionSizes(StringRef file) {
             }
           }
         }
+      } else {
+        consumeError(AOrErr.takeError());
+        error("Mach-O universal file: " + file + " for architecture " +
+               StringRef(I->getArchTypeName()) +
+               " is not a Mach-O file or an archive file");
       }
     }
   } else if (ObjectFile *o = dyn_cast<ObjectFile>(&Bin)) {