Skip to content

Commit 4239805

Browse files
enderbyenderby
committed
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. llvm-svn: 274079
1 parent 9c12639 commit 4239805

File tree

10 files changed

+90
-18
lines changed

10 files changed

+90
-18
lines changed

‎llvm/include/llvm/Object/MachOUniversal.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
#include "llvm/Object/Archive.h"
2020
#include "llvm/Object/Binary.h"
2121
#include "llvm/Object/MachO.h"
22-
#include "llvm/Support/ErrorOr.h"
2322
#include "llvm/Support/MachO.h"
2423

2524
namespace llvm {
@@ -105,7 +104,7 @@ class MachOUniversalBinary : public Binary {
105104

106105
Expected<std::unique_ptr<MachOObjectFile>> getAsObjectFile() const;
107106

108-
ErrorOr<std::unique_ptr<Archive>> getAsArchive() const;
107+
Expected<std::unique_ptr<Archive>> getAsArchive() const;
109108
};
110109

111110
class object_iterator {

‎llvm/lib/Object/MachOUniversal.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ MachOUniversalBinary::ObjectForArch::ObjectForArch(
6868
Expected<std::unique_ptr<MachOObjectFile>>
6969
MachOUniversalBinary::ObjectForArch::getAsObjectFile() const {
7070
if (!Parent)
71-
return errorCodeToError(object_error::parse_failed);
71+
report_fatal_error("MachOUniversalBinary::ObjectForArch::getAsObjectFile() "
72+
"called when Parent is a nullptr");
7273

7374
StringRef ParentData = Parent->getData();
7475
StringRef ObjectData;
@@ -81,10 +82,11 @@ MachOUniversalBinary::ObjectForArch::getAsObjectFile() const {
8182
return ObjectFile::createMachOObjectFile(ObjBuffer);
8283
}
8384

84-
ErrorOr<std::unique_ptr<Archive>>
85+
Expected<std::unique_ptr<Archive>>
8586
MachOUniversalBinary::ObjectForArch::getAsArchive() const {
8687
if (!Parent)
87-
return object_error::parse_failed;
88+
report_fatal_error("MachOUniversalBinary::ObjectForArch::getAsArchive() "
89+
"called when Parent is a nullptr");
8890

8991
StringRef ParentData = Parent->getData();
9092
StringRef ObjectData;
@@ -94,7 +96,7 @@ MachOUniversalBinary::ObjectForArch::getAsArchive() const {
9496
ObjectData = ParentData.substr(Header64.offset, Header64.size);
9597
StringRef ObjectName = Parent->getFileName();
9698
MemoryBufferRef ObjBuffer(ObjectData, ObjectName);
97-
return Archive::create(ObjBuffer);
99+
return errorOrToExpected(Archive::create(ObjBuffer));
98100
}
99101

100102
void MachOUniversalBinary::anchor() { }
@@ -145,11 +147,15 @@ MachOUniversalBinary::MachOUniversalBinary(MemoryBufferRef Source, Error &Err)
145147
Expected<std::unique_ptr<MachOObjectFile>>
146148
MachOUniversalBinary::getObjectForArch(StringRef ArchName) const {
147149
if (Triple(ArchName).getArch() == Triple::ArchType::UnknownArch)
148-
return errorCodeToError(object_error::arch_not_found);
150+
return make_error<GenericBinaryError>(std::move("Unknown architecture "
151+
"named: " + ArchName),
152+
object_error::arch_not_found);
149153

150154
for (object_iterator I = begin_objects(), E = end_objects(); I != E; ++I) {
151155
if (I->getArchTypeName() == ArchName)
152156
return I->getAsObjectFile();
153157
}
154-
return errorCodeToError(object_error::arch_not_found);
158+
return make_error<GenericBinaryError>(std::move("fat file does not "
159+
"contain " + ArchName),
160+
object_error::arch_not_found);
155161
}

‎llvm/test/Object/Inputs/macho-invalid-fat.obj.elf-x86_64

1.48 KB
Binary file not shown.

‎llvm/test/Object/macho-invalid.test

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,3 +97,6 @@ INCOMPLETE-SEGMENT-LOADC-FAT-ARCHIVE: macho-universal-archive-bad2.x86_64.i386(m
9797

9898
RUN: not llvm-objdump -macho -universal-headers %p/Inputs/macho-invalid-fat 2>&1 | FileCheck -check-prefix INVALID-FAT %s
9999
INVALID-FAT: truncated or malformed fat file (fat_arch_64 structs would extend past the end of the file)
100+
101+
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
102+
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

‎llvm/tools/llvm-nm/llvm-nm.cpp

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,7 +1171,7 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
11711171
error(std::move(E), Filename, ArchFlags.size() > 1 ?
11721172
StringRef(I->getArchTypeName()) : StringRef());
11731173
continue;
1174-
} else if (ErrorOr<std::unique_ptr<Archive>> AOrErr =
1174+
} else if (Expected<std::unique_ptr<Archive>> AOrErr =
11751175
I->getAsArchive()) {
11761176
std::unique_ptr<Archive> &A = *AOrErr;
11771177
for (Archive::child_iterator AI = A->child_begin(),
@@ -1209,6 +1209,12 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
12091209
ArchitectureName);
12101210
}
12111211
}
1212+
} else {
1213+
consumeError(AOrErr.takeError());
1214+
error(Filename + " for architecture " +
1215+
StringRef(I->getArchTypeName()) +
1216+
" is not a Mach-O file or an archive file",
1217+
"Mach-O universal file");
12121218
}
12131219
}
12141220
}
@@ -1238,7 +1244,7 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
12381244
ObjOrErr.takeError())) {
12391245
error(std::move(E), Filename);
12401246
return;
1241-
} else if (ErrorOr<std::unique_ptr<Archive>> AOrErr =
1247+
} else if (Expected<std::unique_ptr<Archive>> AOrErr =
12421248
I->getAsArchive()) {
12431249
std::unique_ptr<Archive> &A = *AOrErr;
12441250
for (Archive::child_iterator AI = A->child_begin(),
@@ -1266,6 +1272,12 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
12661272
dumpSymbolNamesFromObject(*O, false, ArchiveName);
12671273
}
12681274
}
1275+
} else {
1276+
consumeError(AOrErr.takeError());
1277+
error(Filename + " for architecture " +
1278+
StringRef(I->getArchTypeName()) +
1279+
" is not a Mach-O file or an archive file",
1280+
"Mach-O universal file");
12691281
}
12701282
return;
12711283
}
@@ -1301,7 +1313,8 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
13011313
error(std::move(E), Filename, moreThanOneArch ?
13021314
StringRef(I->getArchTypeName()) : StringRef());
13031315
continue;
1304-
} else if (ErrorOr<std::unique_ptr<Archive>> AOrErr = I->getAsArchive()) {
1316+
} else if (Expected<std::unique_ptr<Archive>> AOrErr =
1317+
I->getAsArchive()) {
13051318
std::unique_ptr<Archive> &A = *AOrErr;
13061319
for (Archive::child_iterator AI = A->child_begin(), AE = A->child_end();
13071320
AI != AE; ++AI) {
@@ -1336,6 +1349,12 @@ static void dumpSymbolNamesFromFile(std::string &Filename) {
13361349
dumpSymbolNamesFromObject(*O, false, ArchiveName, ArchitectureName);
13371350
}
13381351
}
1352+
} else {
1353+
consumeError(AOrErr.takeError());
1354+
error(Filename + " for architecture " +
1355+
StringRef(I->getArchTypeName()) +
1356+
" is not a Mach-O file or an archive file",
1357+
"Mach-O universal file");
13391358
}
13401359
}
13411360
return;

‎llvm/tools/llvm-objdump/MachODump.cpp

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1621,7 +1621,7 @@ void llvm::ParseInputMachO(StringRef Filename) {
16211621
report_error(Filename, StringRef(), std::move(E),
16221622
ArchitectureName);
16231623
continue;
1624-
} else if (ErrorOr<std::unique_ptr<Archive>> AOrErr =
1624+
} else if (Expected<std::unique_ptr<Archive>> AOrErr =
16251625
I->getAsArchive()) {
16261626
std::unique_ptr<Archive> &A = *AOrErr;
16271627
outs() << "Archive : " << Filename;
@@ -1646,6 +1646,11 @@ void llvm::ParseInputMachO(StringRef Filename) {
16461646
dyn_cast<MachOObjectFile>(&*ChildOrErr.get()))
16471647
ProcessMachO(Filename, O, O->getFileName(), ArchitectureName);
16481648
}
1649+
} else {
1650+
consumeError(AOrErr.takeError());
1651+
error("Mach-O universal file: " + Filename + " for " +
1652+
"architecture " + StringRef(I->getArchTypeName()) +
1653+
" is not a Mach-O file or an archive file");
16491654
}
16501655
}
16511656
}
@@ -1676,7 +1681,7 @@ void llvm::ParseInputMachO(StringRef Filename) {
16761681
ObjOrErr.takeError())) {
16771682
report_error(Filename, std::move(E));
16781683
continue;
1679-
} else if (ErrorOr<std::unique_ptr<Archive>> AOrErr =
1684+
} else if (Expected<std::unique_ptr<Archive>> AOrErr =
16801685
I->getAsArchive()) {
16811686
std::unique_ptr<Archive> &A = *AOrErr;
16821687
outs() << "Archive : " << Filename << "\n";
@@ -1698,6 +1703,11 @@ void llvm::ParseInputMachO(StringRef Filename) {
16981703
dyn_cast<MachOObjectFile>(&*ChildOrErr.get()))
16991704
ProcessMachO(Filename, O, O->getFileName());
17001705
}
1706+
} else {
1707+
consumeError(AOrErr.takeError());
1708+
error("Mach-O universal file: " + Filename + " for architecture " +
1709+
StringRef(I->getArchTypeName()) +
1710+
" is not a Mach-O file or an archive file");
17011711
}
17021712
return;
17031713
}
@@ -1721,7 +1731,8 @@ void llvm::ParseInputMachO(StringRef Filename) {
17211731
ObjOrErr.takeError())) {
17221732
report_error(StringRef(), Filename, std::move(E), ArchitectureName);
17231733
continue;
1724-
} else if (ErrorOr<std::unique_ptr<Archive>> AOrErr = I->getAsArchive()) {
1734+
} else if (Expected<std::unique_ptr<Archive>> AOrErr =
1735+
I->getAsArchive()) {
17251736
std::unique_ptr<Archive> &A = *AOrErr;
17261737
outs() << "Archive : " << Filename;
17271738
if (!ArchitectureName.empty())
@@ -1747,6 +1758,11 @@ void llvm::ParseInputMachO(StringRef Filename) {
17471758
ArchitectureName);
17481759
}
17491760
}
1761+
} else {
1762+
consumeError(AOrErr.takeError());
1763+
error("Mach-O universal file: " + Filename + " for architecture " +
1764+
StringRef(I->getArchTypeName()) +
1765+
" is not a Mach-O file or an archive file");
17501766
}
17511767
}
17521768
return;

‎llvm/tools/llvm-objdump/llvm-objdump.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,12 @@ void llvm::error(std::error_code EC) {
264264
exit(1);
265265
}
266266

267+
LLVM_ATTRIBUTE_NORETURN void llvm::error(Twine Message) {
268+
errs() << ToolName << ": " << Message << ".\n";
269+
errs().flush();
270+
exit(1);
271+
}
272+
267273
LLVM_ATTRIBUTE_NORETURN void llvm::report_error(StringRef File,
268274
std::error_code EC) {
269275
assert(EC);

‎llvm/tools/llvm-objdump/llvm-objdump.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ void PrintSectionHeaders(const object::ObjectFile *o);
8888
void PrintSectionContents(const object::ObjectFile *o);
8989
void PrintSymbolTable(const object::ObjectFile *o, StringRef ArchiveName,
9090
StringRef ArchitectureName = StringRef());
91+
LLVM_ATTRIBUTE_NORETURN void error(Twine Message);
9192
LLVM_ATTRIBUTE_NORETURN void report_error(StringRef File, std::error_code EC);
9293
LLVM_ATTRIBUTE_NORETURN void report_error(StringRef File, llvm::Error E);
9394
LLVM_ATTRIBUTE_NORETURN void report_error(StringRef FileName,

‎llvm/tools/llvm-readobj/llvm-readobj.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ static void dumpMachOUniversalBinary(const MachOUniversalBinary *UBinary) {
459459
OS.flush();
460460
reportError(UBinary->getFileName(), Buf);
461461
}
462-
else if (ErrorOr<std::unique_ptr<Archive>> AOrErr = Obj.getAsArchive())
462+
else if (Expected<std::unique_ptr<Archive>> AOrErr = Obj.getAsArchive())
463463
dumpArchive(&*AOrErr.get());
464464
}
465465
}

‎llvm/tools/llvm-size/llvm-size.cpp

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,13 @@ static bool error(std::error_code ec) {
9999
return true;
100100
}
101101

102+
static bool error(Twine Message) {
103+
HadError = true;
104+
errs() << ToolName << ": " << Message << ".\n";
105+
errs().flush();
106+
return true;
107+
}
108+
102109
// This version of error() prints the archive name and member name, for example:
103110
// "libx.a(foo.o)" after the ToolName before the error message. It sets
104111
// HadError but returns allowing the code to move on to other archive members.
@@ -585,7 +592,7 @@ static void printFileSectionSizes(StringRef file) {
585592
error(std::move(E), file, ArchFlags.size() > 1 ?
586593
StringRef(I->getArchTypeName()) : StringRef());
587594
return;
588-
} else if (ErrorOr<std::unique_ptr<Archive>> AOrErr =
595+
} else if (Expected<std::unique_ptr<Archive>> AOrErr =
589596
I->getAsArchive()) {
590597
std::unique_ptr<Archive> &UA = *AOrErr;
591598
// This is an archive. Iterate over each member and display its
@@ -630,6 +637,11 @@ static void printFileSectionSizes(StringRef file) {
630637
}
631638
}
632639
}
640+
} else {
641+
consumeError(AOrErr.takeError());
642+
error("Mach-O universal file: " + file + " for architecture " +
643+
StringRef(I->getArchTypeName()) +
644+
" is not a Mach-O file or an archive file");
633645
}
634646
}
635647
}
@@ -671,7 +683,7 @@ static void printFileSectionSizes(StringRef file) {
671683
} else if (auto E = isNotObjectErrorInvalidFileType(UO.takeError())) {
672684
error(std::move(E), file);
673685
return;
674-
} else if (ErrorOr<std::unique_ptr<Archive>> AOrErr =
686+
} else if (Expected<std::unique_ptr<Archive>> AOrErr =
675687
I->getAsArchive()) {
676688
std::unique_ptr<Archive> &UA = *AOrErr;
677689
// This is an archive. Iterate over each member and display its
@@ -709,6 +721,11 @@ static void printFileSectionSizes(StringRef file) {
709721
}
710722
}
711723
}
724+
} else {
725+
consumeError(AOrErr.takeError());
726+
error("Mach-O universal file: " + file + " for architecture " +
727+
StringRef(I->getArchTypeName()) +
728+
" is not a Mach-O file or an archive file");
712729
}
713730
return;
714731
}
@@ -744,7 +761,7 @@ static void printFileSectionSizes(StringRef file) {
744761
error(std::move(E), file, MoreThanOneArch ?
745762
StringRef(I->getArchTypeName()) : StringRef());
746763
return;
747-
} else if (ErrorOr<std::unique_ptr<Archive>> AOrErr =
764+
} else if (Expected<std::unique_ptr<Archive>> AOrErr =
748765
I->getAsArchive()) {
749766
std::unique_ptr<Archive> &UA = *AOrErr;
750767
// This is an archive. Iterate over each member and display its sizes.
@@ -781,6 +798,11 @@ static void printFileSectionSizes(StringRef file) {
781798
}
782799
}
783800
}
801+
} else {
802+
consumeError(AOrErr.takeError());
803+
error("Mach-O universal file: " + file + " for architecture " +
804+
StringRef(I->getArchTypeName()) +
805+
" is not a Mach-O file or an archive file");
784806
}
785807
}
786808
} else if (ObjectFile *o = dyn_cast<ObjectFile>(&Bin)) {

0 commit comments

Comments
 (0)