[llvm] r199770 - Be a bit more consistent about using ErrorOr when constructing Binary objects.
Rafael Espindola
rafael.espindola at gmail.com
Tue Jan 21 15:06:55 PST 2014
Author: rafael
Date: Tue Jan 21 17:06:54 2014
New Revision: 199770
URL: http://llvm.org/viewvc/llvm-project?rev=199770&view=rev
Log:
Be a bit more consistent about using ErrorOr when constructing Binary objects.
The constructors of classes deriving from Binary normally take an error_code
as an argument to the constructor. My original intent was to change them
to have a trivial constructor and move the initial parsing logic to a static
method returning an ErrorOr. I changed my mind because:
* A constructor with an error_code out parameter is extremely convenient from
the implementation side. We can incrementally construct the object and give
up when we find an error.
* It is very efficient when constructing on the stack or when there is no
error. The only inefficient case is where heap allocating and an error is
found (we have to free the memory).
The result is that this is a much smaller patch. It just standardizes the
create* helpers to return an ErrorOr.
Almost no functionality change: The only difference is that this found that
we were trying to read past the end of COFF import library but ignoring the
error.
Modified:
llvm/trunk/include/llvm/Object/Archive.h
llvm/trunk/include/llvm/Object/MachOUniversal.h
llvm/trunk/include/llvm/Object/ObjectFile.h
llvm/trunk/lib/Object/Archive.cpp
llvm/trunk/lib/Object/Binary.cpp
llvm/trunk/lib/Object/COFFObjectFile.cpp
llvm/trunk/lib/Object/ELFObjectFile.cpp
llvm/trunk/lib/Object/MachOObjectFile.cpp
llvm/trunk/lib/Object/MachOUniversal.cpp
llvm/trunk/lib/Object/ObjectFile.cpp
llvm/trunk/tools/llvm-objdump/MachODump.cpp
Modified: llvm/trunk/include/llvm/Object/Archive.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/Archive.h?rev=199770&r1=199769&r2=199770&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Object/Archive.h (original)
+++ llvm/trunk/include/llvm/Object/Archive.h Tue Jan 21 17:06:54 2014
@@ -17,6 +17,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Object/Binary.h"
#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/ErrorOr.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBuffer.h"
@@ -163,6 +164,7 @@ public:
};
Archive(MemoryBuffer *source, error_code &ec);
+ static ErrorOr<Archive *> create(MemoryBuffer *Source);
enum Kind {
K_GNU,
Modified: llvm/trunk/include/llvm/Object/MachOUniversal.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/MachOUniversal.h?rev=199770&r1=199769&r2=199770&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Object/MachOUniversal.h (original)
+++ llvm/trunk/include/llvm/Object/MachOUniversal.h Tue Jan 21 17:06:54 2014
@@ -18,6 +18,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Triple.h"
#include "llvm/Object/Binary.h"
+#include "llvm/Support/ErrorOr.h"
#include "llvm/Support/MachO.h"
namespace llvm {
@@ -77,6 +78,7 @@ public:
};
MachOUniversalBinary(MemoryBuffer *Source, error_code &ec);
+ static ErrorOr<MachOUniversalBinary*> create(MemoryBuffer *Source);
object_iterator begin_objects() const {
return ObjectForArch(this, 0);
Modified: llvm/trunk/include/llvm/Object/ObjectFile.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/ObjectFile.h?rev=199770&r1=199769&r2=199770&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Object/ObjectFile.h (original)
+++ llvm/trunk/include/llvm/Object/ObjectFile.h Tue Jan 21 17:06:54 2014
@@ -384,9 +384,9 @@ public:
}
public:
- static ObjectFile *createCOFFObjectFile(MemoryBuffer *Object);
- static ObjectFile *createELFObjectFile(MemoryBuffer *Object);
- static ObjectFile *createMachOObjectFile(MemoryBuffer *Object);
+ static ErrorOr<ObjectFile *> createCOFFObjectFile(MemoryBuffer *Object);
+ static ErrorOr<ObjectFile *> createELFObjectFile(MemoryBuffer *Object);
+ static ErrorOr<ObjectFile *> createMachOObjectFile(MemoryBuffer *Object);
};
// Inline function definitions.
Modified: llvm/trunk/lib/Object/Archive.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/Archive.cpp?rev=199770&r1=199769&r2=199770&view=diff
==============================================================================
--- llvm/trunk/lib/Object/Archive.cpp (original)
+++ llvm/trunk/lib/Object/Archive.cpp Tue Jan 21 17:06:54 2014
@@ -194,6 +194,14 @@ error_code Archive::Child::getAsBinary(O
return object_error::success;
}
+ErrorOr<Archive*> Archive::create(MemoryBuffer *Source) {
+ error_code EC;
+ OwningPtr<Archive> Ret(new Archive(Source, EC));
+ if (EC)
+ return EC;
+ return Ret.take();
+}
+
Archive::Archive(MemoryBuffer *source, error_code &ec)
: Binary(Binary::ID_Archive, source), SymbolTable(child_end()) {
// Check for sufficient magic.
Modified: llvm/trunk/lib/Object/Binary.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/Binary.cpp?rev=199770&r1=199769&r2=199770&view=diff
==============================================================================
--- llvm/trunk/lib/Object/Binary.cpp (original)
+++ llvm/trunk/lib/Object/Binary.cpp Tue Jan 21 17:06:54 2014
@@ -19,7 +19,6 @@
// Include headers for createBinary.
#include "llvm/Object/Archive.h"
-#include "llvm/Object/COFF.h"
#include "llvm/Object/MachOUniversal.h"
#include "llvm/Object/ObjectFile.h"
@@ -45,24 +44,14 @@ StringRef Binary::getFileName() const {
ErrorOr<Binary *> object::createBinary(MemoryBuffer *Source) {
OwningPtr<MemoryBuffer> scopedSource(Source);
sys::fs::file_magic type = sys::fs::identify_magic(Source->getBuffer());
- error_code EC;
switch (type) {
- case sys::fs::file_magic::archive: {
- OwningPtr<Binary> Ret(new Archive(scopedSource.take(), EC));
- if (EC)
- return EC;
- return Ret.take();
- }
+ case sys::fs::file_magic::archive:
+ return Archive::create(scopedSource.take());
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: {
- OwningPtr<Binary> Ret(
- ObjectFile::createELFObjectFile(scopedSource.take()));
- if (!Ret)
- return object_error::invalid_file_type;
- return Ret.take();
- }
+ case sys::fs::file_magic::elf_core:
+ return ObjectFile::createELFObjectFile(scopedSource.take());
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:
@@ -72,28 +61,14 @@ ErrorOr<Binary *> object::createBinary(M
case sys::fs::file_magic::macho_dynamic_linker:
case sys::fs::file_magic::macho_bundle:
case sys::fs::file_magic::macho_dynamically_linked_shared_lib_stub:
- case sys::fs::file_magic::macho_dsym_companion: {
- OwningPtr<Binary> Ret(
- ObjectFile::createMachOObjectFile(scopedSource.take()));
- if (!Ret)
- return object_error::invalid_file_type;
- return Ret.take();
- }
- case sys::fs::file_magic::macho_universal_binary: {
- OwningPtr<Binary> Ret(new MachOUniversalBinary(scopedSource.take(), EC));
- if (EC)
- return EC;
- return Ret.take();
- }
+ case sys::fs::file_magic::macho_dsym_companion:
+ return ObjectFile::createMachOObjectFile(scopedSource.take());
+ case sys::fs::file_magic::macho_universal_binary:
+ return MachOUniversalBinary::create(scopedSource.take());
case sys::fs::file_magic::coff_object:
case sys::fs::file_magic::coff_import_library:
- case sys::fs::file_magic::pecoff_executable: {
- OwningPtr<Binary> Ret(
- ObjectFile::createCOFFObjectFile(scopedSource.take()));
- if (!Ret)
- return object_error::invalid_file_type;
- return Ret.take();
- }
+ case sys::fs::file_magic::pecoff_executable:
+ return ObjectFile::createCOFFObjectFile(scopedSource.take());
case sys::fs::file_magic::unknown:
case sys::fs::file_magic::bitcode:
case sys::fs::file_magic::windows_resource: {
Modified: llvm/trunk/lib/Object/COFFObjectFile.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/COFFObjectFile.cpp?rev=199770&r1=199769&r2=199770&view=diff
==============================================================================
--- llvm/trunk/lib/Object/COFFObjectFile.cpp (original)
+++ llvm/trunk/lib/Object/COFFObjectFile.cpp Tue Jan 21 17:06:54 2014
@@ -514,10 +514,12 @@ COFFObjectFile::COFFObjectFile(MemoryBuf
CurPtr += COFFHeader->SizeOfOptionalHeader;
}
- if (!COFFHeader->isImportLibrary())
- if ((EC = getObject(SectionTable, Data, base() + CurPtr,
- COFFHeader->NumberOfSections * sizeof(coff_section))))
- return;
+ if (COFFHeader->isImportLibrary())
+ return;
+
+ if ((EC = getObject(SectionTable, Data, base() + CurPtr,
+ COFFHeader->NumberOfSections * sizeof(coff_section))))
+ return;
// Initialize the pointer to the symbol table.
if (COFFHeader->PointerToSymbolTable != 0)
@@ -1013,9 +1015,10 @@ error_code ExportDirectoryEntryRef::getS
return object_error::success;
}
-namespace llvm {
-ObjectFile *ObjectFile::createCOFFObjectFile(MemoryBuffer *Object) {
+ErrorOr<ObjectFile *> ObjectFile::createCOFFObjectFile(MemoryBuffer *Object) {
error_code EC;
- return new COFFObjectFile(Object, EC);
-}
+ OwningPtr<COFFObjectFile> Ret(new COFFObjectFile(Object, EC));
+ if (EC)
+ return EC;
+ return Ret.take();
}
Modified: llvm/trunk/lib/Object/ELFObjectFile.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/ELFObjectFile.cpp?rev=199770&r1=199769&r2=199770&view=diff
==============================================================================
--- llvm/trunk/lib/Object/ELFObjectFile.cpp (original)
+++ llvm/trunk/lib/Object/ELFObjectFile.cpp Tue Jan 21 17:06:54 2014
@@ -17,57 +17,60 @@
namespace llvm {
using namespace object;
-// Creates an in-memory object-file by default: createELFObjectFile(Buffer)
-ObjectFile *ObjectFile::createELFObjectFile(MemoryBuffer *Object) {
- std::pair<unsigned char, unsigned char> Ident = getElfArchType(Object);
- error_code ec;
-
+ErrorOr<ObjectFile *> ObjectFile::createELFObjectFile(MemoryBuffer *Obj) {
+ std::pair<unsigned char, unsigned char> Ident = getElfArchType(Obj);
std::size_t MaxAlignment =
- 1ULL << countTrailingZeros(uintptr_t(Object->getBufferStart()));
+ 1ULL << countTrailingZeros(uintptr_t(Obj->getBufferStart()));
+ error_code EC;
+ OwningPtr<ObjectFile> R;
if (Ident.first == ELF::ELFCLASS32 && Ident.second == ELF::ELFDATA2LSB)
#if !LLVM_IS_UNALIGNED_ACCESS_FAST
if (MaxAlignment >= 4)
- return new ELFObjectFile<ELFType<support::little, 4, false> >(Object, ec);
+ R.reset(new ELFObjectFile<ELFType<support::little, 4, false> >(Obj, EC));
else
#endif
if (MaxAlignment >= 2)
- return new ELFObjectFile<ELFType<support::little, 2, false> >(Object, ec);
+ R.reset(new ELFObjectFile<ELFType<support::little, 2, false> >(Obj, EC));
else
llvm_unreachable("Invalid alignment for ELF file!");
else if (Ident.first == ELF::ELFCLASS32 && Ident.second == ELF::ELFDATA2MSB)
#if !LLVM_IS_UNALIGNED_ACCESS_FAST
if (MaxAlignment >= 4)
- return new ELFObjectFile<ELFType<support::big, 4, false> >(Object, ec);
+ R.reset(new ELFObjectFile<ELFType<support::big, 4, false> >(Obj, EC));
else
#endif
if (MaxAlignment >= 2)
- return new ELFObjectFile<ELFType<support::big, 2, false> >(Object, ec);
+ R.reset(new ELFObjectFile<ELFType<support::big, 2, false> >(Obj, EC));
else
llvm_unreachable("Invalid alignment for ELF file!");
else if (Ident.first == ELF::ELFCLASS64 && Ident.second == ELF::ELFDATA2MSB)
#if !LLVM_IS_UNALIGNED_ACCESS_FAST
if (MaxAlignment >= 8)
- return new ELFObjectFile<ELFType<support::big, 8, true> >(Object, ec);
+ R.reset(new ELFObjectFile<ELFType<support::big, 8, true> >(Obj, EC));
else
#endif
if (MaxAlignment >= 2)
- return new ELFObjectFile<ELFType<support::big, 2, true> >(Object, ec);
+ R.reset(new ELFObjectFile<ELFType<support::big, 2, true> >(Obj, EC));
else
llvm_unreachable("Invalid alignment for ELF file!");
else if (Ident.first == ELF::ELFCLASS64 && Ident.second == ELF::ELFDATA2LSB) {
#if !LLVM_IS_UNALIGNED_ACCESS_FAST
if (MaxAlignment >= 8)
- return new ELFObjectFile<ELFType<support::little, 8, true> >(Object, ec);
+ R.reset(new ELFObjectFile<ELFType<support::little, 8, true> >(Obj, EC));
else
#endif
if (MaxAlignment >= 2)
- return new ELFObjectFile<ELFType<support::little, 2, true> >(Object, ec);
+ R.reset(new ELFObjectFile<ELFType<support::little, 2, true> >(Obj, EC));
else
llvm_unreachable("Invalid alignment for ELF file!");
}
+ else
+ report_fatal_error("Buffer is not an ELF object file!");
- report_fatal_error("Buffer is not an ELF object file!");
+ if (EC)
+ return EC;
+ return R.take();
}
} // end namespace llvm
Modified: llvm/trunk/lib/Object/MachOObjectFile.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/MachOObjectFile.cpp?rev=199770&r1=199769&r2=199770&view=diff
==============================================================================
--- llvm/trunk/lib/Object/MachOObjectFile.cpp (original)
+++ llvm/trunk/lib/Object/MachOObjectFile.cpp Tue Jan 21 17:06:54 2014
@@ -1582,25 +1582,25 @@ void MachOObjectFile::ReadULEB128s(uint6
}
}
-ObjectFile *ObjectFile::createMachOObjectFile(MemoryBuffer *Buffer) {
+ErrorOr<ObjectFile *> ObjectFile::createMachOObjectFile(MemoryBuffer *Buffer) {
StringRef Magic = Buffer->getBuffer().slice(0, 4);
- error_code ec;
- OwningPtr<ObjectFile> Ret;
+ error_code EC;
+ OwningPtr<MachOObjectFile> Ret;
if (Magic == "\xFE\xED\xFA\xCE")
- Ret.reset(new MachOObjectFile(Buffer, false, false, ec));
+ Ret.reset(new MachOObjectFile(Buffer, false, false, EC));
else if (Magic == "\xCE\xFA\xED\xFE")
- Ret.reset(new MachOObjectFile(Buffer, true, false, ec));
+ Ret.reset(new MachOObjectFile(Buffer, true, false, EC));
else if (Magic == "\xFE\xED\xFA\xCF")
- Ret.reset(new MachOObjectFile(Buffer, false, true, ec));
+ Ret.reset(new MachOObjectFile(Buffer, false, true, EC));
else if (Magic == "\xCF\xFA\xED\xFE")
- Ret.reset(new MachOObjectFile(Buffer, true, true, ec));
+ Ret.reset(new MachOObjectFile(Buffer, true, true, EC));
else {
delete Buffer;
- return NULL;
+ return object_error::parse_failed;
}
- if (ec)
- return NULL;
+ if (EC)
+ return EC;
return Ret.take();
}
Modified: llvm/trunk/lib/Object/MachOUniversal.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/MachOUniversal.cpp?rev=199770&r1=199769&r2=199770&view=diff
==============================================================================
--- llvm/trunk/lib/Object/MachOUniversal.cpp (original)
+++ llvm/trunk/lib/Object/MachOUniversal.cpp Tue Jan 21 17:06:54 2014
@@ -81,16 +81,26 @@ error_code MachOUniversalBinary::ObjectF
Triple::getArchTypeName(MachOObjectFile::getArch(Header.cputype));
MemoryBuffer *ObjBuffer = MemoryBuffer::getMemBuffer(
ObjectData, ObjectName, false);
- if (ObjectFile *Obj = ObjectFile::createMachOObjectFile(ObjBuffer)) {
- Result.reset(Obj);
- return object_error::success;
- }
+ ErrorOr<ObjectFile *> Obj = ObjectFile::createMachOObjectFile(ObjBuffer);
+ if (error_code EC = Obj.getError())
+ return EC;
+ Result.reset(Obj.get());
+ return object_error::success;
}
return object_error::parse_failed;
}
void MachOUniversalBinary::anchor() { }
+ErrorOr<MachOUniversalBinary *>
+MachOUniversalBinary::create(MemoryBuffer *Source) {
+ error_code EC;
+ OwningPtr<MachOUniversalBinary> Ret(new MachOUniversalBinary(Source, EC));
+ if (EC)
+ return EC;
+ return Ret.take();
+}
+
MachOUniversalBinary::MachOUniversalBinary(MemoryBuffer *Source,
error_code &ec)
: Binary(Binary::ID_MachOUniversalBinary, Source),
Modified: llvm/trunk/lib/Object/ObjectFile.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/ObjectFile.cpp?rev=199770&r1=199769&r2=199770&view=diff
==============================================================================
--- llvm/trunk/lib/Object/ObjectFile.cpp (original)
+++ llvm/trunk/lib/Object/ObjectFile.cpp Tue Jan 21 17:06:54 2014
@@ -56,7 +56,7 @@ ObjectFile *ObjectFile::createObjectFile
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 createELFObjectFile(Object).get();
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:
@@ -67,11 +67,11 @@ ObjectFile *ObjectFile::createObjectFile
case sys::fs::file_magic::macho_bundle:
case sys::fs::file_magic::macho_dynamically_linked_shared_lib_stub:
case sys::fs::file_magic::macho_dsym_companion:
- return createMachOObjectFile(Object);
+ return createMachOObjectFile(Object).get();
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 createCOFFObjectFile(Object).get();
}
llvm_unreachable("Unexpected Object File Type");
}
Modified: llvm/trunk/tools/llvm-objdump/MachODump.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/MachODump.cpp?rev=199770&r1=199769&r2=199770&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-objdump/MachODump.cpp (original)
+++ llvm/trunk/tools/llvm-objdump/MachODump.cpp Tue Jan 21 17:06:54 2014
@@ -207,8 +207,8 @@ void llvm::DisassembleInputMachO(StringR
return;
}
- OwningPtr<MachOObjectFile> MachOOF(static_cast<MachOObjectFile*>(
- ObjectFile::createMachOObjectFile(Buff.take())));
+ OwningPtr<MachOObjectFile> MachOOF(static_cast<MachOObjectFile *>(
+ ObjectFile::createMachOObjectFile(Buff.take()).get()));
DisassembleInputMachO2(Filename, MachOOF.get());
}
@@ -297,7 +297,7 @@ static void DisassembleInputMachO2(Strin
errs() << "llvm-objdump: " << Filename << ": " << ec.message() << '\n';
return;
}
- DbgObj = ObjectFile::createMachOObjectFile(Buf.take());
+ DbgObj = ObjectFile::createMachOObjectFile(Buf.take()).get();
}
// Setup the DIContext
More information about the llvm-commits
mailing list