<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 25, 2016 at 10:25 AM, Lang Hames via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: lhames<br>
Date: Fri Mar 25 12:25:34 2016<br>
New Revision: 264425<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=264425&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=264425&view=rev</a><br>
Log:<br>
[Object] Start threading Error through MachOObjectFile construction.<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/Object/Error.h<br>
llvm/trunk/include/llvm/Object/MachO.h<br>
llvm/trunk/lib/Object/Error.cpp<br>
llvm/trunk/lib/Object/MachOObjectFile.cpp<br>
llvm/trunk/tools/llvm-rtdyld/llvm-rtdyld.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/Object/Error.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/Error.h?rev=264425&r1=264424&r2=264425&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/Error.h?rev=264425&r1=264424&r2=264425&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Object/Error.h (original)<br>
+++ llvm/trunk/include/llvm/Object/Error.h Fri Mar 25 12:25:34 2016<br>
@@ -14,11 +14,15 @@<br>
#ifndef LLVM_OBJECT_ERROR_H<br>
#define LLVM_OBJECT_ERROR_H<br>
<br>
+#include "llvm/ADT/Twine.h"<br>
+#include "llvm/Support/Error.h"<br>
#include <system_error><br>
<br>
namespace llvm {<br>
namespace object {<br>
<br>
+class Binary;<br>
+<br>
const std::error_category &object_category();<br>
<br>
enum class object_error {<br>
@@ -39,6 +43,41 @@ inline std::error_code make_error_code(o<br>
return std::error_code(static_cast<int>(e), object_category());<br>
}<br>
<br>
+/// Base class for all errors indicating malformed binary files.<br>
+///<br>
+/// Having a subclass for all malformed binary files allows archive-walking<br>
+/// code to skip malformed files without having to understand every possible<br>
+/// way that a binary file might be malformed.<br>
+///<br>
+/// Currently inherits from ECError for easy interoperability with<br>
+/// std::error_code, but this will be removed in the future.<br>
+class BinaryError : public ErrorInfo<BinaryError, ECError> {<br></blockquote><div><br></div><div>Might as well just make this class a struct? (its publicly inheriting, has only public members) or at least drop the empty "private:" section?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+public:<br>
+ static char ID;<br>
+ BinaryError() {<br>
+ // Default to parse_failed, can be overridden with setErrorCode.<br>
+ setErrorCode(make_error_code(object_error::parse_failed));<br>
+ }<br>
+private:<br>
+};<br>
+<br>
+/// Generic binary error.<br>
+///<br>
+/// For errors that don't require their own specific sub-error (most errors)<br>
+/// this class can be used to describe the error via a string message.<br>
+class GenericBinaryError : public ErrorInfo<GenericBinaryError, BinaryError> {<br>
+public:<br>
+ static char ID;<br>
+ GenericBinaryError(std::string FileName, Twine Msg);<br>
+ GenericBinaryError(std::string FileName, Twine Msg, object_error ECOverride);<br>
+ const std::string &getFileName() const { return FileName; }<br>
+ const std::string &getMessage() const { return Msg; }<br>
+ void log(raw_ostream &OS) const override;<br>
+private:<br>
+ std::string FileName;<br>
+ std::string Msg;<br>
+};<br>
+<br>
} // end namespace object.<br>
<br>
} // end namespace llvm.<br>
<br>
Modified: llvm/trunk/include/llvm/Object/MachO.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/MachO.h?rev=264425&r1=264424&r2=264425&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/MachO.h?rev=264425&r1=264424&r2=264425&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Object/MachO.h (original)<br>
+++ llvm/trunk/include/llvm/Object/MachO.h Fri Mar 25 12:25:34 2016<br>
@@ -194,7 +194,7 @@ public:<br>
typedef LoadCommandList::const_iterator load_command_iterator;<br>
<br>
MachOObjectFile(MemoryBufferRef Object, bool IsLittleEndian, bool Is64Bits,<br>
- std::error_code &EC);<br>
+ Error &Err);<br>
<br>
void moveSymbolNext(DataRefImpl &Symb) const override;<br>
<br>
<br>
Modified: llvm/trunk/lib/Object/Error.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/Error.cpp?rev=264425&r1=264424&r2=264425&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/Error.cpp?rev=264425&r1=264424&r2=264425&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Object/Error.cpp (original)<br>
+++ llvm/trunk/lib/Object/Error.cpp Fri Mar 25 12:25:34 2016<br>
@@ -58,6 +58,21 @@ std::string _object_error_category::mess<br>
"defined.");<br>
}<br>
<br>
+char BinaryError::ID = 0;<br>
+char GenericBinaryError::ID = 0;<br></blockquote><div><br></div><div>I'm assuming these don't really need values( just their address is used) so perhaps leave them uninitialized? (rather than giving the impression their value is important)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+GenericBinaryError::GenericBinaryError(std::string FileName, Twine Msg)<br></blockquote><div><br></div><div>Might as well pass Msg as a string instead of a twine, if there's no opportunity to delay stringification? (Twine is mostly used for cases where it's passed into an API that only conditionally uses it, so the concatenation can be avoided in the cases where it's not needed) This would allow moves/avoid copies, etc.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ : FileName(std::move(FileName)), Msg(Msg.str()) {}<br>
+<br>
+GenericBinaryError::GenericBinaryError(std::string FileName, Twine Msg, object_error ECOverride)<br>
+ : FileName(std::move(FileName)), Msg(Msg.str()) { </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ setErrorCode(make_error_code(ECOverride));<br>
+}<br>
+<br>
+void GenericBinaryError::log(raw_ostream &OS) const {<br>
+ OS << "Error in " << FileName << ": " << Msg;<br>
+}<br>
+<br>
static ManagedStatic<_object_error_category> error_category;<br>
<br>
const std::error_category &object::object_category() {<br>
<br>
Modified: llvm/trunk/lib/Object/MachOObjectFile.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/MachOObjectFile.cpp?rev=264425&r1=264424&r2=264425&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/MachOObjectFile.cpp?rev=264425&r1=264424&r2=264425&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Object/MachOObjectFile.cpp (original)<br>
+++ llvm/trunk/lib/Object/MachOObjectFile.cpp Fri Mar 25 12:25:34 2016<br>
@@ -38,6 +38,22 @@ namespace {<br>
};<br>
}<br>
<br>
+// FIXME: Remove ECOverride once Error has been plumbed down to obj tool code.</blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+static Error<br>
+malformedError(std::string FileName, std::string Msg,<br>
+ object_error ECOverride = object_error::parse_failed) {<br>
+ return make_error<GenericBinaryError>(std::move(FileName), std::move(Msg),<br>
+ ECOverride);<br>
+}<br>
+<br>
+<br>
+// FIXME: Remove ECOverride once Error has been plumbed down to obj tool code.<br>
+static Error<br>
+malformedError(const MachOObjectFile &Obj, std::string Msg,<br>
+ object_error ECOverride = object_error::parse_failed) {<br>
+ return malformedError(Obj.getFileName(), std::move(Msg), ECOverride);<br>
+}<br>
+<br>
// FIXME: Replace all uses of this function with getStructOrErr.<br>
template <typename T><br>
static T getStruct(const MachOObjectFile *O, const char *P) {<br>
@@ -53,10 +69,10 @@ static T getStruct(const MachOObjectFile<br>
}<br>
<br>
template <typename T><br>
-static ErrorOr<T> getStructOrErr(const MachOObjectFile *O, const char *P) {<br>
+static Expected<T> getStructOrErr(const MachOObjectFile *O, const char *P) {<br>
// Don't read before the beginning or past the end of the file<br>
if (P < O->getData().begin() || P + sizeof(T) > O->getData().end())<br>
- return object_error::parse_failed;<br>
+ return malformedError(*O, "Structure read out-of-range");<br>
<br>
T Cmd;<br>
memcpy(&Cmd, P, sizeof(T));<br>
@@ -161,27 +177,25 @@ static uint32_t getSectionFlags(const Ma<br>
return Sect.flags;<br>
}<br>
<br>
-static ErrorOr<MachOObjectFile::LoadCommandInfo><br>
+static Expected<MachOObjectFile::LoadCommandInfo><br>
getLoadCommandInfo(const MachOObjectFile *Obj, const char *Ptr) {<br>
- auto CmdOrErr = getStructOrErr<MachO::load_command>(Obj, Ptr);<br>
- if (!CmdOrErr)<br>
- return CmdOrErr.getError();<br>
- if (CmdOrErr->cmdsize < 8)<br>
- return object_error::macho_small_load_command;<br>
- MachOObjectFile::LoadCommandInfo Load;<br>
- Load.Ptr = Ptr;<br>
- Load.C = CmdOrErr.get();<br>
- return Load;<br>
+ if (auto CmdOrErr = getStructOrErr<MachO::load_command>(Obj, Ptr)) {<br>
+ if (CmdOrErr->cmdsize < 8)<br>
+ return malformedError(*Obj, "Mach-O load command with size < 8 bytes",<br>
+ object_error::macho_small_load_command);<br>
+ return MachOObjectFile::LoadCommandInfo({Ptr, *CmdOrErr});<br></blockquote><div><br></div><div>You can just "return {Ptr, *CmdOrErr};" if you're going to use braced init anyway/if you like.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ } else<br></blockquote><div><br></div><div>Else-after-return</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ return CmdOrErr.takeError();<br>
}<br>
<br>
-static ErrorOr<MachOObjectFile::LoadCommandInfo><br>
+static Expected<MachOObjectFile::LoadCommandInfo><br>
getFirstLoadCommandInfo(const MachOObjectFile *Obj) {<br>
unsigned HeaderSize = Obj->is64Bit() ? sizeof(MachO::mach_header_64)<br>
: sizeof(MachO::mach_header);<br>
return getLoadCommandInfo(Obj, getPtr(Obj, HeaderSize));<br>
}<br>
<br>
-static ErrorOr<MachOObjectFile::LoadCommandInfo><br>
+static Expected<MachOObjectFile::LoadCommandInfo><br>
getNextLoadCommandInfo(const MachOObjectFile *Obj,<br>
const MachOObjectFile::LoadCommandInfo &L) {<br>
return getLoadCommandInfo(Obj, L.Ptr + L.C.cmdsize);<br>
@@ -189,92 +203,104 @@ getNextLoadCommandInfo(const MachOObject<br>
<br>
template <typename T><br>
static void parseHeader(const MachOObjectFile *Obj, T &Header,<br>
- std::error_code &EC) {<br>
- auto HeaderOrErr = getStructOrErr<T>(Obj, getPtr(Obj, 0));<br>
- if (HeaderOrErr)<br>
- Header = HeaderOrErr.get();<br>
+ Error &Err) {<br>
+ if (auto HeaderOrErr = getStructOrErr<T>(Obj, getPtr(Obj, 0)))<br>
+ Header = *HeaderOrErr;<br>
else<br>
- EC = HeaderOrErr.getError();<br>
+ Err = HeaderOrErr.takeError();<br>
}<br>
<br>
// Parses LC_SEGMENT or LC_SEGMENT_64 load command, adds addresses of all<br>
// sections to \param Sections, and optionally sets<br>
// \param IsPageZeroSegment to true.<br>
template <typename SegmentCmd><br>
-static std::error_code parseSegmentLoadCommand(<br>
+static Error parseSegmentLoadCommand(<br>
const MachOObjectFile *Obj, const MachOObjectFile::LoadCommandInfo &Load,<br>
SmallVectorImpl<const char *> &Sections, bool &IsPageZeroSegment) {<br>
const unsigned SegmentLoadSize = sizeof(SegmentCmd);<br>
if (Load.C.cmdsize < SegmentLoadSize)<br>
- return object_error::macho_load_segment_too_small;<br>
- auto SegOrErr = getStructOrErr<SegmentCmd>(Obj, Load.Ptr);<br>
- if (!SegOrErr)<br>
- return SegOrErr.getError();<br>
- SegmentCmd S = SegOrErr.get();<br>
- const unsigned SectionSize =<br>
+ return malformedError(*Obj,<br>
+ "Mach-O segment load command size is too small",<br>
+ object_error::macho_load_segment_too_small);<br>
+ if (auto SegOrErr = getStructOrErr<SegmentCmd>(Obj, Load.Ptr)) {<br>
+ SegmentCmd S = SegOrErr.get();<br>
+ const unsigned SectionSize =<br>
Obj->is64Bit() ? sizeof(MachO::section_64) : sizeof(MachO::section);<br>
- if (S.nsects > std::numeric_limits<uint32_t>::max() / SectionSize ||<br>
- S.nsects * SectionSize > Load.C.cmdsize - SegmentLoadSize)<br>
- return object_error::macho_load_segment_too_many_sections;<br>
- for (unsigned J = 0; J < S.nsects; ++J) {<br>
- const char *Sec = getSectionPtr(Obj, Load, J);<br>
- Sections.push_back(Sec);<br>
- }<br>
- IsPageZeroSegment |= StringRef("__PAGEZERO").equals(S.segname);<br>
- return std::error_code();<br>
+ if (S.nsects > std::numeric_limits<uint32_t>::max() / SectionSize ||<br>
+ S.nsects * SectionSize > Load.C.cmdsize - SegmentLoadSize)<br>
+ return malformedError(*Obj,<br>
+ "Mach-O segment load command contains too many "<br>
+ "sections",<br>
+ object_error::macho_load_segment_too_many_sections);<br>
+ for (unsigned J = 0; J < S.nsects; ++J) {<br>
+ const char *Sec = getSectionPtr(Obj, Load, J);<br>
+ Sections.push_back(Sec);<br>
+ }<br>
+ IsPageZeroSegment |= StringRef("__PAGEZERO").equals(S.segname);<br>
+ } else<br>
+ return SegOrErr.takeError();<br>
+<br>
+ return Error::success();<br>
}<br>
<br>
MachOObjectFile::MachOObjectFile(MemoryBufferRef Object, bool IsLittleEndian,<br>
- bool Is64bits, std::error_code &EC)<br>
+ bool Is64bits, Error &Err)<br>
: ObjectFile(getMachOType(IsLittleEndian, Is64bits), Object),<br>
SymtabLoadCmd(nullptr), DysymtabLoadCmd(nullptr),<br>
DataInCodeLoadCmd(nullptr), LinkOptHintsLoadCmd(nullptr),<br>
DyldInfoLoadCmd(nullptr), UuidLoadCmd(nullptr),<br>
HasPageZeroSegment(false) {<br>
+<br>
+ // We have to check Err before it's assigned to.<br>
+ if (Err)<br>
+ llvm_unreachable("Err should be in success state at entry to constructor.");<br></blockquote><div><br></div><div>Branch-to-unreachable should be assert. But then the error wouldn't be checked? Perhaps need a more explicit discard operation for that.<br><br>If we're going to use Err out parameters, perhaps the caller should have some way of constructing an Err in the checked state instead of callers having to clear it?<br><br>(alternatively this API could be refactored into a named ctor style? (returning Expected<MachOObjectFile> and doing most of this logic in a static function rather than in the ctor? (& a private ctor that allows the processed data to be passed in to initialize the object)))</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
if (is64Bit())<br>
- parseHeader(this, Header64, EC);<br>
+ parseHeader(this, Header64, Err);<br>
else<br>
- parseHeader(this, Header, EC);<br>
- if (EC)<br>
+ parseHeader(this, Header, Err);<br>
+ if (Err)<br>
return;<br>
<br>
uint32_t LoadCommandCount = getHeader().ncmds;<br>
if (LoadCommandCount == 0)<br>
return;<br>
<br>
- auto LoadOrErr = getFirstLoadCommandInfo(this);<br>
- if (!LoadOrErr) {<br>
- EC = LoadOrErr.getError();<br>
+ LoadCommandInfo Load;<br>
+ if (auto LoadOrErr = getFirstLoadCommandInfo(this))<br>
+ Load = *LoadOrErr;<br>
+ else {<br>
+ Err = LoadOrErr.takeError();<br>
return;<br>
}<br>
- LoadCommandInfo Load = LoadOrErr.get();<br>
+<br>
for (unsigned I = 0; I < LoadCommandCount; ++I) {<br>
LoadCommands.push_back(Load);<br>
if (Load.C.cmd == MachO::LC_SYMTAB) {<br>
// Multiple symbol tables<br>
if (SymtabLoadCmd) {<br>
- EC = object_error::parse_failed;<br>
+ Err = malformedError(*this, "Multiple symbol tables");<br>
return;<br>
}<br>
SymtabLoadCmd = Load.Ptr;<br>
} else if (Load.C.cmd == MachO::LC_DYSYMTAB) {<br>
// Multiple dynamic symbol tables<br>
if (DysymtabLoadCmd) {<br>
- EC = object_error::parse_failed;<br>
+ Err = malformedError(*this, "Multiple dynamic symbol tables");<br>
return;<br>
}<br>
DysymtabLoadCmd = Load.Ptr;<br>
} else if (Load.C.cmd == MachO::LC_DATA_IN_CODE) {<br>
// Multiple data in code tables<br>
if (DataInCodeLoadCmd) {<br>
- EC = object_error::parse_failed;<br>
+ Err = malformedError(*this, "Multiple data-in-code tables");<br>
return;<br>
}<br>
DataInCodeLoadCmd = Load.Ptr;<br>
} else if (Load.C.cmd == MachO::LC_LINKER_OPTIMIZATION_HINT) {<br>
// Multiple linker optimization hint tables<br>
if (LinkOptHintsLoadCmd) {<br>
- EC = object_error::parse_failed;<br>
+ Err = malformedError(*this, "Multiple linker optimization hint tables");<br>
return;<br>
}<br>
LinkOptHintsLoadCmd = Load.Ptr;<br>
@@ -282,24 +308,24 @@ MachOObjectFile::MachOObjectFile(MemoryB<br>
Load.C.cmd == MachO::LC_DYLD_INFO_ONLY) {<br>
// Multiple dyldinfo load commands<br>
if (DyldInfoLoadCmd) {<br>
- EC = object_error::parse_failed;<br>
+ Err = malformedError(*this, "Multiple dyldinfo load commands");<br>
return;<br>
}<br>
DyldInfoLoadCmd = Load.Ptr;<br>
} else if (Load.C.cmd == MachO::LC_UUID) {<br>
// Multiple UUID load commands<br>
if (UuidLoadCmd) {<br>
- EC = object_error::parse_failed;<br>
+ Err = malformedError(*this, "Multiple UUID load commands");<br>
return;<br>
}<br>
UuidLoadCmd = Load.Ptr;<br>
} else if (Load.C.cmd == MachO::LC_SEGMENT_64) {<br>
- if ((EC = parseSegmentLoadCommand<MachO::segment_command_64>(<br>
- this, Load, Sections, HasPageZeroSegment)))<br>
+ if ((Err = parseSegmentLoadCommand<MachO::segment_command_64>(<br>
+ this, Load, Sections, HasPageZeroSegment)))<br>
return;<br>
} else if (Load.C.cmd == MachO::LC_SEGMENT) {<br>
- if ((EC = parseSegmentLoadCommand<MachO::segment_command>(<br>
- this, Load, Sections, HasPageZeroSegment)))<br>
+ if ((Err = parseSegmentLoadCommand<MachO::segment_command>(<br>
+ this, Load, Sections, HasPageZeroSegment)))<br>
return;<br>
} else if (Load.C.cmd == MachO::LC_LOAD_DYLIB ||<br>
Load.C.cmd == MachO::LC_LOAD_WEAK_DYLIB ||<br>
@@ -309,19 +335,20 @@ MachOObjectFile::MachOObjectFile(MemoryB<br>
Libraries.push_back(Load.Ptr);<br>
}<br>
if (I < LoadCommandCount - 1) {<br>
- auto LoadOrErr = getNextLoadCommandInfo(this, Load);<br>
- if (!LoadOrErr) {<br>
- EC = LoadOrErr.getError();<br>
+ if (auto LoadOrErr = getNextLoadCommandInfo(this, Load))<br>
+ Load = *LoadOrErr;<br>
+ else {<br>
+ Err = LoadOrErr.takeError();<br>
return;<br>
}<br>
- Load = LoadOrErr.get();<br>
}<br>
}<br>
if (!SymtabLoadCmd) {<br>
if (DysymtabLoadCmd) {<br>
- // Diagnostic("truncated or malformed object (contains LC_DYSYMTAB load "<br>
- // "command without a LC_SYMTAB load command)");<br>
- EC = object_error::parse_failed;<br>
+ Err = malformedError(*this,<br>
+ "truncated or malformed object (contains "<br>
+ "LC_DYSYMTAB load command without a LC_SYMTAB load "<br>
+ "command)");<br>
return;<br>
}<br>
} else if (DysymtabLoadCmd) {</blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
@@ -330,49 +357,57 @@ MachOObjectFile::MachOObjectFile(MemoryB<br>
MachO::dysymtab_command Dysymtab =<br>
getStruct<MachO::dysymtab_command>(this, DysymtabLoadCmd);<br>
if (Dysymtab.nlocalsym != 0 && Dysymtab.ilocalsym > Symtab.nsyms) {<br>
- // Diagnostic("truncated or malformed object (ilocalsym in LC_DYSYMTAB "<br>
- // "load command extends past the end of the symbol table)"<br>
- EC = object_error::parse_failed;<br>
+ Err = malformedError(*this,<br>
+ "truncated or malformed object (iolocalsym in "<br>
+ "LC_DYSYMTAB load command extends past the end of "<br>
+ "the symbol table)");<br>
return;<br>
}<br>
uint64_t big_size = Dysymtab.ilocalsym;<br>
big_size += Dysymtab.nlocalsym;<br>
if (Dysymtab.nlocalsym != 0 && big_size > Symtab.nsyms) {<br>
- // Diagnostic("truncated or malformed object (ilocalsym plus nlocalsym "<br>
- // "in LC_DYSYMTAB load command extends past the end of the symbol table)"<br>
- EC = object_error::parse_failed;<br>
+ Err = malformedError(*this,<br>
+ "truncated or malformed object (ilocalsym plus "<br>
+ "nlocalsym in LC_DYSYMTAB load command extends past "<br>
+ "the end of the symbol table)");<br>
return;<br>
}<br>
if (Dysymtab.nextdefsym != 0 && Dysymtab.ilocalsym > Symtab.nsyms) {<br>
- // Diagnostic("truncated or malformed object (nextdefsym in LC_DYSYMTAB "<br>
- // "load command extends past the end of the symbol table)"<br>
- EC = object_error::parse_failed;<br>
+ Err = malformedError(*this,<br>
+ "truncated or malformed object (nextdefsym in "<br>
+ "LC_DYSYMTAB load command extends past the end of "<br>
+ "the symbol table)");<br>
return;<br>
}<br>
big_size = Dysymtab.iextdefsym;<br>
big_size += Dysymtab.nextdefsym;<br>
if (Dysymtab.nextdefsym != 0 && big_size > Symtab.nsyms) {<br>
- // Diagnostic("truncated or malformed object (iextdefsym plus nextdefsym "<br>
- // "in LC_DYSYMTAB load command extends past the end of the symbol table)"<br>
- EC = object_error::parse_failed;<br>
+ Err = malformedError(*this,<br>
+ "truncated or malformed object (iextdefsym plus "<br>
+ "nextdefsym in LC_DYSYMTAB load command extends "<br>
+ "past the end of the symbol table)");<br>
return;<br>
}<br>
if (Dysymtab.nundefsym != 0 && Dysymtab.iundefsym > Symtab.nsyms) {<br>
- // Diagnostic("truncated or malformed object (nundefsym in LC_DYSYMTAB "<br>
- // "load command extends past the end of the symbol table)"<br>
- EC = object_error::parse_failed;<br>
+ Err = malformedError(*this,<br>
+ "truncated or malformed object (nundefsym in "<br>
+ "LC_DYSYMTAB load command extends past the end of "<br>
+ "the symbol table)");<br>
return;<br>
}<br>
big_size = Dysymtab.iundefsym;<br>
big_size += Dysymtab.nundefsym;<br>
if (Dysymtab.nundefsym != 0 && big_size > Symtab.nsyms) {<br>
- // Diagnostic("truncated or malformed object (iundefsym plus nundefsym "<br>
- // "in LC_DYSYMTAB load command extends past the end of the symbol table)"<br>
- EC = object_error::parse_failed;<br>
+ Err = malformedError(*this,<br>
+ "truncated or malformed object (iundefsym plus "<br>
+ "nundefsym in LC_DYSYMTAB load command extends past "<br>
+ "the end of the symbol table");<br>
return;<br>
}<br>
}<br>
assert(LoadCommands.size() == LoadCommandCount);<br>
+<br>
+ Err = Error::success();<br>
}<br>
<br>
void MachOObjectFile::moveSymbolNext(DataRefImpl &Symb) const {<br>
@@ -2381,20 +2416,29 @@ bool MachOObjectFile::isRelocatableObjec<br>
ErrorOr<std::unique_ptr<MachOObjectFile>><br>
ObjectFile::createMachOObjectFile(MemoryBufferRef Buffer) {<br>
StringRef Magic = Buffer.getBuffer().slice(0, 4);<br>
- std::error_code EC;<br>
std::unique_ptr<MachOObjectFile> Ret;<br>
- if (Magic == "\xFE\xED\xFA\xCE")<br>
- Ret.reset(new MachOObjectFile(Buffer, false, false, EC));<br>
- else if (Magic == "\xCE\xFA\xED\xFE")<br>
- Ret.reset(new MachOObjectFile(Buffer, true, false, EC));<br>
- else if (Magic == "\xFE\xED\xFA\xCF")<br>
- Ret.reset(new MachOObjectFile(Buffer, false, true, EC));<br>
- else if (Magic == "\xCF\xFA\xED\xFE")<br>
- Ret.reset(new MachOObjectFile(Buffer, true, true, EC));<br>
- else<br>
+ if (Magic == "\xFE\xED\xFA\xCE") {<br>
+ Error Err;<br>
+ Ret.reset(new MachOObjectFile(Buffer, false, false, Err));<br></blockquote><div><br></div><div>I would generally favor MakeUnique, but I see this is existing code (& perhaps the ctor is private/protected/somesuch)<br><br>Why the change to put the error checking inside the ifs here - it seems verbose/unfortunate?<br><br>(I guess if you put it outside then you end up prematurely raising the "checked" flag on the error before it's returned? Is there some way to fix that, API-wise? have a way to check-without-clearing?)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ if (Err)<br>
+ return errorToErrorCode(std::move(Err));<br>
+ } else if (Magic == "\xCE\xFA\xED\xFE") {<br>
+ Error Err;<br>
+ Ret.reset(new MachOObjectFile(Buffer, true, false, Err));<br>
+ if (Err)<br>
+ return errorToErrorCode(std::move(Err));<br>
+ } else if (Magic == "\xFE\xED\xFA\xCF") {<br>
+ Error Err;<br>
+ Ret.reset(new MachOObjectFile(Buffer, false, true, Err));<br>
+ if (Err)<br>
+ return errorToErrorCode(std::move(Err));<br>
+ } else if (Magic == "\xCF\xFA\xED\xFE") {<br>
+ Error Err;<br>
+ Ret.reset(new MachOObjectFile(Buffer, true, true, Err));<br>
+ if (Err)<br>
+ return errorToErrorCode(std::move(Err));<br>
+ } else<br>
return object_error::parse_failed;<br>
<br>
- if (EC)<br>
- return EC;<br>
return std::move(Ret);<br>
}<br>
<br>
Modified: llvm/trunk/tools/llvm-rtdyld/llvm-rtdyld.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-rtdyld/llvm-rtdyld.cpp?rev=264425&r1=264424&r2=264425&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-rtdyld/llvm-rtdyld.cpp?rev=264425&r1=264424&r2=264425&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/tools/llvm-rtdyld/llvm-rtdyld.cpp (original)<br>
+++ llvm/trunk/tools/llvm-rtdyld/llvm-rtdyld.cpp Fri Mar 25 12:25:34 2016<br>
@@ -254,9 +254,9 @@ uint8_t *TrivialMemoryManager::allocateD<br>
<br>
static const char *ProgramName;<br>
<br>
-static int Error(const Twine &Msg) {<br>
+static int ErrorAndExit(const Twine &Msg) {<br>
errs() << ProgramName << ": error: " << Msg << "\n";<br>
- return 1;<br>
+ exit(1);<br>
}<br>
<br>
static void loadDylibs() {<br>
@@ -290,13 +290,13 @@ static int printLineInfoForInput(bool Lo<br>
ErrorOr<std::unique_ptr<MemoryBuffer>> InputBuffer =<br>
MemoryBuffer::getFileOrSTDIN(File);<br>
if (std::error_code EC = InputBuffer.getError())<br>
- return Error("unable to read input: '" + EC.message() + "'");<br>
+ ErrorAndExit("unable to read input: '" + EC.message() + "'");<br>
<br>
ErrorOr<std::unique_ptr<ObjectFile>> MaybeObj(<br>
ObjectFile::createObjectFile((*InputBuffer)->getMemBufferRef()));<br>
<br>
if (std::error_code EC = MaybeObj.getError())<br>
- return Error("unable to create object file: '" + EC.message() + "'");<br>
+ ErrorAndExit("unable to create object file: '" + EC.message() + "'");<br>
<br>
ObjectFile &Obj = **MaybeObj;<br>
<br>
@@ -309,7 +309,7 @@ static int printLineInfoForInput(bool Lo<br>
Dyld.loadObject(Obj);<br>
<br>
if (Dyld.hasError())<br>
- return Error(Dyld.getErrorString());<br>
+ ErrorAndExit(Dyld.getErrorString());<br>
<br>
// Resolve all the relocations we can.<br>
Dyld.resolveRelocations();<br>
@@ -400,19 +400,19 @@ static int executeInput() {<br>
ErrorOr<std::unique_ptr<MemoryBuffer>> InputBuffer =<br>
MemoryBuffer::getFileOrSTDIN(File);<br>
if (std::error_code EC = InputBuffer.getError())<br>
- return Error("unable to read input: '" + EC.message() + "'");<br>
+ ErrorAndExit("unable to read input: '" + EC.message() + "'");<br>
ErrorOr<std::unique_ptr<ObjectFile>> MaybeObj(<br>
ObjectFile::createObjectFile((*InputBuffer)->getMemBufferRef()));<br>
<br>
if (std::error_code EC = MaybeObj.getError())<br>
- return Error("unable to create object file: '" + EC.message() + "'");<br>
+ ErrorAndExit("unable to create object file: '" + EC.message() + "'");<br>
<br>
ObjectFile &Obj = **MaybeObj;<br>
<br>
// Load the object file<br>
Dyld.loadObject(Obj);<br>
if (Dyld.hasError()) {<br>
- return Error(Dyld.getErrorString());<br>
+ ErrorAndExit(Dyld.getErrorString());<br>
}<br>
}<br>
<br>
@@ -423,7 +423,7 @@ static int executeInput() {<br>
// Get the address of the entry point (_main by default).<br>
void *MainAddress = Dyld.getSymbolLocalAddress(EntryPoint);<br>
if (!MainAddress)<br>
- return Error("no definition for '" + EntryPoint + "'");<br>
+ ErrorAndExit("no definition for '" + EntryPoint + "'");<br>
<br>
// Invalidate the instruction cache for each loaded function.<br>
for (auto &FM : MemMgr.FunctionMemory) {<br>
@@ -432,7 +432,7 @@ static int executeInput() {<br>
// setExecutable will call InvalidateInstructionCache.<br>
std::string ErrorStr;<br>
if (!sys::Memory::setExecutable(FM, &ErrorStr))<br>
- return Error("unable to mark function executable: '" + ErrorStr + "'");<br>
+ ErrorAndExit("unable to mark function executable: '" + ErrorStr + "'");<br>
}<br>
<br>
// Dispatch to _main().<br>
@@ -452,12 +452,12 @@ static int checkAllExpressions(RuntimeDy<br>
ErrorOr<std::unique_ptr<MemoryBuffer>> CheckerFileBuf =<br>
MemoryBuffer::getFileOrSTDIN(CheckerFileName);<br>
if (std::error_code EC = CheckerFileBuf.getError())<br>
- return Error("unable to read input '" + CheckerFileName + "': " +<br>
+ ErrorAndExit("unable to read input '" + CheckerFileName + "': " +<br>
EC.message());<br>
<br>
if (!Checker.checkAllRulesInBuffer("# rtdyld-check:",<br>
CheckerFileBuf.get().get()))<br>
- return Error("some checks in '" + CheckerFileName + "' failed");<br>
+ ErrorAndExit("some checks in '" + CheckerFileName + "' failed");<br>
}<br>
return 0;<br>
}<br>
@@ -606,7 +606,7 @@ static int linkAndVerify() {<br>
<br>
// Check for missing triple.<br>
if (TripleName == "")<br>
- return Error("-triple required when running in -verify mode.");<br>
+ ErrorAndExit("-triple required when running in -verify mode.");<br>
<br>
// Look up the target and build the disassembler.<br>
Triple TheTriple(Triple::normalize(TripleName));<br>
@@ -614,29 +614,29 @@ static int linkAndVerify() {<br>
const Target *TheTarget =<br>
TargetRegistry::lookupTarget("", TheTriple, ErrorStr);<br>
if (!TheTarget)<br>
- return Error("Error accessing target '" + TripleName + "': " + ErrorStr);<br>
+ ErrorAndExit("Error accessing target '" + TripleName + "': " + ErrorStr);<br>
<br>
TripleName = TheTriple.getTriple();<br>
<br>
std::unique_ptr<MCSubtargetInfo> STI(<br>
TheTarget->createMCSubtargetInfo(TripleName, MCPU, ""));<br>
if (!STI)<br>
- return Error("Unable to create subtarget info!");<br>
+ ErrorAndExit("Unable to create subtarget info!");<br>
<br>
std::unique_ptr<MCRegisterInfo> MRI(TheTarget->createMCRegInfo(TripleName));<br>
if (!MRI)<br>
- return Error("Unable to create target register info!");<br>
+ ErrorAndExit("Unable to create target register info!");<br>
<br>
std::unique_ptr<MCAsmInfo> MAI(TheTarget->createMCAsmInfo(*MRI, TripleName));<br>
if (!MAI)<br>
- return Error("Unable to create target asm info!");<br>
+ ErrorAndExit("Unable to create target asm info!");<br>
<br>
MCContext Ctx(MAI.get(), MRI.get(), nullptr);<br>
<br>
std::unique_ptr<MCDisassembler> Disassembler(<br>
TheTarget->createMCDisassembler(*STI, Ctx));<br>
if (!Disassembler)<br>
- return Error("Unable to create disassembler!");<br>
+ ErrorAndExit("Unable to create disassembler!");<br>
<br>
std::unique_ptr<MCInstrInfo> MII(TheTarget->createMCInstrInfo());<br>
<br>
@@ -663,20 +663,20 @@ static int linkAndVerify() {<br>
MemoryBuffer::getFileOrSTDIN(Filename);<br>
<br>
if (std::error_code EC = InputBuffer.getError())<br>
- return Error("unable to read input: '" + EC.message() + "'");<br>
+ ErrorAndExit("unable to read input: '" + EC.message() + "'");<br>
<br>
ErrorOr<std::unique_ptr<ObjectFile>> MaybeObj(<br>
ObjectFile::createObjectFile((*InputBuffer)->getMemBufferRef()));<br>
<br>
if (std::error_code EC = MaybeObj.getError())<br>
- return Error("unable to create object file: '" + EC.message() + "'");<br>
+ ErrorAndExit("unable to create object file: '" + EC.message() + "'");<br>
<br>
ObjectFile &Obj = **MaybeObj;<br>
<br>
// Load the object file<br>
Dyld.loadObject(Obj);<br>
if (Dyld.hasError()) {<br>
- return Error(Dyld.getErrorString());<br>
+ ErrorAndExit(Dyld.getErrorString());<br>
}<br>
}<br>
<br>
@@ -692,7 +692,7 @@ static int linkAndVerify() {<br>
<br>
int ErrorCode = checkAllExpressions(Checker);<br>
if (Dyld.hasError())<br>
- return Error("RTDyld reported an error applying relocations:\n " +<br>
+ ErrorAndExit("RTDyld reported an error applying relocations:\n " +<br>
Dyld.getErrorString());<br>
<br>
return ErrorCode;<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>