From c827f2cd7686fe64a8662a5f9e73c24ce63ca594 Mon Sep 17 00:00:00 2001 From: Kevin Enderby Date: Tue, 28 Jun 2016 23:16:13 +0000 Subject: [PATCH] =?utf8?q?Finish=20cleaning=20up=20most=20of=20the=20error?= =?utf8?q?=20handling=20in=20libObject=E2=80=99s=20MachOUniversalBinary=20?= =?utf8?q?and=20its=20clients=20to=20use=20the=20new=20llvm::Error=20model?= =?utf8?q?=20for=20error=20handling.?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 --- include/llvm/Object/MachOUniversal.h | 3 +- lib/Object/MachOUniversal.cpp | 18 +++++++---- .../Inputs/macho-invalid-fat.obj.elf-x86_64 | Bin 0 -> 1516 bytes test/Object/macho-invalid.test | 3 ++ tools/llvm-nm/llvm-nm.cpp | 25 ++++++++++++++-- tools/llvm-objdump/MachODump.cpp | 22 ++++++++++++-- tools/llvm-objdump/llvm-objdump.cpp | 6 ++++ tools/llvm-objdump/llvm-objdump.h | 1 + tools/llvm-readobj/llvm-readobj.cpp | 2 +- tools/llvm-size/llvm-size.cpp | 28 ++++++++++++++++-- 10 files changed, 90 insertions(+), 18 deletions(-) create mode 100644 test/Object/Inputs/macho-invalid-fat.obj.elf-x86_64 diff --git a/include/llvm/Object/MachOUniversal.h b/include/llvm/Object/MachOUniversal.h index f4c2eacf181..7eb2af944f3 100644 --- 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> getAsObjectFile() const; - ErrorOr> getAsArchive() const; + Expected> getAsArchive() const; }; class object_iterator { diff --git a/lib/Object/MachOUniversal.cpp b/lib/Object/MachOUniversal.cpp index e6880587cde..5c8501010c4 100644 --- a/lib/Object/MachOUniversal.cpp +++ b/lib/Object/MachOUniversal.cpp @@ -68,7 +68,8 @@ MachOUniversalBinary::ObjectForArch::ObjectForArch( Expected> 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> +Expected> 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> MachOUniversalBinary::getObjectForArch(StringRef ArchName) const { if (Triple(ArchName).getArch() == Triple::ArchType::UnknownArch) - return errorCodeToError(object_error::arch_not_found); + return make_error(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(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 index 0000000000000000000000000000000000000000..4fa5afb9087f760fbdea7f4bede59c24972fa85d GIT binary patch literal 1516 zcmbtSO=}cE5UtrUAI5B0gMtu|@gfnDW+NVq2U%S;dk~Sp;z^`;)3U*t9hn(%^&(zG zZ}~amK?QG~{1bv#?_#Vs(`A~T!Gk{NuBulbT~jq*e!c%dL_t8bz%t}>mT2xPOL}?h z&g~XF76DzOBTHDKgVvGWE7cygX^Dn|x8FX({*LP%t_Pn7CXUmM=vkV@d*_LE`u(fX z`b{(1*VE{-+E$k~x4IWA4GF-hN>K1(mx5U*SX*9LJm5yS(el838rSue&g)jcJ%5h_ zV5|IjQs@!OBCF&?w9Ld>6=qhD+S7$5HOg~RSy~z6G)c^qebclsYUl27GcWY`F$W;c zOddYUbYk$t-+);iqaTM&F=68*u>h=~j*u%gA*8t`nyX-)b%az-rsfD~zQ!SP)#`G`omw;#6BDOuOo`r~(vzn}ZeygwCALL}3pRk?2l~&$3K8o7c<0wDx&OH$PR36B zFYPf0$oExN6)QUA8Xs(-$H&FMHrci036^+ACVj^87cB2@cxe5;Kn)igY@p`9=5KK> zYWg#k_gC2G!he68o#5m&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 9661f8e2020..2cd0ede4a53 100644 --- 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> AOrErr = + } else if (Expected> AOrErr = I->getAsArchive()) { std::unique_ptr &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> AOrErr = + } else if (Expected> AOrErr = I->getAsArchive()) { std::unique_ptr &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> AOrErr = I->getAsArchive()) { + } else if (Expected> AOrErr = + I->getAsArchive()) { std::unique_ptr &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 b92599a02f8..82e7f8a258b 100644 --- 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> AOrErr = + } else if (Expected> AOrErr = I->getAsArchive()) { std::unique_ptr &A = *AOrErr; outs() << "Archive : " << Filename; @@ -1646,6 +1646,11 @@ void llvm::ParseInputMachO(StringRef Filename) { dyn_cast(&*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> AOrErr = + } else if (Expected> AOrErr = I->getAsArchive()) { std::unique_ptr &A = *AOrErr; outs() << "Archive : " << Filename << "\n"; @@ -1698,6 +1703,11 @@ void llvm::ParseInputMachO(StringRef Filename) { dyn_cast(&*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> AOrErr = I->getAsArchive()) { + } else if (Expected> AOrErr = + I->getAsArchive()) { std::unique_ptr &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 3c536af5713..bb171ec3fc3 100644 --- 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 439018963cb..5b10ee87ca8 100644 --- 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 06d5e058edf..ad97dea2d02 100644 --- 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> AOrErr = Obj.getAsArchive()) + else if (Expected> AOrErr = Obj.getAsArchive()) dumpArchive(&*AOrErr.get()); } } diff --git a/tools/llvm-size/llvm-size.cpp b/tools/llvm-size/llvm-size.cpp index 544eb104f5f..ecc0a0eac3c 100644 --- 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> AOrErr = + } else if (Expected> AOrErr = I->getAsArchive()) { std::unique_ptr &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> AOrErr = + } else if (Expected> AOrErr = I->getAsArchive()) { std::unique_ptr &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> AOrErr = + } else if (Expected> AOrErr = I->getAsArchive()) { std::unique_ptr &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(&Bin)) { -- 2.50.1