Change llvm-objdump, llvm-nm and llvm-size when reporting an object file error

when the object is in an archive to use something like libx.a(foo.o) as part of
the error message.

Also changed llvm-objdump and llvm-size to be like llvm-nm and ignore non-object
files in archives and not produce any error message.

To do this Archive::Child::getAsBinary() was changed from ErrorOr<...> to
Expected<...> then that was threaded up to its users.

Converting this interface to Expected<> from ErrorOr<> does involve
touching a number of places. To contain the changes for now the use of
errorToErrorCode() is still used in one place yet to be fully converted.

Again 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 comments for those.

llvm-svn: 269784

Author: enderby

llvm/include/llvm/Object/Archive.h

@@ -101,7 +101,7 @@ class Archive : public Binary {
 
     ErrorOr<MemoryBufferRef> getMemoryBufferRef() const;
 
-    ErrorOr<std::unique_ptr<Binary>>
+    Expected<std::unique_ptr<Binary>>
     getAsBinary(LLVMContext *Context = nullptr) const;
   };
 

llvm/include/llvm/Object/Error.h

@@ -81,4 +81,34 @@ template <>
 struct is_error_code_enum<llvm::object::object_error> : std::true_type {};
 }
 
+namespace llvm {
+namespace object {
+
+// isNotObjectErrorInvalidFileType() is used when looping through the children
+// of an archive after calling getAsBinary() on the child and it returns an
+// llvm::Error.  In the cases we want to loop through the children and ignore the
+// non-objects in the archive this is used to test the error to see if an
+// error() function needs to called on the llvm::Error.
+static inline llvm::Error isNotObjectErrorInvalidFileType(llvm::Error Err) {
+  if (auto Err2 =
+       handleErrors(std::move(Err),
+         [](std::unique_ptr<ECError> M) {
+           // Try to handle 'M'. If successful, return a success value from
+           // the handler.
+           if (M->convertToErrorCode() == object_error::invalid_file_type)
+             return Error::success();
+
+           // We failed to handle 'M' - return it from the handler.
+           // This value will be passed back from catchErrors and
+           // wind up in Err2, where it will be returned from this function.
+           return Error(std::move(M));
+         }))
+    return Err2;
+  return Err;
+}
+
+} // end namespace object.
+
+} // end namespace llvm.
+
 #endif

llvm/lib/ExecutionEngine/MCJIT/MCJIT.cpp

@@ -329,10 +329,13 @@ RuntimeDyld::SymbolInfo MCJIT::findSymbol(const std::string &Name,
       report_fatal_error(EC.message());
     if (ChildIt != A->child_end()) {
       // FIXME: Support nested archives?
-      ErrorOr<std::unique_ptr<object::Binary>> ChildBinOrErr =
+      Expected<std::unique_ptr<object::Binary>> ChildBinOrErr =
           (*ChildIt)->getAsBinary();
-      if (ChildBinOrErr.getError())
+      if (!ChildBinOrErr) {
+        // TODO: Actually report errors helpfully.
+        consumeError(ChildBinOrErr.takeError());
         continue;
+      }
       std::unique_ptr<object::Binary> &ChildBin = ChildBinOrErr.get();
       if (ChildBin->isObject()) {
         std::unique_ptr<object::ObjectFile> OF(

llvm/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h

@@ -263,10 +263,13 @@ class OrcMCJITReplacement : public ExecutionEngine {
         report_fatal_error(EC.message());
       if (ChildIt != A->child_end()) {
         // FIXME: Support nested archives?
-        ErrorOr<std::unique_ptr<object::Binary>> ChildBinOrErr =
+        Expected<std::unique_ptr<object::Binary>> ChildBinOrErr =
             (*ChildIt)->getAsBinary();
-        if (ChildBinOrErr.getError())
+        if (!ChildBinOrErr) {
+          // TODO: Actually report errors helpfully.
+          consumeError(ChildBinOrErr.takeError());
           continue;
+        }
         std::unique_ptr<object::Binary> &ChildBin = ChildBinOrErr.get();
         if (ChildBin->isObject()) {
           std::vector<std::unique_ptr<object::ObjectFile>> ObjSet;

llvm/lib/Object/Archive.cpp

@@ -228,6 +228,11 @@ ErrorOr<StringRef> Archive::Child::getName() const {
     if (name.substr(3).rtrim(' ').getAsInteger(10, name_size))
       llvm_unreachable("Long name length is not an ingeter");
     return Data.substr(sizeof(ArchiveMemberHeader), name_size).rtrim('\0');
+  } else {
+    // It is not a long name so trim the blanks at the end of the name.
+    if (name[name.size() - 1] != '/') {
+      return name.rtrim(' ');
+    }
   }
   // It's a simple name.
   if (name[name.size() - 1] == '/')
@@ -246,16 +251,16 @@ ErrorOr<MemoryBufferRef> Archive::Child::getMemoryBufferRef() const {
   return MemoryBufferRef(*Buf, Name);
 }
 
-ErrorOr<std::unique_ptr<Binary>>
+Expected<std::unique_ptr<Binary>>
 Archive::Child::getAsBinary(LLVMContext *Context) const {
   ErrorOr<MemoryBufferRef> BuffOrErr = getMemoryBufferRef();
   if (std::error_code EC = BuffOrErr.getError())
-    return EC;
+    return errorCodeToError(EC);
 
   auto BinaryOrErr = createBinary(BuffOrErr.get(), Context);
   if (BinaryOrErr)
     return std::move(*BinaryOrErr);
-  return errorToErrorCode(BinaryOrErr.takeError());
+  return BinaryOrErr.takeError();
 }
 
 ErrorOr<std::unique_ptr<Archive>> Archive::create(MemoryBufferRef Source) {

llvm/test/Object/Inputs/macho-bad-archive1.a

@@ -56,6 +56,10 @@ RUN: llvm-nm -pax %p/Inputs/macho-invalid-symbol-name-past-eof 2>&1 \
 RUN:      | FileCheck -check-prefix NAME-PAST-EOF-nm-pax %s
 NAME-PAST-EOF-nm-pax: 0000000000000000 64 00 0000 fe000002 bad string index
 
+RUN: not llvm-objdump -t %p/Inputs/macho-bad-archive1.a 2>&1 \
+RUN:      | FileCheck -check-prefix NAME-PAST-EOF-ARCHIVE %s
+NAME-PAST-EOF-ARCHIVE: macho-bad-archive1.a(macho-invalid-symbol-name-past-eof) truncated or malformed object (bad string index: 4261412866 for symbol at index 0)
+
 RUN: llvm-nm %p/Inputs/macho-invalid-section-index-getSectionRawName 2>&1 \
 RUN:      | FileCheck -check-prefix INVALID-SECTION-IDX-SYMBOL-SEC %s
 INVALID-SECTION-IDX-SYMBOL-SEC: 0000000100000000 S __mh_execute_header
@@ -74,3 +78,6 @@ INVALID-HEADER: The file was not recognized as a valid object file
 
 RUN: not llvm-objdump -macho -private-headers %p/Inputs/macho64-invalid-incomplete-segment-load-command 2>&1 | FileCheck -check-prefix INCOMPLETE-SEGMENT-LOADC %s
 INCOMPLETE-SEGMENT-LOADC: truncated or malformed object (load commands extend past the end of the file)
+
+RUN: not llvm-objdump -macho -private-headers %p/Inputs/macho-bad-archive2.a 2>&1 | FileCheck -check-prefix INCOMPLETE-SEGMENT-LOADC-ARCHIVE %s
+INCOMPLETE-SEGMENT-LOADC-ARCHIVE: macho-bad-archive2.a(macho64-invalid-incomplete-segment-load-command) truncated or malformed object (load commands extend past the end of the file)

llvm/test/Object/Inputs/macho-bad-archive2.a

@@ -55,9 +55,10 @@ RUN: llvm-nm %p/Inputs/archive-test.a-empty
 
 This archive has an unaligned member and a unknown format member.
 GNU AR is able to parse the unaligned member and warns about the member with
-the unknown format. We should probably simply warn on both. For now just check
-that we don't produce an error.
-RUN: llvm-nm %p/Inputs/corrupt-archive.a
+the unknown format. We should probably simply warn on both. For now we just
+produce an error for the unknown format.
+RUN: not llvm-nm %p/Inputs/corrupt-archive.a 2>&1 | FileCheck %s -check-prefix CORRUPT
+CORRUPT: corrupt-archive.a(trivial-object-test2.elf-x86-64) Invalid data was encountered while parsing the file
 
 
 RUN: llvm-nm %p/Inputs/thin.a | FileCheck %s -check-prefix THIN

llvm/test/Object/macho-invalid.test

@@ -485,11 +485,17 @@ static void dumpArchive(const Archive *Arc) {
   for (auto &ErrorOrChild : Arc->children()) {
     error(ErrorOrChild.getError());
     const Archive::Child &ArcC = *ErrorOrChild;
-    ErrorOr<std::unique_ptr<Binary>> ChildOrErr = ArcC.getAsBinary();
-    if (std::error_code EC = ChildOrErr.getError()) {
+    Expected<std::unique_ptr<Binary>> ChildOrErr = ArcC.getAsBinary();
+    if (!ChildOrErr) {
       // Ignore non-object files.
-      if (EC != object_error::invalid_file_type)
-        reportError(Arc->getFileName(), EC.message());
+      if (auto E = isNotObjectErrorInvalidFileType(ChildOrErr.takeError())) {
+        std::string Buf;
+        raw_string_ostream OS(Buf);
+        logAllUnhandledErrors(std::move(E), OS, "");
+        OS.flush();
+        reportError(Arc->getFileName(), Buf);
+      }
+      ChildOrErr.takeError();
       continue;
     }