<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi David,<div class=""><br class=""></div><div class="">Thanks for your time to take a look at these changes and your thoughtful comments. There were some fixes to this in <span style="font-family: 'Helvetica Neue';" class="">r277223.</span></div><div class=""><font face="Helvetica Neue" class=""><br class=""></font></div><div class=""><font face="Helvetica Neue" class="">I’ve added my thoughts and comments below.</font></div><div class=""><br class=""></div><div class=""><div><blockquote type="cite" class=""><div class="">On Aug 1, 2016, at 10:32 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Fri, Jul 29, 2016 at 10:51 AM Kevin Enderby via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: enderby<br class="">
Date: Fri Jul 29 12:44:13 2016<br class="">
New Revision: 277177<br class="">
<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=277177&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=277177&view=rev</a><br class="">
Log:<br class="">
The next step along the way to getting good error messages for bad archives.<br class="">
<br class="">
As mentioned in commit log for r276686 this next step is adding a new<br class="">
method in the ArchiveMemberHeader class to get the full name that<br class="">
does proper error checking, and can be use for error messages.<br class="">
<br class="">
To do this the name of ArchiveMemberHeader::getName() is changed to<br class="">
ArchiveMemberHeader::getRawName() to be consistent with<br class="">
Archive::Child::getRawName(). Then the “new” method is the addition<br class="">
of a new implementation of ArchiveMemberHeader::getName() which gets<br class="">
the full name and provides proper error checking. Which is mostly a rewrite<br class="">
of what was Archive::Child::getName() and cleaning up incorrect uses of<br class="">
llvm_unreachable() in the code which were actually just cases of errors<br class="">
in the input Archives.<br class="">
<br class="">
Then Archive::Child::getName() is changed to return Expected<> and use<br class="">
the new implementation of ArchiveMemberHeader::getName() .<br class="">
<br class="">
Also needed to change Archive::getMemoryBufferRef() with these<br class="">
changes to return Expected<> as well to propagate Errors up.<br class="">
As well as changing Archive::isThinMember() to return Expected<> .<br class="">
<br class="">
Added:<br class="">
llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus10.a<br class="">
llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus6.a (with props)<br class="">
llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus7.a<br class="">
llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus8.a<br class="">
llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus9.a<br class="">
Modified:<br class="">
llvm/trunk/include/llvm/Object/Archive.h<br class="">
llvm/trunk/lib/Object/Archive.cpp<br class="">
llvm/trunk/lib/Object/ArchiveWriter.cpp<br class="">
llvm/trunk/test/tools/llvm-objdump/malformed-archives.test<br class="">
llvm/trunk/tools/dsymutil/BinaryHolder.cpp<br class="">
llvm/trunk/tools/llvm-ar/llvm-ar.cpp<br class="">
llvm/trunk/tools/llvm-nm/llvm-nm.cpp<br class="">
llvm/trunk/tools/llvm-objdump/MachODump.cpp<br class="">
llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp<br class="">
llvm/trunk/tools/llvm-size/llvm-size.cpp<br class="">
<br class="">
Modified: llvm/trunk/include/llvm/Object/Archive.h<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/Archive.h?rev=277177&r1=277176&r2=277177&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/Archive.h?rev=277177&r1=277176&r2=277177&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/include/llvm/Object/Archive.h (original)<br class="">
+++ llvm/trunk/include/llvm/Object/Archive.h Fri Jul 29 12:44:13 2016<br class="">
@@ -36,7 +36,10 @@ public:<br class="">
// ArchiveMemberHeader() = default;<br class="">
<br class="">
/// Get the name without looking up long names.<br class="">
- llvm::StringRef getName() const;<br class="">
+ Expected<llvm::StringRef> getRawName() const;<br class="">
+<br class="">
+ /// Get the name looking up long names.<br class="">
+ Expected<llvm::StringRef> getName(uint64_t Size) const;<br class="">
<br class="">
/// Members are not larger than 4GB.<br class="">
Expected<uint32_t> getSize() const;<br class="">
@@ -82,7 +85,7 @@ public:<br class="">
/// \brief Offset from Data to the start of the file.<br class="">
uint16_t StartOfFile;<br class="">
<br class="">
- bool isThinMember() const;<br class="">
+ Expected<bool> isThinMember() const;<br class="">
<br class="">
public:<br class="">
Child(const Archive *Parent, const char *Start, Error *Err);<br class="">
@@ -96,9 +99,9 @@ public:<br class="">
const Archive *getParent() const { return Parent; }<br class="">
Expected<Child> getNext() const;<br class="">
<br class="">
- ErrorOr<StringRef> getName() const;<br class="">
+ Expected<StringRef> getName() const;<br class="">
ErrorOr<std::string> getFullName() const;<br class="">
- StringRef getRawName() const { return Header.getName(); }<br class="">
+ Expected<StringRef> getRawName() const { return Header.getRawName(); }<br class="">
sys::TimeValue getLastModified() const {<br class="">
return Header.getLastModified();<br class="">
}<br class="">
@@ -118,7 +121,7 @@ public:<br class="">
ErrorOr<StringRef> getBuffer() const;<br class="">
uint64_t getChildOffset() const;<br class="">
<br class="">
- ErrorOr<MemoryBufferRef> getMemoryBufferRef() const;<br class="">
+ Expected<MemoryBufferRef> getMemoryBufferRef() const;<br class="">
<br class="">
Expected<std::unique_ptr<Binary>><br class="">
getAsBinary(LLVMContext *Context = nullptr) const;<br class="">
@@ -238,6 +241,7 @@ public:<br class="">
<br class="">
bool hasSymbolTable() const;<br class="">
StringRef getSymbolTable() const { return SymbolTable; }<br class="">
+ StringRef getStringTable() const { return StringTable; }<br class="">
uint32_t getNumberOfSymbols() const;<br class="">
<br class="">
std::vector<std::unique_ptr<MemoryBuffer>> takeThinBuffers() {<br class="">
<br class="">
Modified: llvm/trunk/lib/Object/Archive.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/Archive.cpp?rev=277177&r1=277176&r2=277177&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/Archive.cpp?rev=277177&r1=277176&r2=277177&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/lib/Object/Archive.cpp (original)<br class="">
+++ llvm/trunk/lib/Object/Archive.cpp Fri Jul 29 12:44:13 2016<br class="">
@@ -43,17 +43,17 @@ ArchiveMemberHeader::ArchiveMemberHeader<br class="">
return;<br class="">
ErrorAsOutParameter ErrAsOutParam(Err);<br class="">
<br class="">
- // TODO: For errors messages with the ArchiveMemberHeader class use the<br class="">
- // archive member name instead of the the offset to the archive member header.<br class="">
- // When there is also error getting the member name then use the offset to<br class="">
- // the member in the message.<br class="">
-<br class="">
if (Size < sizeof(ArMemHdrType)) {<br class="">
if (Err) {<br class="">
- uint64_t Offset = RawHeaderPtr - Parent->getData().data();<br class="">
- *Err = malformedError("remaining size of archive too small for next "<br class="">
- "archive member header at offset " +<br class="">
- Twine(Offset));<br class="">
+ Twine Msg("remaining size of archive too small for next archive member "<br class="">
+ "header ");<br class="">
+ Expected<StringRef> NameOrErr = getName(Size);<br class="">
+ if (!NameOrErr) {<br class="">
+ consumeError(NameOrErr.takeError());<br class="">
+ uint64_t Offset = RawHeaderPtr - Parent->getData().data();<br class="">
+ *Err = malformedError(Msg + "at offset " + Twine(Offset));<br class="">
+ } else<br class="">
+ *Err = malformedError(Msg + "for " + Twine(NameOrErr.get()));<br class=""></blockquote><div class=""><br class=""></div><div class="">If you like, this code might be a bit tidier by rolling in the initialization into the conditional:<br class=""><br class="">if (Expected<StringRef> NameOrErr = getName(Size))<br class=""> *Err = malformedError(...)<br class="">else {<br class=""> consumeError(...)<br class=""> …</div></div></div></div></blockquote><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class="">}<br class=""></div></div></div></div></blockquote><div><br class=""></div>I can see that. But being a bit more old school I like to see definitions and initializations on their own line. But it does seems like lots of code in llvm puts the definitions and initialization into the conditional. I personally find it a bit harder to read. But happy to change to that.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""><br class="">Also, not sure, but this code might be a bit easier less indented:<br class=""><br class="">if (Size < sizeof....) {<br class=""> if (!Err)<br class=""> return;<br class=""> /* populate *Err */<br class=""> return;<br class="">}<br class=""><br class="">It's the same number of lines, but reduces indentation (but does duplicate the return which might make it harder to read - not sure)<br class=""></div></div></div></div></blockquote><div><br class=""></div>Yep I can see doing that either way. And your way does the early return which seems to be more what llvm likes. Seems a bit harder to read to me as I like the populating an Err after an "if (Err)” when the Error is optional.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class="">Also "Twine(NameOrErr.get())" doesn't need the Twine() wrapper</div></div></div></div></blockquote><div><br class=""></div>Yep. And that was fixed with the follow on changes in <span style="font-family: 'Helvetica Neue';" class="">r277223.</span></div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""> and you can use op* rather than .get() if you like (this applies to all uses of Expected::get in this patch/generally), so you can rewrite it as "*NameOrErr”.<br class=""></div></div></div></div></blockquote><div><br class=""></div>Yep I have seen that done both ways. I find the overloading the op* vs op.get() to be much harder to read especially when there are pointers in the body of code. When I see a .get() I read that quicker as to what is going on. But again I’m a bit old school, but would be happy to use the op* if that is what people prefer.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""><br class="">(all points similarly below)<br class=""></div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
}<br class="">
return;<br class="">
}<br class="">
@@ -64,18 +64,35 @@ ArchiveMemberHeader::ArchiveMemberHeader<br class="">
OS.write_escaped(llvm::StringRef(ArMemHdr->Terminator,<br class="">
sizeof(ArMemHdr->Terminator)));<br class="">
OS.flush();<br class="">
- uint64_t Offset = RawHeaderPtr - Parent->getData().data();<br class="">
- *Err = malformedError("terminator characters in archive member \"" + Buf +<br class="">
- "\" not the correct \"`\\n\" values for the "<br class="">
- "archive member header at offset " + Twine(Offset));<br class="">
+ Twine Msg("terminator characters in archive member \"" + Buf + "\" not "<br class="">
+ "the correct \"`\\n\" values for the archive member header ");<br class="">
+ Expected<StringRef> NameOrErr = getName(Size);<br class="">
+ if (!NameOrErr) {<br class="">
+ consumeError(NameOrErr.takeError());<br class="">
+ uint64_t Offset = RawHeaderPtr - Parent->getData().data();<br class="">
+ *Err = malformedError(Msg + "at offset " + Twine(Offset));<br class="">
+ } else<br class="">
+ *Err = malformedError(Msg + "for " + Twine(NameOrErr.get()));<br class="">
}<br class="">
return;<br class="">
}<br class="">
}<br class="">
<br class="">
-StringRef ArchiveMemberHeader::getName() const {<br class="">
+// This gets the raw name from the ArMemHdr->Name field and checks that it is<br class="">
+// valid for the kind of archive. If it is not valid it returns an Error.<br class="">
+Expected<StringRef> ArchiveMemberHeader::getRawName() const {<br class="">
char EndCond;<br class="">
- if (ArMemHdr->Name[0] == '/' || ArMemHdr->Name[0] == '#')<br class="">
+ auto Kind = Parent->kind();<br class="">
+ if (Kind == Archive::K_BSD || Kind == Archive::K_DARWIN64) {<br class="">
+ if (ArMemHdr->Name[0] == ' ') {<br class="">
+ uint64_t Offset = reinterpret_cast<const char *>(ArMemHdr) -<br class="">
+ Parent->getData().data();<br class="">
+ return malformedError("name contains a leading space for archive member "<br class="">
+ "header at offset " + Twine(Offset));<br class="">
+ }<br class="">
+ EndCond = ' ';<br class="">
+ }<br class="">
+ else if (ArMemHdr->Name[0] == '/' || ArMemHdr->Name[0] == '#')<br class="">
EndCond = ' ';<br class="">
else<br class="">
EndCond = '/';<br class="">
@@ -88,6 +105,105 @@ StringRef ArchiveMemberHeader::getName()<br class="">
return llvm::StringRef(ArMemHdr->Name, end);<br class="">
}<br class="">
<br class="">
+// This gets the name looking up long names. Size is the size of the archive<br class="">
+// member including the header, so the size of any name following the header<br class="">
+// is checked to make sure it does not overflow.<br class="">
+Expected<StringRef> ArchiveMemberHeader::getName(uint64_t Size) const {<br class="">
+<br class="">
+ // This can be called from the ArchiveMemberHeader constructor when the<br class="">
+ // archive header is truncated to produce an error message with the name.<br class="">
+ // Make sure the name field is not truncated.<br class="">
+ if (Size < offsetof(ArMemHdrType, Name) + sizeof(ArMemHdr->Name)) {<br class="">
+ uint64_t ArchiveOffset = reinterpret_cast<const char *>(ArMemHdr) -<br class="">
+ Parent->getData().data();<br class="">
+ return malformedError("archive header truncated before the name field "<br class="">
+ "for archive member header at offset " +<br class="">
+ Twine(ArchiveOffset));<br class="">
+ }<br class="">
+<br class="">
+ // The raw name itself can be invalid.<br class="">
+ Expected<StringRef> NameOrErr = getRawName();<br class="">
+ if (!NameOrErr)<br class="">
+ return NameOrErr.takeError();<br class="">
+ StringRef Name = NameOrErr.get();<br class="">
+<br class="">
+ // Check if it's a special name.<br class="">
+ if (Name[0] == '/') {<br class="">
+ if (Name.size() == 1) // Linker member.<br class="">
+ return Name;<br class="">
+ if (Name.size() == 2 && Name[1] == '/') // String table.<br class="">
+ return Name;<br class="">
+ // It's a long name.<br class="">
+ // Get the string table offset.<br class="">
+ std::size_t StringOffset;<br class="">
+ if (Name.substr(1).rtrim(' ').getAsInteger(10, StringOffset)) {<br class="">
+ std::string Buf;<br class="">
+ raw_string_ostream OS(Buf);<br class="">
+ OS.write_escaped(Name.substr(1).rtrim(' '),<br class="">
+ sizeof(Name.substr(1)).rtrim(' '));<br class="">
+ OS.flush();<br class="">
+ uint64_t ArchiveOffset = reinterpret_cast<const char *>(ArMemHdr) -<br class="">
+ Parent->getData().data();<br class="">
+ return malformedError("long name offset characters after the '/' are "<br class="">
+ "not all decimal numbers: '" + Buf + "' for "<br class="">
+ "archive member header at offset " +<br class="">
+ Twine(ArchiveOffset));<br class="">
+ }<br class="">
+<br class="">
+ // Verify it.<br class="">
+ if (StringOffset >= Parent->getStringTable().size()) {<br class="">
+ uint64_t ArchiveOffset = reinterpret_cast<const char *>(ArMemHdr) -<br class="">
+ Parent->getData().data();<br class="">
+ return malformedError("long name offset " + Twine(StringOffset) + " past "<br class="">
+ "the end of the string table for archive member "<br class="">
+ "header at offset " + Twine(ArchiveOffset));<br class="">
+ }<br class="">
+ const char *addr = Parent->getStringTable().begin() + StringOffset;<br class="">
+<br class="">
+ // GNU long file names end with a "/\n".<br class="">
+ if (Parent->kind() == Archive::K_GNU ||<br class="">
+ Parent->kind() == Archive::K_MIPS64) {<br class="">
+ StringRef::size_type End = StringRef(addr).find('\n');<br class="">
+ return StringRef(addr, End - 1);<br class="">
+ }<br class="">
+ return StringRef(addr);<br class=""></blockquote><div class=""><br class=""></div><div class="">"return addr;" should suffice here?</div></div></div></div></blockquote><div><br class=""></div>It might, as it might implicitly convert the const char * to the method’s return type of StringRef. But as noted in the commit log, new implementation of ArchiveMemberHeader::getName() is just a re-write of what was Archive::Child::getName(). And the code in Archive::Child::getName() that was deleted (see below) as exactly as above:</div><div><br class=""></div><div>- return StringRef(addr);</div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ } else if (Name.startswith("#1/")) {<br class=""></blockquote><div class=""><br class=""></div><div class="">Drop the else-after-return <a href="http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return" class="">http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return</a></div></div></div></div></blockquote><div><br class=""></div>Sure I can make that change. But again the code in Archive::Child::getName() that was deleted (again see below) was exactly as above:</div><div><br class=""></div><div>- } else if (name.startswith("#1/")) {</div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ uint64_t NameLength;<br class="">
+ if (Name.substr(3).rtrim(' ').getAsInteger(10, NameLength)) {<br class="">
+ std::string Buf;<br class="">
+ raw_string_ostream OS(Buf);<br class="">
+ OS.write_escaped(Name.substr(3).rtrim(' '),<br class="">
+ sizeof(Name.substr(3)).rtrim(' '));<br class="">
+ OS.flush();<br class="">
+ uint64_t ArchiveOffset = reinterpret_cast<const char *>(ArMemHdr) -<br class="">
+ Parent->getData().data();<br class="">
+ return malformedError("long name length characters after the #1/ are "<br class="">
+ "not all decimal numbers: '" + Buf + "' for "<br class="">
+ "archive member header at offset " +<br class="">
+ Twine(ArchiveOffset));<br class="">
+ }<br class="">
+ if (getSizeOf() + NameLength > Size) {<br class="">
+ uint64_t ArchiveOffset = reinterpret_cast<const char *>(ArMemHdr) -<br class="">
+ Parent->getData().data();<br class="">
+ return malformedError("long name length: " + Twine(NameLength) +<br class="">
+ " extends past the end of the member or archive "<br class="">
+ "for archive member header at offset " +<br class="">
+ Twine(ArchiveOffset));<br class="">
+ }<br class="">
+ return StringRef(reinterpret_cast<const char *>(ArMemHdr) + getSizeOf(),<br class="">
+ NameLength).rtrim('\0');<br class="">
+ } else {<br class=""></blockquote><div class=""><br class="">Another else-after-return<br class=""></div></div></div></div></blockquote><div><br class=""></div>Again from the original code (see below), just re-written:</div><div><br class=""></div><div>- return Data.substr(Header.getSizeOf(), name_size).rtrim('\0');<br class="">- } else {</div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ // It is not a long name so trim the blanks at the end of the name.<br class="">
+ if (Name[Name.size() - 1] != '/') {</blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ return Name.rtrim(' ');<br class="">
+ }<br class=""></blockquote><div class=""><br class="">Probably drop the {} on a single line statement.<br class=""></div></div></div></div></blockquote><div><br class=""></div>And another from the original code (see below):</div><div><br class=""></div><div>- // It is not a long name so trim the blanks at the end of the name.<br class="">- if (name[name.size() - 1] != '/') {<br class="">- return name.rtrim(' ');<br class="">- }</div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ }<br class="">
+ // It's a simple name.<br class="">
+ if (Name[Name.size() - 1] == '/')<br class="">
+ return Name.substr(0, Name.size() - 1);<br class="">
+ return Name;<br class="">
+}<br class="">
+<br class="">
Expected<uint32_t> ArchiveMemberHeader::getSize() const {<br class="">
uint32_t Ret;<br class="">
if (llvm::StringRef(ArMemHdr->Size,<br class="">
@@ -166,7 +282,14 @@ Archive::Child::Child(const Archive *Par<br class="">
<br class="">
uint64_t Size = Header.getSizeOf();<br class="">
Data = StringRef(Start, Size);<br class="">
- if (!isThinMember()) {<br class="">
+ Expected<bool> isThinOrErr = isThinMember();<br class="">
+ if (!isThinOrErr) {<br class="">
+ if (Err)<br class="">
+ *Err = isThinOrErr.takeError();<br class="">
+ return;<br class="">
+ }<br class="">
+ bool isThin = isThinOrErr.get();<br class="">
+ if (!isThin) {<br class="">
Expected<uint64_t> MemberSize = getRawSize();<br class="">
if (!MemberSize) {<br class="">
if (Err)<br class="">
@@ -180,11 +303,30 @@ Archive::Child::Child(const Archive *Par<br class="">
// Setup StartOfFile and PaddingBytes.<br class="">
StartOfFile = Header.getSizeOf();<br class="">
// Don't include attached name.<br class="">
- StringRef Name = getRawName();<br class="">
+ Expected<StringRef> NameOrErr = getRawName();<br class="">
+ if (!NameOrErr){<br class="">
+ if (Err)<br class="">
+ *Err = NameOrErr.takeError();<br class="">
+ return;<br class="">
+ }<br class="">
+ StringRef Name = NameOrErr.get();<br class="">
if (Name.startswith("#1/")) {<br class="">
uint64_t NameSize;<br class="">
- if (Name.substr(3).rtrim(' ').getAsInteger(10, NameSize))<br class="">
- llvm_unreachable("Long name length is not an integer");<br class="">
+ if (Name.substr(3).rtrim(' ').getAsInteger(10, NameSize)) {<br class="">
+ if (Err) {<br class="">
+ std::string Buf;<br class="">
+ raw_string_ostream OS(Buf);<br class="">
+ OS.write_escaped(Name.substr(3).rtrim(' '),<br class="">
+ sizeof(Name.substr(3)).rtrim(' '));<br class="">
+ OS.flush();<br class="">
+ uint64_t Offset = Start - Parent->getData().data();<br class="">
+ *Err = malformedError("long name length characters after the #1/ are "<br class="">
+ "not all decimal numbers: '" + Buf + "' for "<br class="">
+ "archive member header at offset " +<br class="">
+ Twine(Offset));<br class="">
+ return;<br class=""></blockquote><div class=""><br class=""></div><div class="">Is this return meant to be outside the conditional like the other similar cases you've added? (ie: the 'if (Name... )' is presumably the failure test - then conditionally populate the Err if it was passed, then exit regardless)</div></div></div></div></blockquote><div><br class=""></div>I can see that. But I have really no idea where this is called when the Err is a nullptr and what it expects when there is an error. In the old code it would have called llvm_unreachable() regardless if Err is a nullptr or not. Then would fall through (when not compiled for DEBUG) to the line of code below to increment StartOfFile with then uninitialized value of NameSize. So I was trying to leave that logical path unchanged as it was as I don’t understand how it is used by its callers.</div><div><br class=""></div><div>I just spoke to Lang Hames on this and his thoughts are that constructor for Archive::Child::Child() should have an assert at the top such that unless Parent an Start are both nullptr then Err can’t be nullptr. Then the code above, and throughout the Archive::Child::Child() constructor can assume Err is not nullptr and such tests are removed. What do you and others think about that approach?</div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ }<br class="">
+ }<br class="">
StartOfFile += NameSize;<br class="">
}<br class="">
}<br class="">
@@ -203,16 +345,22 @@ Expected<uint64_t> Archive::Child::getRa<br class="">
return Header.getSize();<br class="">
}<br class="">
<br class="">
-bool Archive::Child::isThinMember() const {<br class="">
- StringRef Name = Header.getName();<br class="">
+Expected<bool> Archive::Child::isThinMember() const {<br class="">
+ Expected<StringRef> NameOrErr = Header.getRawName();<br class="">
+ if (!NameOrErr)<br class="">
+ return NameOrErr.takeError();<br class="">
+ StringRef Name = NameOrErr.get();<br class="">
return Parent->IsThin && Name != "/" && Name != "//";<br class="">
}<br class="">
<br class="">
ErrorOr<std::string> Archive::Child::getFullName() const {<br class="">
- assert(isThinMember());<br class="">
- ErrorOr<StringRef> NameOrErr = getName();<br class="">
- if (std::error_code EC = NameOrErr.getError())<br class="">
- return EC;<br class="">
+ Expected<bool> isThin = isThinMember();<br class="">
+ if (!isThin)<br class="">
+ return errorToErrorCode(isThin.takeError());<br class="">
+ assert(isThin.get());<span style="line-height:1.5" class=""> </span></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ Expected<StringRef> NameOrErr = getName();<br class="">
+ if (!NameOrErr)<br class="">
+ return errorToErrorCode(NameOrErr.takeError());<br class="">
StringRef Name = *NameOrErr;<br class="">
if (sys::path::is_absolute(Name))<br class="">
return Name;<br class="">
@@ -224,7 +372,11 @@ ErrorOr<std::string> Archive::Child::get<br class="">
}<br class="">
<br class="">
ErrorOr<StringRef> Archive::Child::getBuffer() const {<br class="">
- if (!isThinMember()) {<br class="">
+ Expected<bool> isThinOrErr = isThinMember();<br class="">
+ if (!isThinOrErr)<br class="">
+ return errorToErrorCode(isThinOrErr.takeError());<br class="">
+ bool isThin = isThinOrErr.get();<br class="">
+ if (!isThin) {<br class="">
Expected<uint32_t> Size = getSize();<br class="">
if (!Size)<br class="">
return errorToErrorCode(Size.takeError());<br class="">
@@ -257,8 +409,9 @@ Expected<Archive::Child> Archive::Child:<br class="">
if (NextLoc > Parent->Data.getBufferEnd()) {<br class="">
Twine Msg("offset to next archive member past the end of the archive after "<br class="">
"member ");<br class="">
- ErrorOr<StringRef> NameOrErr = getName();<br class="">
- if (NameOrErr.getError()) {<br class="">
+ Expected<StringRef> NameOrErr = getName();<br class="">
+ if (!NameOrErr) {<br class="">
+ consumeError(NameOrErr.takeError());<br class="">
uint64_t Offset = Data.data() - Parent->getData().data();<br class="">
return malformedError(Msg + "at offset " + Twine(Offset));<br class="">
} else<br class="">
@@ -279,64 +432,34 @@ uint64_t Archive::Child::getChildOffset(<br class="">
return offset;<br class="">
}<br class="">
<br class="">
-ErrorOr<StringRef> Archive::Child::getName() const {<br class="">
- StringRef name = getRawName();<br class="">
- // Check if it's a special name.<br class="">
- if (name[0] == '/') {<br class="">
- if (name.size() == 1) // Linker member.<br class="">
- return name;<br class="">
- if (name.size() == 2 && name[1] == '/') // String table.<br class="">
- return name;<br class="">
- // It's a long name.<br class="">
- // Get the offset.<br class="">
- std::size_t offset;<br class="">
- if (name.substr(1).rtrim(' ').getAsInteger(10, offset))<br class="">
- llvm_unreachable("Long name offset is not an integer");<br class="">
-<br class="">
- // Verify it.<br class="">
- if (offset >= Parent->StringTable.size())<br class="">
- return object_error::parse_failed;<br class="">
- const char *addr = Parent->StringTable.begin() + offset;<br class="">
-<br class="">
- // GNU long file names end with a "/\n".<br class="">
- if (Parent->kind() == K_GNU || Parent->kind() == K_MIPS64) {<br class="">
- StringRef::size_type End = StringRef(addr).find('\n');<br class="">
- return StringRef(addr, End - 1);<br class="">
- }<br class="">
- return StringRef(addr);<br class="">
- } else if (name.startswith("#1/")) {<br class="">
- uint64_t name_size;<br class="">
- if (name.substr(3).rtrim(' ').getAsInteger(10, name_size))<br class="">
- llvm_unreachable("Long name length is not an ingeter");<br class="">
- return Data.substr(Header.getSizeOf(), name_size).rtrim('\0');<br class="">
- } else {<br class="">
- // It is not a long name so trim the blanks at the end of the name.<br class="">
- if (name[name.size() - 1] != '/') {<br class="">
- return name.rtrim(' ');<br class="">
- }<br class="">
- }<br class="">
- // It's a simple name.<br class="">
- if (name[name.size() - 1] == '/')<br class="">
- return name.substr(0, name.size() - 1);<br class="">
- return name;<br class="">
+Expected<StringRef> Archive::Child::getName() const {<br class="">
+ Expected<uint64_t> RawSizeOrErr = getRawSize();<br class="">
+ if (!RawSizeOrErr)<br class="">
+ return RawSizeOrErr.takeError();<br class="">
+ uint64_t RawSize = RawSizeOrErr.get();<br class="">
+ Expected<StringRef> NameOrErr = Header.getName(Header.getSizeOf() + RawSize);<br class="">
+ if (!NameOrErr)<br class="">
+ return NameOrErr.takeError();<br class="">
+ StringRef Name = NameOrErr.get();<br class="">
+ return Name;<br class="">
}<br class="">
<br class="">
-ErrorOr<MemoryBufferRef> Archive::Child::getMemoryBufferRef() const {<br class="">
- ErrorOr<StringRef> NameOrErr = getName();<br class="">
- if (std::error_code EC = NameOrErr.getError())<br class="">
- return EC;<br class="">
+Expected<MemoryBufferRef> Archive::Child::getMemoryBufferRef() const {<br class="">
+ Expected<StringRef> NameOrErr = getName();<br class="">
+ if (!NameOrErr)<br class="">
+ return NameOrErr.takeError();<br class="">
StringRef Name = NameOrErr.get();<br class="">
ErrorOr<StringRef> Buf = getBuffer();<br class="">
if (std::error_code EC = Buf.getError())<br class="">
- return EC;<br class="">
+ return errorCodeToError(EC);<br class="">
return MemoryBufferRef(*Buf, Name);<br class="">
}<br class="">
<br class="">
Expected<std::unique_ptr<Binary>><br class="">
Archive::Child::getAsBinary(LLVMContext *Context) const {<br class="">
- ErrorOr<MemoryBufferRef> BuffOrErr = getMemoryBufferRef();<br class="">
- if (std::error_code EC = BuffOrErr.getError())<br class="">
- return errorCodeToError(EC);<br class="">
+ Expected<MemoryBufferRef> BuffOrErr = getMemoryBufferRef();<br class="">
+ if (!BuffOrErr)<br class="">
+ return BuffOrErr.takeError();<br class="">
<br class="">
auto BinaryOrErr = createBinary(BuffOrErr.get(), Context);<br class="">
if (BinaryOrErr)<br class="">
@@ -372,17 +495,20 @@ Archive::Archive(MemoryBufferRef Source,<br class="">
return;<br class="">
}<br class="">
<br class="">
+ // Make sure Format is initialized before any call to<br class="">
+ // ArchiveMemberHeader::getName() is made. This could be a valid empty<br class="">
+ // archive which is the same in all formats. So claiming it to be gnu to is<br class="">
+ // fine if not totally correct before we look for a string table or table of<br class="">
+ // contents.<br class="">
+ Format = K_GNU;<br class="">
+<br class="">
// Get the special members.<br class="">
child_iterator I = child_begin(Err, false);<br class="">
if (Err)<br class="">
return;<br class="">
child_iterator E = child_end();<br class="">
<br class="">
- // This is at least a valid empty archive. Since an empty archive is the<br class="">
- // same in all formats, just claim it to be gnu to make sure Format is<br class="">
- // initialized.<br class="">
- Format = K_GNU;<br class="">
-<br class="">
+ // See if this is a valid empty archive and if so return.<br class="">
if (I == E) {<br class="">
Err = Error::success();<br class="">
return;<br class="">
@@ -397,7 +523,12 @@ Archive::Archive(MemoryBufferRef Source,<br class="">
return false;<br class="">
};<br class="">
<br class="">
- StringRef Name = C->getRawName();<br class="">
+ Expected<StringRef> NameOrErr = C->getRawName();<br class="">
+ if (!NameOrErr) {<br class="">
+ Err = NameOrErr.takeError();<br class="">
+ return;<br class="">
+ }<br class="">
+ StringRef Name = NameOrErr.get();<br class="">
<br class="">
// Below is the pattern that is used to figure out the archive format<br class="">
// GNU archive format<br class="">
@@ -437,9 +568,9 @@ Archive::Archive(MemoryBufferRef Source,<br class="">
if (Name.startswith("#1/")) {<br class="">
Format = K_BSD;<br class="">
// We know this is BSD, so getName will work since there is no string table.<br class="">
- ErrorOr<StringRef> NameOrErr = C->getName();<br class="">
- if (auto ec = NameOrErr.getError()) {<br class="">
- Err = errorCodeToError(ec);<br class="">
+ Expected<StringRef> NameOrErr = C->getName();<br class="">
+ if (!NameOrErr) {<br class="">
+ Err = NameOrErr.takeError();<br class="">
return;<br class="">
}<br class="">
Name = NameOrErr.get();<br class="">
@@ -481,7 +612,12 @@ Archive::Archive(MemoryBufferRef Source,<br class="">
Err = Error::success();<br class="">
return;<br class="">
}<br class="">
- Name = C->getRawName();<br class="">
+ Expected<StringRef> NameOrErr = C->getRawName();<br class="">
+ if (!NameOrErr) {<br class="">
+ Err = NameOrErr.takeError();<br class="">
+ return;<br class="">
+ }<br class="">
+ Name = NameOrErr.get();<br class="">
}<br class="">
<br class="">
if (Name == "//") {<br class="">
@@ -522,7 +658,12 @@ Archive::Archive(MemoryBufferRef Source,<br class="">
return;<br class="">
}<br class="">
<br class="">
- Name = C->getRawName();<br class="">
+ NameOrErr = C->getRawName();<br class="">
+ if (!NameOrErr) {<br class="">
+ Err = NameOrErr.takeError();<br class="">
+ return;<br class="">
+ }<br class="">
+ Name = NameOrErr.get();<br class="">
<br class="">
if (Name == "//") {<br class="">
// The string table is never an external member, so we just assert on the<br class="">
<br class="">
Modified: llvm/trunk/lib/Object/ArchiveWriter.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/ArchiveWriter.cpp?rev=277177&r1=277176&r2=277177&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/ArchiveWriter.cpp?rev=277177&r1=277176&r2=277177&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/lib/Object/ArchiveWriter.cpp (original)<br class="">
+++ llvm/trunk/lib/Object/ArchiveWriter.cpp Fri Jul 29 12:44:13 2016<br class="">
@@ -40,9 +40,9 @@ NewArchiveMember::NewArchiveMember(Memor<br class="">
Expected<NewArchiveMember><br class="">
NewArchiveMember::getOldMember(const object::Archive::Child &OldMember,<br class="">
bool Deterministic) {<br class="">
- ErrorOr<llvm::MemoryBufferRef> BufOrErr = OldMember.getMemoryBufferRef();<br class="">
+ Expected<llvm::MemoryBufferRef> BufOrErr = OldMember.getMemoryBufferRef();<br class="">
if (!BufOrErr)<br class="">
- return errorCodeToError(BufOrErr.getError());<br class="">
+ return BufOrErr.takeError();<br class="">
<br class="">
NewArchiveMember M;<br class="">
M.Buf = MemoryBuffer::getMemBuffer(*BufOrErr, false);<br class="">
<br class="">
Added: llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus10.a<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus10.a?rev=277177&view=auto" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus10.a?rev=277177&view=auto</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus10.a (added)<br class="">
+++ llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus10.a Fri Jul 29 12:44:13 2016<br class="">
@@ -0,0 +1,13 @@<br class="">
+!<arch><br class="">
+// 26 `<br class="">
+1234567890123456hello.c/<br class="">
+<br class="">
+/507 0 0 0 644 102 `<br class="">
+#include <stdio.h><br class="">
+#include <stdlib.h><br class="">
+int<br class="">
+main()<br class="">
+{<br class="">
+ printf("Hello World\n");<br class="">
+ return EXIT_SUCCESS;<br class="">
+}<br class="">
<br class="">
Added: llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus6.a<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus6.a?rev=277177&view=auto" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus6.a?rev=277177&view=auto</a><br class="">
==============================================================================<br class="">
Binary file - no diff available.<br class="">
<br class="">
Propchange: llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus6.a<br class="">
------------------------------------------------------------------------------<br class="">
svn:mime-type = application/octet-stream<br class="">
<br class="">
Added: llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus7.a<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus7.a?rev=277177&view=auto" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus7.a?rev=277177&view=auto</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus7.a (added)<br class="">
+++ llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus7.a Fri Jul 29 12:44:13 2016<br class="">
@@ -0,0 +1,10 @@<br class="">
+!<arch><br class="">
+#1/@123$ 1469564779 124 0 100644 102 `<br class="">
+#include <stdio.h><br class="">
+#include <stdlib.h><br class="">
+int<br class="">
+main()<br class="">
+{<br class="">
+ printf("Hello World\n");<br class="">
+ return EXIT_SUCCESS;<br class="">
+}<br class="">
<br class="">
Added: llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus8.a<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus8.a?rev=277177&view=auto" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus8.a?rev=277177&view=auto</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus8.a (added)<br class="">
+++ llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus8.a Fri Jul 29 12:44:13 2016<br class="">
@@ -0,0 +1,13 @@<br class="">
+!<arch><br class="">
+foo.c 1444941645 124 0 100644 17 `<br class="">
+void foo(void){}<br class="">
+<br class="">
+#1/1234 1469564779 124 0 100644 126 `<br class="">
+1234567890123456Xhello.c#include <stdio.h><br class="">
+#include <stdlib.h><br class="">
+int<br class="">
+main()<br class="">
+{<br class="">
+ printf("Hello World\n");<br class="">
+ return EXIT_SUCCESS;<br class="">
+}<br class="">
<br class="">
Added: llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus9.a<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus9.a?rev=277177&view=auto" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus9.a?rev=277177&view=auto</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus9.a (added)<br class="">
+++ llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus9.a Fri Jul 29 12:44:13 2016<br class="">
@@ -0,0 +1,13 @@<br class="">
+!<arch><br class="">
+// 26 `<br class="">
+1234567890123456hello.c/<br class="">
+<br class="">
+/&a25* 0 0 0 644 102 `<br class="">
+#include <stdio.h><br class="">
+#include <stdlib.h><br class="">
+int<br class="">
+main()<br class="">
+{<br class="">
+ printf("Hello World\n");<br class="">
+ return EXIT_SUCCESS;<br class="">
+}<br class="">
<br class="">
Modified: llvm/trunk/test/tools/llvm-objdump/malformed-archives.test<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/malformed-archives.test?rev=277177&r1=277176&r2=277177&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/malformed-archives.test?rev=277177&r1=277176&r2=277177&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/test/tools/llvm-objdump/malformed-archives.test (original)<br class="">
+++ llvm/trunk/test/tools/llvm-objdump/malformed-archives.test Fri Jul 29 12:44:13 2016<br class="">
@@ -23,10 +23,38 @@<br class="">
# RUN: %p/Inputs/libbogus4.a \<br class="">
# RUN: 2>&1 | FileCheck -check-prefix=bogus4 %s<br class="">
<br class="">
-# bogus4: libbogus4.a': truncated or malformed archive (remaining size of archive too small for next archive member header at offset 170)<br class="">
+# bogus4: libbogus4.a': truncated or malformed archive (remaining size of archive too small for next archive member header for foo.c)<br class="">
<br class="">
# RUN: not llvm-objdump -macho -archive-headers \<br class="">
# RUN: %p/Inputs/libbogus5.a \<br class="">
# RUN: 2>&1 | FileCheck -check-prefix=bogus5 %s<br class="">
<br class="">
-# bogus5: libbogus5.a': truncated or malformed archive (terminator characters in archive member "@\n" not the correct "`\n" values for the archive member header at offset 8)<br class="">
+# bogus5: libbogus5.a': truncated or malformed archive (terminator characters in archive member "@\n" not the correct "`\n" values for the archive member header for hello.c)<br class="">
+<br class="">
+# RUN: not llvm-objdump -macho -archive-headers \<br class="">
+# RUN: %p/Inputs/libbogus6.a \<br class="">
+# RUN: 2>&1 | FileCheck -check-prefix=bogus6 %s<br class="">
+<br class="">
+# bogus6: libbogus6.a': truncated or malformed archive (name contains a leading space for archive member header at offset 96)<br class="">
+<br class="">
+# RUN: not llvm-objdump -macho -archive-headers \<br class="">
+# RUN: %p/Inputs/libbogus7.a \<br class="">
+# RUN: 2>&1 | FileCheck -check-prefix=bogus7 %s<br class="">
+<br class="">
+# bogus7: libbogus7.a': truncated or malformed archive (long name length characters after the #1/ are not all decimal numbers: '@123$' for archive member header at offset 8)<br class="">
+<br class="">
+# RUN: not llvm-objdump -macho -archive-headers \<br class="">
+# RUN: %p/Inputs/libbogus8.a \<br class="">
+# RUN: 2>&1 | FileCheck -check-prefix=bogus8 %s<br class="">
+<br class="">
+# bogus8: libbogus8.a(???) truncated or malformed archive (long name length: 1234 extends past the end of the member or archive for archive member header at offset 86)<br class="">
+<br class="">
+# RUN: not llvm-objdump -s %p/Inputs/libbogus9.a \<br class="">
+# RUN: 2>&1 | FileCheck -check-prefix=bogus9 %s<br class="">
+<br class="">
+# bogus9: libbogus9.a(???) truncated or malformed archive (long name offset characters after the '/' are not all decimal numbers: '&a25*' for archive member header at offset 94)<br class="">
+<br class="">
+# RUN: not llvm-objdump -s %p/Inputs/libbogus10.a \<br class="">
+# RUN: 2>&1 | FileCheck -check-prefix=bogus10 %s<br class="">
+<br class="">
+# bogus10: libbogus10.a(???) truncated or malformed archive (long name offset 507 past the end of the string table for archive member header at offset 94)<br class="">
<br class="">
Modified: llvm/trunk/tools/dsymutil/BinaryHolder.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/BinaryHolder.cpp?rev=277177&r1=277176&r2=277177&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/BinaryHolder.cpp?rev=277177&r1=277176&r2=277177&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/tools/dsymutil/BinaryHolder.cpp (original)<br class="">
+++ llvm/trunk/tools/dsymutil/BinaryHolder.cpp Fri Jul 29 12:44:13 2016<br class="">
@@ -115,8 +115,8 @@ BinaryHolder::GetArchiveMemberBuffers(St<br class="">
if (Verbose)<br class="">
outs() << "\tfound member in current archive.\n";<br class="">
auto ErrOrMem = Child.getMemoryBufferRef();<br class="">
- if (auto Err = ErrOrMem.getError())<br class="">
- return Err;<br class="">
+ if (!ErrOrMem)<br class="">
+ return errorToErrorCode(ErrOrMem.takeError());<br class="">
Buffers.push_back(*ErrOrMem);<br class="">
}<br class="">
}<br class="">
<br class="">
Modified: llvm/trunk/tools/llvm-ar/llvm-ar.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-ar/llvm-ar.cpp?rev=277177&r1=277176&r2=277177&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-ar/llvm-ar.cpp?rev=277177&r1=277176&r2=277177&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/tools/llvm-ar/llvm-ar.cpp (original)<br class="">
+++ llvm/trunk/tools/llvm-ar/llvm-ar.cpp Fri Jul 29 12:44:13 2016<br class="">
@@ -409,8 +409,8 @@ static void performReadOperation(Archive<br class="">
{<br class="">
Error Err;<br class="">
for (auto &C : OldArchive->children(Err)) {<br class="">
- ErrorOr<StringRef> NameOrErr = C.getName();<br class="">
- failIfError(NameOrErr.getError());<br class="">
+ Expected<StringRef> NameOrErr = C.getName();<br class="">
+ failIfError(NameOrErr.takeError());<br class="">
StringRef Name = NameOrErr.get();<br class="">
<br class="">
if (Filter) {<br class="">
@@ -537,8 +537,8 @@ computeNewArchiveMembers(ArchiveOperatio<br class="">
Error Err;<br class="">
for (auto &Child : OldArchive->children(Err)) {<br class="">
int Pos = Ret.size();<br class="">
- ErrorOr<StringRef> NameOrErr = Child.getName();<br class="">
- failIfError(NameOrErr.getError());<br class="">
+ Expected<StringRef> NameOrErr = Child.getName();<br class="">
+ failIfError(NameOrErr.takeError());<br class="">
StringRef Name = NameOrErr.get();<br class="">
if (Name == PosName) {<br class="">
assert(AddAfter || AddBefore);<br class="">
<br class="">
Modified: llvm/trunk/tools/llvm-nm/llvm-nm.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-nm/llvm-nm.cpp?rev=277177&r1=277176&r2=277177&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-nm/llvm-nm.cpp?rev=277177&r1=277176&r2=277177&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/tools/llvm-nm/llvm-nm.cpp (original)<br class="">
+++ llvm/trunk/tools/llvm-nm/llvm-nm.cpp Fri Jul 29 12:44:13 2016<br class="">
@@ -198,13 +198,14 @@ static void error(llvm::Error E, StringR<br class="">
HadError = true;<br class="">
errs() << ToolName << ": " << FileName;<br class="">
<br class="">
- ErrorOr<StringRef> NameOrErr = C.getName();<br class="">
+ Expected<StringRef> NameOrErr = C.getName();<br class="">
// TODO: if we have a error getting the name then it would be nice to print<br class="">
// the index of which archive member this is and or its offset in the<br class="">
// archive instead of "???" as the name.<br class="">
- if (NameOrErr.getError())<br class="">
+ if (!NameOrErr) {<br class="">
+ consumeError(NameOrErr.takeError());<br class="">
errs() << "(" << "???" << ")";<br class="">
- else<br class="">
+ } else<br class="">
errs() << "(" << NameOrErr.get() << ")";<br class="">
<br class="">
if (!ArchitectureName.empty())<br class="">
@@ -1099,9 +1100,11 @@ static void dumpSymbolNamesFromFile(std:<br class="">
ErrorOr<Archive::Child> C = I->getMember();<br class="">
if (error(C.getError()))<br class="">
return;<br class="">
- ErrorOr<StringRef> FileNameOrErr = C->getName();<br class="">
- if (error(FileNameOrErr.getError()))<br class="">
+ Expected<StringRef> FileNameOrErr = C->getName();<br class="">
+ if (!FileNameOrErr) {<br class="">
+ error(FileNameOrErr.takeError(), Filename);<br class="">
return;<br class="">
+ }<br class="">
StringRef SymName = I->getName();<br class="">
outs() << SymName << " in " << FileNameOrErr.get() << "\n";<br class="">
}<br class="">
<br class="">
Modified: llvm/trunk/tools/llvm-objdump/MachODump.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/MachODump.cpp?rev=277177&r1=277176&r2=277177&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/MachODump.cpp?rev=277177&r1=277176&r2=277177&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/tools/llvm-objdump/MachODump.cpp (original)<br class="">
+++ llvm/trunk/tools/llvm-objdump/MachODump.cpp Fri Jul 29 12:44:13 2016<br class="">
@@ -1521,16 +1521,23 @@ static void printArchiveChild(StringRef<br class="">
}<br class="">
<br class="">
if (verbose) {<br class="">
- ErrorOr<StringRef> NameOrErr = C.getName();<br class="">
- if (NameOrErr.getError()) {<br class="">
- StringRef RawName = C.getRawName();<br class="">
+ Expected<StringRef> NameOrErr = C.getName();<br class="">
+ if (!NameOrErr) {<br class="">
+ consumeError(NameOrErr.takeError());<br class="">
+ Expected<StringRef> NameOrErr = C.getRawName();<br class="">
+ if (!NameOrErr)<br class="">
+ report_error(Filename, C, NameOrErr.takeError(), ArchitectureName);<br class="">
+ StringRef RawName = NameOrErr.get();<br class="">
outs() << RawName << "\n";<br class="">
} else {<br class="">
StringRef Name = NameOrErr.get();<br class="">
outs() << Name << "\n";<br class="">
}<br class="">
} else {<br class="">
- StringRef RawName = C.getRawName();<br class="">
+ Expected<StringRef> NameOrErr = C.getRawName();<br class="">
+ if (!NameOrErr)<br class="">
+ report_error(Filename, C, NameOrErr.takeError(), ArchitectureName);<br class="">
+ StringRef RawName = NameOrErr.get();<br class="">
outs() << RawName << "\n";<br class="">
}<br class="">
}<br class="">
<br class="">
Modified: llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp?rev=277177&r1=277176&r2=277177&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp?rev=277177&r1=277176&r2=277177&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp (original)<br class="">
+++ llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp Fri Jul 29 12:44:13 2016<br class="">
@@ -312,13 +312,14 @@ LLVM_ATTRIBUTE_NORETURN void llvm::repor<br class="">
const object::Archive::Child &C,<br class="">
llvm::Error E,<br class="">
StringRef ArchitectureName) {<br class="">
- ErrorOr<StringRef> NameOrErr = C.getName();<br class="">
+ Expected<StringRef> NameOrErr = C.getName();<br class="">
// TODO: if we have a error getting the name then it would be nice to print<br class="">
// the index of which archive member this is and or its offset in the<br class="">
// archive instead of "???" as the name.<br class="">
- if (NameOrErr.getError())<br class="">
+ if (!NameOrErr) {<br class="">
+ consumeError(NameOrErr.takeError());<br class="">
llvm::report_error(ArchiveName, "???", std::move(E), ArchitectureName);<br class="">
- else<br class="">
+ } else<br class="">
llvm::report_error(ArchiveName, NameOrErr.get(), std::move(E),<br class="">
ArchitectureName);<br class="">
}<br class="">
<br class="">
Modified: llvm/trunk/tools/llvm-size/llvm-size.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-size/llvm-size.cpp?rev=277177&r1=277176&r2=277177&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-size/llvm-size.cpp?rev=277177&r1=277176&r2=277177&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/tools/llvm-size/llvm-size.cpp (original)<br class="">
+++ llvm/trunk/tools/llvm-size/llvm-size.cpp Fri Jul 29 12:44:13 2016<br class="">
@@ -114,13 +114,14 @@ static void error(llvm::Error E, StringR<br class="">
HadError = true;<br class="">
errs() << ToolName << ": " << FileName;<br class="">
<br class="">
- ErrorOr<StringRef> NameOrErr = C.getName();<br class="">
+ Expected<StringRef> NameOrErr = C.getName();<br class="">
// TODO: if we have a error getting the name then it would be nice to print<br class="">
// the index of which archive member this is and or its offset in the<br class="">
// archive instead of "???" as the name.<br class="">
- if (NameOrErr.getError())<br class="">
+ if (!NameOrErr) {<br class="">
+ consumeError(NameOrErr.takeError());<br class="">
errs() << "(" << "???" << ")";<br class="">
- else<br class="">
+ } else<br class="">
errs() << "(" << NameOrErr.get() << ")";<br class="">
<br class="">
if (!ArchitectureName.empty())<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="">
</blockquote></div></div>
</div></blockquote></div><br class=""></div></body></html>