Thread Expected<...> up from createMachOObjectFile() to allow llvm-objdump to produce a real error message

Produce the first specific error message for a malformed Mach-O file describing
the problem instead of the generic message for object_error::parse_failed of
"Invalid data was encountered while parsing the file”.  Many more good error
messages will follow after this first one.

This is built on Lang Hames’ great work of adding the ’Error' class for
structured error handling and threading Error through MachOObjectFile
construction.  And making createMachOObjectFile return Expected<...> .

So to to get the error to the llvm-obdump tool, I changed the stack of
these methods to also return Expected<...> :

  object::ObjectFile::createObjectFile()
  object::SymbolicFile::createSymbolicFile()
  object::createBinary()

Then finally in ParseInputMachO() in MachODump.cpp the error can
be reported and the specific error message can be printed in llvm-objdump
and can be seen in the existing test case for the existing malformed binary
but with the updated error message.

Converting these interfaces to Expected<> from ErrorOr<> does involve
touching a number of places. To contain the changes for now use of
errorToErrorCode() and errorOrToExpected() are used where the callers
are yet to be converted.

Also there some were bugs in the existing code that did not deal with the
old ErrorOr<> return values.  So now with Expected<> since they must be
checked and the error handled, I added a TODO and a comment:
“// TODO: Actually report errors helpfully” and a call something like
consumeError(ObjOrErr.takeError()) so the buggy code will not crash
since needed to deal with the Error.

Note there is one fix also needed to lld/COFF/InputFiles.cpp that goes along
with this that I will commit right after this.  So expect lld not to built
after this commit and before the next one.

llvm-svn: 265606

9 files changed, 49 insertions, 36 deletions

diff --git a/llvm/lib/Object/Archive.cpp b/llvm/lib/Object/Archive.cpp
index 9c8e3dbe2bd..8a3b34b0158 100644
--- a/llvm/lib/Object/Archive.cpp
+++ b/llvm/lib/Object/Archive.cpp
@@ -245,7 +245,10 @@ Archive::Child::getAsBinary(LLVMContext *Context) const {
   if (std::error_code EC = BuffOrErr.getError())
     return EC;
 
-  return createBinary(BuffOrErr.get(), Context);
+  auto BinaryOrErr = createBinary(BuffOrErr.get(), Context);
+  if (BinaryOrErr)
+    return std::move(*BinaryOrErr);
+  return errorToErrorCode(BinaryOrErr.takeError());
 }
 
 ErrorOr<std::unique_ptr<Archive>> Archive::create(MemoryBufferRef Source) {
diff --git a/llvm/lib/Object/ArchiveWriter.cpp b/llvm/lib/Object/ArchiveWriter.cpp
index c7343fdc171..a1be2fc760c 100644
--- a/llvm/lib/Object/ArchiveWriter.cpp
+++ b/llvm/lib/Object/ArchiveWriter.cpp
@@ -231,11 +231,14 @@ writeSymbolTable(raw_fd_ostream &Out, object::Archive::Kind Kind,
   LLVMContext Context;
   for (unsigned MemberNum = 0, N = Members.size(); MemberNum < N; ++MemberNum) {
     MemoryBufferRef MemberBuffer = Buffers[MemberNum];
-    ErrorOr<std::unique_ptr<object::SymbolicFile>> ObjOrErr =
+    Expected<std::unique_ptr<object::SymbolicFile>> ObjOrErr =
         object::SymbolicFile::createSymbolicFile(
             MemberBuffer, sys::fs::file_magic::unknown, &Context);
-    if (!ObjOrErr)
-      continue;  // FIXME: check only for "not an object file" errors.
+    if (!ObjOrErr) {
+      // FIXME: check only for "not an object file" errors.
+      consumeError(ObjOrErr.takeError());
+      continue;
+    }
     object::SymbolicFile &Obj = *ObjOrErr.get();
 
     if (!HeaderStartOffset) {
diff --git a/llvm/lib/Object/Binary.cpp b/llvm/lib/Object/Binary.cpp
index a2b167a665c..723b279d9e6 100644
--- a/llvm/lib/Object/Binary.cpp
+++ b/llvm/lib/Object/Binary.cpp
@@ -36,13 +36,13 @@ StringRef Binary::getFileName() const { return Data.getBufferIdentifier(); }
 
 MemoryBufferRef Binary::getMemoryBufferRef() const { return Data; }
 
-ErrorOr<std::unique_ptr<Binary>> object::createBinary(MemoryBufferRef Buffer,
+Expected<std::unique_ptr<Binary>> object::createBinary(MemoryBufferRef Buffer,
                                                       LLVMContext *Context) {
   sys::fs::file_magic Type = sys::fs::identify_magic(Buffer.getBuffer());
 
   switch (Type) {
     case sys::fs::file_magic::archive:
-      return Archive::create(Buffer);
+      return errorOrToExpected(Archive::create(Buffer));
     case sys::fs::file_magic::elf:
     case sys::fs::file_magic::elf_relocatable:
     case sys::fs::file_magic::elf_executable:
@@ -65,26 +65,26 @@ ErrorOr<std::unique_ptr<Binary>> object::createBinary(MemoryBufferRef Buffer,
     case sys::fs::file_magic::bitcode:
       return ObjectFile::createSymbolicFile(Buffer, Type, Context);
     case sys::fs::file_magic::macho_universal_binary:
-      return MachOUniversalBinary::create(Buffer);
+      return errorOrToExpected(MachOUniversalBinary::create(Buffer));
     case sys::fs::file_magic::unknown:
     case sys::fs::file_magic::windows_resource:
       // Unrecognized object file format.
-      return object_error::invalid_file_type;
+      return errorCodeToError(object_error::invalid_file_type);
   }
   llvm_unreachable("Unexpected Binary File Type");
 }
 
-ErrorOr<OwningBinary<Binary>> object::createBinary(StringRef Path) {
+Expected<OwningBinary<Binary>> object::createBinary(StringRef Path) {
   ErrorOr<std::unique_ptr<MemoryBuffer>> FileOrErr =
       MemoryBuffer::getFileOrSTDIN(Path);
   if (std::error_code EC = FileOrErr.getError())
-    return EC;
+    return errorCodeToError(EC);
   std::unique_ptr<MemoryBuffer> &Buffer = FileOrErr.get();
 
-  ErrorOr<std::unique_ptr<Binary>> BinOrErr =
+  Expected<std::unique_ptr<Binary>> BinOrErr =
       createBinary(Buffer->getMemBufferRef());
-  if (std::error_code EC = BinOrErr.getError())
-    return EC;
+  if (!BinOrErr)
+    return BinOrErr.takeError();
   std::unique_ptr<Binary> &Bin = BinOrErr.get();
 
   return OwningBinary<Binary>(std::move(Bin), std::move(Buffer));
diff --git a/llvm/lib/Object/IRObjectFile.cpp b/llvm/lib/Object/IRObjectFile.cpp
index 49737b5f6f9..a4b97efc645 100644
--- a/llvm/lib/Object/IRObjectFile.cpp
+++ b/llvm/lib/Object/IRObjectFile.cpp
@@ -285,10 +285,10 @@ ErrorOr<MemoryBufferRef> IRObjectFile::findBitcodeInMemBuffer(MemoryBufferRef Ob
   case sys::fs::file_magic::elf_relocatable:
   case sys::fs::file_magic::macho_object:
   case sys::fs::file_magic::coff_object: {
-    ErrorOr<std::unique_ptr<ObjectFile>> ObjFile =
+    Expected<std::unique_ptr<ObjectFile>> ObjFile =
         ObjectFile::createObjectFile(Object, Type);
     if (!ObjFile)
-      return ObjFile.getError();
+      return errorToErrorCode(ObjFile.takeError());
     return findBitcodeInObject(*ObjFile->get());
   }
   default:
diff --git a/llvm/lib/Object/MachOObjectFile.cpp b/llvm/lib/Object/MachOObjectFile.cpp
index d9d19fa1e8e..c6643cb8c03 100644
--- a/llvm/lib/Object/MachOObjectFile.cpp
+++ b/llvm/lib/Object/MachOObjectFile.cpp
@@ -192,6 +192,10 @@ static Expected<MachOObjectFile::LoadCommandInfo>
 getFirstLoadCommandInfo(const MachOObjectFile *Obj) {
   unsigned HeaderSize = Obj->is64Bit() ? sizeof(MachO::mach_header_64)
                                        : sizeof(MachO::mach_header);
+  if (sizeof(MachOObjectFile::LoadCommandInfo) > Obj->getHeader().sizeofcmds)
+    return malformedError(*Obj, "truncated or malformed object (load command "
+                          "0 extends past the end all load commands in the "
+                          "file)");
   return getLoadCommandInfo(Obj, getPtr(Obj, HeaderSize));
 }
 
diff --git a/llvm/lib/Object/ModuleSummaryIndexObjectFile.cpp b/llvm/lib/Object/ModuleSummaryIndexObjectFile.cpp
index 53d3f8e61b5..ca45144f05f 100644
--- a/llvm/lib/Object/ModuleSummaryIndexObjectFile.cpp
+++ b/llvm/lib/Object/ModuleSummaryIndexObjectFile.cpp
@@ -56,10 +56,10 @@ ModuleSummaryIndexObjectFile::findBitcodeInMemBuffer(MemoryBufferRef Object) {
   case sys::fs::file_magic::elf_relocatable:
   case sys::fs::file_magic::macho_object:
   case sys::fs::file_magic::coff_object: {
-    ErrorOr<std::unique_ptr<ObjectFile>> ObjFile =
+    Expected<std::unique_ptr<ObjectFile>> ObjFile =
         ObjectFile::createObjectFile(Object, Type);
     if (!ObjFile)
-      return ObjFile.getError();
+      return errorToErrorCode(ObjFile.takeError());
     return findBitcodeInObject(*ObjFile->get());
   }
   default:
diff --git a/llvm/lib/Object/Object.cpp b/llvm/lib/Object/Object.cpp
index b44c1a16fd0..4932f233f17 100644
--- a/llvm/lib/Object/Object.cpp
+++ b/llvm/lib/Object/Object.cpp
@@ -61,11 +61,14 @@ wrap(const relocation_iterator *SI) {
 // ObjectFile creation
 LLVMObjectFileRef LLVMCreateObjectFile(LLVMMemoryBufferRef MemBuf) {
   std::unique_ptr<MemoryBuffer> Buf(unwrap(MemBuf));
-  ErrorOr<std::unique_ptr<ObjectFile>> ObjOrErr(
+  Expected<std::unique_ptr<ObjectFile>> ObjOrErr(
       ObjectFile::createObjectFile(Buf->getMemBufferRef()));
   std::unique_ptr<ObjectFile> Obj;
-  if (!ObjOrErr)
+  if (!ObjOrErr) {
+    // TODO: Actually report errors helpfully.
+    consumeError(ObjOrErr.takeError());
     return nullptr;
+  }
 
   auto *Ret = new OwningBinary<ObjectFile>(std::move(ObjOrErr.get()), std::move(Buf));
   return wrap(Ret);
diff --git a/llvm/lib/Object/ObjectFile.cpp b/llvm/lib/Object/ObjectFile.cpp
index ceb933f4341..31ec22bd2e9 100644
--- a/llvm/lib/Object/ObjectFile.cpp
+++ b/llvm/lib/Object/ObjectFile.cpp
@@ -66,7 +66,7 @@ section_iterator ObjectFile::getRelocatedSection(DataRefImpl Sec) const {
   return section_iterator(SectionRef(Sec, this));
 }
 
-ErrorOr<std::unique_ptr<ObjectFile>>
+Expected<std::unique_ptr<ObjectFile>>
 ObjectFile::createObjectFile(MemoryBufferRef Object, sys::fs::file_magic Type) {
   StringRef Data = Object.getBuffer();
   if (Type == sys::fs::file_magic::unknown)
@@ -78,13 +78,13 @@ ObjectFile::createObjectFile(MemoryBufferRef Object, sys::fs::file_magic Type) {
   case sys::fs::file_magic::archive:
   case sys::fs::file_magic::macho_universal_binary:
   case sys::fs::file_magic::windows_resource:
-    return object_error::invalid_file_type;
+    return errorCodeToError(object_error::invalid_file_type);
   case sys::fs::file_magic::elf:
   case sys::fs::file_magic::elf_relocatable:
   case sys::fs::file_magic::elf_executable:
   case sys::fs::file_magic::elf_shared_object:
   case sys::fs::file_magic::elf_core:
-    return createELFObjectFile(Object);
+    return errorOrToExpected(createELFObjectFile(Object));
   case sys::fs::file_magic::macho_object:
   case sys::fs::file_magic::macho_executable:
   case sys::fs::file_magic::macho_fixed_virtual_memory_shared_lib:
@@ -96,27 +96,27 @@ ObjectFile::createObjectFile(MemoryBufferRef Object, sys::fs::file_magic Type) {
   case sys::fs::file_magic::macho_dynamically_linked_shared_lib_stub:
   case sys::fs::file_magic::macho_dsym_companion:
   case sys::fs::file_magic::macho_kext_bundle:
-    return expectedToErrorOr(createMachOObjectFile(Object));
+    return createMachOObjectFile(Object);
   case sys::fs::file_magic::coff_object:
   case sys::fs::file_magic::coff_import_library:
   case sys::fs::file_magic::pecoff_executable:
-    return createCOFFObjectFile(Object);
+    return errorOrToExpected(createCOFFObjectFile(Object));
   }
   llvm_unreachable("Unexpected Object File Type");
 }
 
-ErrorOr<OwningBinary<ObjectFile>>
+Expected<OwningBinary<ObjectFile>>
 ObjectFile::createObjectFile(StringRef ObjectPath) {
   ErrorOr<std::unique_ptr<MemoryBuffer>> FileOrErr =
       MemoryBuffer::getFile(ObjectPath);
   if (std::error_code EC = FileOrErr.getError())
-    return EC;
+    return errorCodeToError(EC);
   std::unique_ptr<MemoryBuffer> Buffer = std::move(FileOrErr.get());
 
-  ErrorOr<std::unique_ptr<ObjectFile>> ObjOrErr =
+  Expected<std::unique_ptr<ObjectFile>> ObjOrErr =
       createObjectFile(Buffer->getMemBufferRef());
-  if (std::error_code EC = ObjOrErr.getError())
-    return EC;
+  if (!ObjOrErr)
+    ObjOrErr.takeError();
   std::unique_ptr<ObjectFile> Obj = std::move(ObjOrErr.get());
 
   return OwningBinary<ObjectFile>(std::move(Obj), std::move(Buffer));
diff --git a/llvm/lib/Object/SymbolicFile.cpp b/llvm/lib/Object/SymbolicFile.cpp
index bf79dfb8da6..1e8e31b6b22 100644
--- a/llvm/lib/Object/SymbolicFile.cpp
+++ b/llvm/lib/Object/SymbolicFile.cpp
@@ -26,7 +26,7 @@ SymbolicFile::SymbolicFile(unsigned int Type, MemoryBufferRef Source)
 
 SymbolicFile::~SymbolicFile() {}
 
-ErrorOr<std::unique_ptr<SymbolicFile>> SymbolicFile::createSymbolicFile(
+Expected<std::unique_ptr<SymbolicFile>> SymbolicFile::createSymbolicFile(
     MemoryBufferRef Object, sys::fs::file_magic Type, LLVMContext *Context) {
   StringRef Data = Object.getBuffer();
   if (Type == sys::fs::file_magic::unknown)
@@ -35,13 +35,13 @@ ErrorOr<std::unique_ptr<SymbolicFile>> SymbolicFile::createSymbolicFile(
   switch (Type) {
   case sys::fs::file_magic::bitcode:
     if (Context)
-      return IRObjectFile::create(Object, *Context);
+      return errorOrToExpected(IRObjectFile::create(Object, *Context));
   // Fallthrough
   case sys::fs::file_magic::unknown:
   case sys::fs::file_magic::archive:
   case sys::fs::file_magic::macho_universal_binary:
   case sys::fs::file_magic::windows_resource:
-    return object_error::invalid_file_type;
+    return errorCodeToError(object_error::invalid_file_type);
   case sys::fs::file_magic::elf:
   case sys::fs::file_magic::elf_executable:
   case sys::fs::file_magic::elf_shared_object:
@@ -63,7 +63,7 @@ ErrorOr<std::unique_ptr<SymbolicFile>> SymbolicFile::createSymbolicFile(
   case sys::fs::file_magic::elf_relocatable:
   case sys::fs::file_magic::macho_object:
   case sys::fs::file_magic::coff_object: {
-    ErrorOr<std::unique_ptr<ObjectFile>> Obj =
+    Expected<std::unique_ptr<ObjectFile>> Obj =
         ObjectFile::createObjectFile(Object, Type);
     if (!Obj || !Context)
       return std::move(Obj);
@@ -73,9 +73,9 @@ ErrorOr<std::unique_ptr<SymbolicFile>> SymbolicFile::createSymbolicFile(
     if (!BCData)
       return std::move(Obj);
 
-    return IRObjectFile::create(
-        MemoryBufferRef(BCData->getBuffer(), Object.getBufferIdentifier()),
-        *Context);
+    return errorOrToExpected(IRObjectFile::create(
+                                 MemoryBufferRef(BCData->getBuffer(),
+                                 Object.getBufferIdentifier()), *Context));
   }
   }
   llvm_unreachable("Unexpected Binary File Type");