[llvm] r277177 - The next step along the way to getting good error messages for bad archives.
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 2 10:29:27 PDT 2016
Hi David,
I don't know enough about the API here and why !Parent && !Start are
> relevant properties with regard to error handling.
Short version: Archive::ArchiveMemberHeader can either point at a header
(which may be malformed), or it can be set to a sentinel value by pointing
it at null. In the latter case Error is permitted to be null, since
no error could occur. So the invariant is "If I'm pointed at real data,
there must be a non-null Error pointer available to report malformed data
on".
- Lang.
On Mon, Aug 1, 2016 at 3:00 PM, David Blaikie via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Re: Stylistic things you're more comfortable with. In general, I'm not
> insisting on any changes, just mentioning it & totally up to you. I'll
> mention some of the benefits/reasons (beyond project consistency) I'd
> encourage them:
> * Declarations in conditionals: This keeps the scope of the variables to
> their use, which is generally preferable. (also takes up one less line of
> code)
> * op* rather than .get() - shorter and pretty common across a variety of
> APIs (see std-or-llvm Optional for an example that isn't a pointer)
>
> As for the "this was the way it was" - fair point. I just committed those
> myself in r277394
>
> And as for the final point/discussion... *skip ahead to the end*
>
> On Mon, Aug 1, 2016 at 2:45 PM Kevin Enderby <enderby at apple.com> wrote:
>
>> Hi David,
>>
>> Thanks for your time to take a look at these changes and your thoughtful
>> comments. There were some fixes to this in r277223.
>>
>> I’ve added my thoughts and comments below.
>>
>> On Aug 1, 2016, at 10:32 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> On Fri, Jul 29, 2016 at 10:51 AM Kevin Enderby via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Author: enderby
>>> Date: Fri Jul 29 12:44:13 2016
>>> New Revision: 277177
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=277177&view=rev
>>> Log:
>>> The next step along the way to getting good error messages for bad
>>> archives.
>>>
>>> As mentioned in commit log for r276686 this next step is adding a new
>>> method in the ArchiveMemberHeader class to get the full name that
>>> does proper error checking, and can be use for error messages.
>>>
>>> To do this the name of ArchiveMemberHeader::getName() is changed to
>>> ArchiveMemberHeader::getRawName() to be consistent with
>>> Archive::Child::getRawName(). Then the “new” method is the addition
>>> of a new implementation of ArchiveMemberHeader::getName() which gets
>>> the full name and provides proper error checking. Which is mostly a
>>> rewrite
>>> of what was Archive::Child::getName() and cleaning up incorrect uses of
>>> llvm_unreachable() in the code which were actually just cases of errors
>>> in the input Archives.
>>>
>>> Then Archive::Child::getName() is changed to return Expected<> and use
>>> the new implementation of ArchiveMemberHeader::getName() .
>>>
>>> Also needed to change Archive::getMemoryBufferRef() with these
>>> changes to return Expected<> as well to propagate Errors up.
>>> As well as changing Archive::isThinMember() to return Expected<> .
>>>
>>> Added:
>>> llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus10.a
>>> llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus6.a (with props)
>>> llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus7.a
>>> llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus8.a
>>> llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus9.a
>>> Modified:
>>> llvm/trunk/include/llvm/Object/Archive.h
>>> llvm/trunk/lib/Object/Archive.cpp
>>> llvm/trunk/lib/Object/ArchiveWriter.cpp
>>> llvm/trunk/test/tools/llvm-objdump/malformed-archives.test
>>> llvm/trunk/tools/dsymutil/BinaryHolder.cpp
>>> llvm/trunk/tools/llvm-ar/llvm-ar.cpp
>>> llvm/trunk/tools/llvm-nm/llvm-nm.cpp
>>> llvm/trunk/tools/llvm-objdump/MachODump.cpp
>>> llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp
>>> llvm/trunk/tools/llvm-size/llvm-size.cpp
>>>
>>> Modified: llvm/trunk/include/llvm/Object/Archive.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/Archive.h?rev=277177&r1=277176&r2=277177&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/Object/Archive.h (original)
>>> +++ llvm/trunk/include/llvm/Object/Archive.h Fri Jul 29 12:44:13 2016
>>> @@ -36,7 +36,10 @@ public:
>>> // ArchiveMemberHeader() = default;
>>>
>>> /// Get the name without looking up long names.
>>> - llvm::StringRef getName() const;
>>> + Expected<llvm::StringRef> getRawName() const;
>>> +
>>> + /// Get the name looking up long names.
>>> + Expected<llvm::StringRef> getName(uint64_t Size) const;
>>>
>>> /// Members are not larger than 4GB.
>>> Expected<uint32_t> getSize() const;
>>> @@ -82,7 +85,7 @@ public:
>>> /// \brief Offset from Data to the start of the file.
>>> uint16_t StartOfFile;
>>>
>>> - bool isThinMember() const;
>>> + Expected<bool> isThinMember() const;
>>>
>>> public:
>>> Child(const Archive *Parent, const char *Start, Error *Err);
>>> @@ -96,9 +99,9 @@ public:
>>> const Archive *getParent() const { return Parent; }
>>> Expected<Child> getNext() const;
>>>
>>> - ErrorOr<StringRef> getName() const;
>>> + Expected<StringRef> getName() const;
>>> ErrorOr<std::string> getFullName() const;
>>> - StringRef getRawName() const { return Header.getName(); }
>>> + Expected<StringRef> getRawName() const { return
>>> Header.getRawName(); }
>>> sys::TimeValue getLastModified() const {
>>> return Header.getLastModified();
>>> }
>>> @@ -118,7 +121,7 @@ public:
>>> ErrorOr<StringRef> getBuffer() const;
>>> uint64_t getChildOffset() const;
>>>
>>> - ErrorOr<MemoryBufferRef> getMemoryBufferRef() const;
>>> + Expected<MemoryBufferRef> getMemoryBufferRef() const;
>>>
>>> Expected<std::unique_ptr<Binary>>
>>> getAsBinary(LLVMContext *Context = nullptr) const;
>>> @@ -238,6 +241,7 @@ public:
>>>
>>> bool hasSymbolTable() const;
>>> StringRef getSymbolTable() const { return SymbolTable; }
>>> + StringRef getStringTable() const { return StringTable; }
>>> uint32_t getNumberOfSymbols() const;
>>>
>>> std::vector<std::unique_ptr<MemoryBuffer>> takeThinBuffers() {
>>>
>>> Modified: llvm/trunk/lib/Object/Archive.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/Archive.cpp?rev=277177&r1=277176&r2=277177&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/Object/Archive.cpp (original)
>>> +++ llvm/trunk/lib/Object/Archive.cpp Fri Jul 29 12:44:13 2016
>>> @@ -43,17 +43,17 @@ ArchiveMemberHeader::ArchiveMemberHeader
>>> return;
>>> ErrorAsOutParameter ErrAsOutParam(Err);
>>>
>>> - // TODO: For errors messages with the ArchiveMemberHeader class use
>>> the
>>> - // archive member name instead of the the offset to the archive
>>> member header.
>>> - // When there is also error getting the member name then use the
>>> offset to
>>> - // the member in the message.
>>> -
>>> if (Size < sizeof(ArMemHdrType)) {
>>> if (Err) {
>>> - uint64_t Offset = RawHeaderPtr - Parent->getData().data();
>>> - *Err = malformedError("remaining size of archive too small for
>>> next "
>>> - "archive member header at offset " +
>>> - Twine(Offset));
>>> + Twine Msg("remaining size of archive too small for next archive
>>> member "
>>> + "header ");
>>> + Expected<StringRef> NameOrErr = getName(Size);
>>> + if (!NameOrErr) {
>>> + consumeError(NameOrErr.takeError());
>>> + uint64_t Offset = RawHeaderPtr - Parent->getData().data();
>>> + *Err = malformedError(Msg + "at offset " + Twine(Offset));
>>> + } else
>>> + *Err = malformedError(Msg + "for " + Twine(NameOrErr.get()));
>>>
>>
>> If you like, this code might be a bit tidier by rolling in the
>> initialization into the conditional:
>>
>> if (Expected<StringRef> NameOrErr = getName(Size))
>> *Err = malformedError(...)
>> else {
>> consumeError(...)
>>
>> …
>>
>> }
>>
>>
>> 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.
>>
>>
>> Also, not sure, but this code might be a bit easier less indented:
>>
>> if (Size < sizeof....) {
>> if (!Err)
>> return;
>> /* populate *Err */
>> return;
>> }
>>
>> It's the same number of lines, but reduces indentation (but does
>> duplicate the return which might make it harder to read - not sure)
>>
>>
>> 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.
>>
>> Also "Twine(NameOrErr.get())" doesn't need the Twine() wrapper
>>
>>
>> Yep. And that was fixed with the follow on changes in r277223.
>>
>> 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”.
>>
>>
>> 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.
>>
>>
>> (all points similarly below)
>>
>>
>>> }
>>> return;
>>> }
>>> @@ -64,18 +64,35 @@ ArchiveMemberHeader::ArchiveMemberHeader
>>> OS.write_escaped(llvm::StringRef(ArMemHdr->Terminator,
>>> sizeof(ArMemHdr->Terminator)));
>>> OS.flush();
>>> - uint64_t Offset = RawHeaderPtr - Parent->getData().data();
>>> - *Err = malformedError("terminator characters in archive member
>>> \"" + Buf +
>>> - "\" not the correct \"`\\n\" values for the
>>> "
>>> - "archive member header at offset " +
>>> Twine(Offset));
>>> + Twine Msg("terminator characters in archive member \"" + Buf +
>>> "\" not "
>>> + "the correct \"`\\n\" values for the archive member
>>> header ");
>>> + Expected<StringRef> NameOrErr = getName(Size);
>>> + if (!NameOrErr) {
>>> + consumeError(NameOrErr.takeError());
>>> + uint64_t Offset = RawHeaderPtr - Parent->getData().data();
>>> + *Err = malformedError(Msg + "at offset " + Twine(Offset));
>>> + } else
>>> + *Err = malformedError(Msg + "for " + Twine(NameOrErr.get()));
>>> }
>>> return;
>>> }
>>> }
>>>
>>> -StringRef ArchiveMemberHeader::getName() const {
>>> +// This gets the raw name from the ArMemHdr->Name field and checks that
>>> it is
>>> +// valid for the kind of archive. If it is not valid it returns an
>>> Error.
>>> +Expected<StringRef> ArchiveMemberHeader::getRawName() const {
>>> char EndCond;
>>> - if (ArMemHdr->Name[0] == '/' || ArMemHdr->Name[0] == '#')
>>> + auto Kind = Parent->kind();
>>> + if (Kind == Archive::K_BSD || Kind == Archive::K_DARWIN64) {
>>> + if (ArMemHdr->Name[0] == ' ') {
>>> + uint64_t Offset = reinterpret_cast<const char *>(ArMemHdr) -
>>> + Parent->getData().data();
>>> + return malformedError("name contains a leading space for archive
>>> member "
>>> + "header at offset " + Twine(Offset));
>>> + }
>>> + EndCond = ' ';
>>> + }
>>> + else if (ArMemHdr->Name[0] == '/' || ArMemHdr->Name[0] == '#')
>>> EndCond = ' ';
>>> else
>>> EndCond = '/';
>>> @@ -88,6 +105,105 @@ StringRef ArchiveMemberHeader::getName()
>>> return llvm::StringRef(ArMemHdr->Name, end);
>>> }
>>>
>>> +// This gets the name looking up long names. Size is the size of the
>>> archive
>>> +// member including the header, so the size of any name following the
>>> header
>>> +// is checked to make sure it does not overflow.
>>> +Expected<StringRef> ArchiveMemberHeader::getName(uint64_t Size) const {
>>> +
>>> + // This can be called from the ArchiveMemberHeader constructor when
>>> the
>>> + // archive header is truncated to produce an error message with the
>>> name.
>>> + // Make sure the name field is not truncated.
>>> + if (Size < offsetof(ArMemHdrType, Name) + sizeof(ArMemHdr->Name)) {
>>> + uint64_t ArchiveOffset = reinterpret_cast<const char *>(ArMemHdr) -
>>> + Parent->getData().data();
>>> + return malformedError("archive header truncated before the name
>>> field "
>>> + "for archive member header at offset " +
>>> + Twine(ArchiveOffset));
>>> + }
>>> +
>>> + // The raw name itself can be invalid.
>>> + Expected<StringRef> NameOrErr = getRawName();
>>> + if (!NameOrErr)
>>> + return NameOrErr.takeError();
>>> + StringRef Name = NameOrErr.get();
>>> +
>>> + // Check if it's a special name.
>>> + if (Name[0] == '/') {
>>> + if (Name.size() == 1) // Linker member.
>>> + return Name;
>>> + if (Name.size() == 2 && Name[1] == '/') // String table.
>>> + return Name;
>>> + // It's a long name.
>>> + // Get the string table offset.
>>> + std::size_t StringOffset;
>>> + if (Name.substr(1).rtrim(' ').getAsInteger(10, StringOffset)) {
>>> + std::string Buf;
>>> + raw_string_ostream OS(Buf);
>>> + OS.write_escaped(Name.substr(1).rtrim(' '),
>>> + sizeof(Name.substr(1)).rtrim('
>>> '));
>>> + OS.flush();
>>> + uint64_t ArchiveOffset = reinterpret_cast<const char *>(ArMemHdr)
>>> -
>>> + Parent->getData().data();
>>> + return malformedError("long name offset characters after the '/'
>>> are "
>>> + "not all decimal numbers: '" + Buf + "' for
>>> "
>>> + "archive member header at offset " +
>>> + Twine(ArchiveOffset));
>>> + }
>>> +
>>> + // Verify it.
>>> + if (StringOffset >= Parent->getStringTable().size()) {
>>> + uint64_t ArchiveOffset = reinterpret_cast<const char *>(ArMemHdr)
>>> -
>>> + Parent->getData().data();
>>> + return malformedError("long name offset " + Twine(StringOffset) +
>>> " past "
>>> + "the end of the string table for archive
>>> member "
>>> + "header at offset " + Twine(ArchiveOffset));
>>> + }
>>> + const char *addr = Parent->getStringTable().begin() + StringOffset;
>>> +
>>> + // GNU long file names end with a "/\n".
>>> + if (Parent->kind() == Archive::K_GNU ||
>>> + Parent->kind() == Archive::K_MIPS64) {
>>> + StringRef::size_type End = StringRef(addr).find('\n');
>>> + return StringRef(addr, End - 1);
>>> + }
>>> + return StringRef(addr);
>>>
>>
>> "return addr;" should suffice here?
>>
>>
>> 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:
>>
>> - return StringRef(addr);
>>
>>
>>
>>> + } else if (Name.startswith("#1/")) {
>>>
>>
>> Drop the else-after-return
>> http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
>>
>>
>> Sure I can make that change. But again the code in
>> Archive::Child::getName() that was deleted (again see below) was exactly as
>> above:
>>
>> - } else if (name.startswith("#1/")) {
>>
>>
>>
>>> + uint64_t NameLength;
>>
>>
>>> + if (Name.substr(3).rtrim(' ').getAsInteger(10, NameLength)) {
>>> + std::string Buf;
>>> + raw_string_ostream OS(Buf);
>>> + OS.write_escaped(Name.substr(3).rtrim(' '),
>>> + sizeof(Name.substr(3)).rtrim('
>>> '));
>>> + OS.flush();
>>> + uint64_t ArchiveOffset = reinterpret_cast<const char *>(ArMemHdr)
>>> -
>>> + Parent->getData().data();
>>> + return malformedError("long name length characters after the #1/
>>> are "
>>> + "not all decimal numbers: '" + Buf + "' for
>>> "
>>> + "archive member header at offset " +
>>> + Twine(ArchiveOffset));
>>> + }
>>> + if (getSizeOf() + NameLength > Size) {
>>> + uint64_t ArchiveOffset = reinterpret_cast<const char *>(ArMemHdr)
>>> -
>>> + Parent->getData().data();
>>> + return malformedError("long name length: " + Twine(NameLength) +
>>> + " extends past the end of the member or
>>> archive "
>>> + "for archive member header at offset " +
>>> + Twine(ArchiveOffset));
>>> + }
>>> + return StringRef(reinterpret_cast<const char *>(ArMemHdr) +
>>> getSizeOf(),
>>> + NameLength).rtrim('\0');
>>> + } else {
>>>
>>
>> Another else-after-return
>>
>>
>> Again from the original code (see below), just re-written:
>>
>> - return Data.substr(Header.getSizeOf(), name_size).rtrim('\0');
>> - } else {
>>
>>
>>
>>> + // It is not a long name so trim the blanks at the end of the name.
>>
>>
>>> + if (Name[Name.size() - 1] != '/') {
>>
>> + return Name.rtrim(' ');
>>> + }
>>>
>>
>> Probably drop the {} on a single line statement.
>>
>>
>> And another from the original code (see below):
>>
>> - // It is not a long name so trim the blanks at the end of the name.
>> - if (name[name.size() - 1] != '/') {
>> - return name.rtrim(' ');
>> - }
>>
>>
>>
>>> + }
>>
>>
>>> + // It's a simple name.
>>> + if (Name[Name.size() - 1] == '/')
>>> + return Name.substr(0, Name.size() - 1);
>>> + return Name;
>>> +}
>>> +
>>> Expected<uint32_t> ArchiveMemberHeader::getSize() const {
>>> uint32_t Ret;
>>> if (llvm::StringRef(ArMemHdr->Size,
>>> @@ -166,7 +282,14 @@ Archive::Child::Child(const Archive *Par
>>>
>>> uint64_t Size = Header.getSizeOf();
>>> Data = StringRef(Start, Size);
>>> - if (!isThinMember()) {
>>> + Expected<bool> isThinOrErr = isThinMember();
>>> + if (!isThinOrErr) {
>>> + if (Err)
>>> + *Err = isThinOrErr.takeError();
>>> + return;
>>> + }
>>> + bool isThin = isThinOrErr.get();
>>> + if (!isThin) {
>>> Expected<uint64_t> MemberSize = getRawSize();
>>> if (!MemberSize) {
>>> if (Err)
>>> @@ -180,11 +303,30 @@ Archive::Child::Child(const Archive *Par
>>> // Setup StartOfFile and PaddingBytes.
>>> StartOfFile = Header.getSizeOf();
>>> // Don't include attached name.
>>> - StringRef Name = getRawName();
>>> + Expected<StringRef> NameOrErr = getRawName();
>>> + if (!NameOrErr){
>>> + if (Err)
>>> + *Err = NameOrErr.takeError();
>>> + return;
>>> + }
>>> + StringRef Name = NameOrErr.get();
>>> if (Name.startswith("#1/")) {
>>> uint64_t NameSize;
>>> - if (Name.substr(3).rtrim(' ').getAsInteger(10, NameSize))
>>> - llvm_unreachable("Long name length is not an integer");
>>> + if (Name.substr(3).rtrim(' ').getAsInteger(10, NameSize)) {
>>> + if (Err) {
>>> + std::string Buf;
>>> + raw_string_ostream OS(Buf);
>>> + OS.write_escaped(Name.substr(3).rtrim(' '),
>>> + sizeof(Name.substr(3)).rtrim('
>>> '));
>>> + OS.flush();
>>> + uint64_t Offset = Start - Parent->getData().data();
>>> + *Err = malformedError("long name length characters after the
>>> #1/ are "
>>> + "not all decimal numbers: '" + Buf + "'
>>> for "
>>> + "archive member header at offset " +
>>> + Twine(Offset));
>>> + return;
>>>
>>
>> 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)
>>
>>
>> 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.
>>
>> 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?
>>
>
> I don't know enough about the API here and why !Parent && !Start are
> relevant properties with regard to error handling. If you two reckon that's
> relevant, sure - check that property. Especially if it allows you to check
> that property and bail out early, then provide a stronger invariant for the
> rest of the code.
>
> (don't get me wrong - happy to chat about the design further/don't mean to
> be dismissive, just that I don't have enough context myself right now &
> realize explaining it to me might not be worthwhile if you two have it
> covered.
>
> - Dave
>
>
>>
>>
>>
>>> + }
>>> + }
>>> StartOfFile += NameSize;
>>> }
>>> }
>>> @@ -203,16 +345,22 @@ Expected<uint64_t> Archive::Child::getRa
>>> return Header.getSize();
>>> }
>>>
>>> -bool Archive::Child::isThinMember() const {
>>> - StringRef Name = Header.getName();
>>> +Expected<bool> Archive::Child::isThinMember() const {
>>> + Expected<StringRef> NameOrErr = Header.getRawName();
>>> + if (!NameOrErr)
>>> + return NameOrErr.takeError();
>>> + StringRef Name = NameOrErr.get();
>>> return Parent->IsThin && Name != "/" && Name != "//";
>>> }
>>>
>>> ErrorOr<std::string> Archive::Child::getFullName() const {
>>> - assert(isThinMember());
>>> - ErrorOr<StringRef> NameOrErr = getName();
>>> - if (std::error_code EC = NameOrErr.getError())
>>> - return EC;
>>> + Expected<bool> isThin = isThinMember();
>>> + if (!isThin)
>>> + return errorToErrorCode(isThin.takeError());
>>> + assert(isThin.get());
>>
>> + Expected<StringRef> NameOrErr = getName();
>>> + if (!NameOrErr)
>>> + return errorToErrorCode(NameOrErr.takeError());
>>> StringRef Name = *NameOrErr;
>>> if (sys::path::is_absolute(Name))
>>> return Name;
>>> @@ -224,7 +372,11 @@ ErrorOr<std::string> Archive::Child::get
>>> }
>>>
>>> ErrorOr<StringRef> Archive::Child::getBuffer() const {
>>> - if (!isThinMember()) {
>>> + Expected<bool> isThinOrErr = isThinMember();
>>> + if (!isThinOrErr)
>>> + return errorToErrorCode(isThinOrErr.takeError());
>>> + bool isThin = isThinOrErr.get();
>>> + if (!isThin) {
>>> Expected<uint32_t> Size = getSize();
>>> if (!Size)
>>> return errorToErrorCode(Size.takeError());
>>> @@ -257,8 +409,9 @@ Expected<Archive::Child> Archive::Child:
>>> if (NextLoc > Parent->Data.getBufferEnd()) {
>>> Twine Msg("offset to next archive member past the end of the
>>> archive after "
>>> "member ");
>>> - ErrorOr<StringRef> NameOrErr = getName();
>>> - if (NameOrErr.getError()) {
>>> + Expected<StringRef> NameOrErr = getName();
>>> + if (!NameOrErr) {
>>> + consumeError(NameOrErr.takeError());
>>> uint64_t Offset = Data.data() - Parent->getData().data();
>>> return malformedError(Msg + "at offset " + Twine(Offset));
>>> } else
>>> @@ -279,64 +432,34 @@ uint64_t Archive::Child::getChildOffset(
>>> return offset;
>>> }
>>>
>>> -ErrorOr<StringRef> Archive::Child::getName() const {
>>> - StringRef name = getRawName();
>>> - // Check if it's a special name.
>>> - if (name[0] == '/') {
>>> - if (name.size() == 1) // Linker member.
>>> - return name;
>>> - if (name.size() == 2 && name[1] == '/') // String table.
>>> - return name;
>>> - // It's a long name.
>>> - // Get the offset.
>>> - std::size_t offset;
>>> - if (name.substr(1).rtrim(' ').getAsInteger(10, offset))
>>> - llvm_unreachable("Long name offset is not an integer");
>>> -
>>> - // Verify it.
>>> - if (offset >= Parent->StringTable.size())
>>> - return object_error::parse_failed;
>>> - const char *addr = Parent->StringTable.begin() + offset;
>>> -
>>> - // GNU long file names end with a "/\n".
>>> - if (Parent->kind() == K_GNU || Parent->kind() == K_MIPS64) {
>>> - StringRef::size_type End = StringRef(addr).find('\n');
>>> - return StringRef(addr, End - 1);
>>> - }
>>> - return StringRef(addr);
>>> - } else if (name.startswith("#1/")) {
>>> - uint64_t name_size;
>>> - if (name.substr(3).rtrim(' ').getAsInteger(10, name_size))
>>> - llvm_unreachable("Long name length is not an ingeter");
>>> - return Data.substr(Header.getSizeOf(), name_size).rtrim('\0');
>>> - } else {
>>> - // It is not a long name so trim the blanks at the end of the name.
>>> - if (name[name.size() - 1] != '/') {
>>> - return name.rtrim(' ');
>>> - }
>>> - }
>>> - // It's a simple name.
>>> - if (name[name.size() - 1] == '/')
>>> - return name.substr(0, name.size() - 1);
>>> - return name;
>>> +Expected<StringRef> Archive::Child::getName() const {
>>> + Expected<uint64_t> RawSizeOrErr = getRawSize();
>>> + if (!RawSizeOrErr)
>>> + return RawSizeOrErr.takeError();
>>> + uint64_t RawSize = RawSizeOrErr.get();
>>> + Expected<StringRef> NameOrErr = Header.getName(Header.getSizeOf() +
>>> RawSize);
>>> + if (!NameOrErr)
>>> + return NameOrErr.takeError();
>>> + StringRef Name = NameOrErr.get();
>>> + return Name;
>>> }
>>>
>>> -ErrorOr<MemoryBufferRef> Archive::Child::getMemoryBufferRef() const {
>>> - ErrorOr<StringRef> NameOrErr = getName();
>>> - if (std::error_code EC = NameOrErr.getError())
>>> - return EC;
>>> +Expected<MemoryBufferRef> Archive::Child::getMemoryBufferRef() const {
>>> + Expected<StringRef> NameOrErr = getName();
>>> + if (!NameOrErr)
>>> + return NameOrErr.takeError();
>>> StringRef Name = NameOrErr.get();
>>> ErrorOr<StringRef> Buf = getBuffer();
>>> if (std::error_code EC = Buf.getError())
>>> - return EC;
>>> + return errorCodeToError(EC);
>>> return MemoryBufferRef(*Buf, Name);
>>> }
>>>
>>> Expected<std::unique_ptr<Binary>>
>>> Archive::Child::getAsBinary(LLVMContext *Context) const {
>>> - ErrorOr<MemoryBufferRef> BuffOrErr = getMemoryBufferRef();
>>> - if (std::error_code EC = BuffOrErr.getError())
>>> - return errorCodeToError(EC);
>>> + Expected<MemoryBufferRef> BuffOrErr = getMemoryBufferRef();
>>> + if (!BuffOrErr)
>>> + return BuffOrErr.takeError();
>>>
>>> auto BinaryOrErr = createBinary(BuffOrErr.get(), Context);
>>> if (BinaryOrErr)
>>> @@ -372,17 +495,20 @@ Archive::Archive(MemoryBufferRef Source,
>>> return;
>>> }
>>>
>>> + // Make sure Format is initialized before any call to
>>> + // ArchiveMemberHeader::getName() is made. This could be a valid
>>> empty
>>> + // archive which is the same in all formats. So claiming it to be
>>> gnu to is
>>> + // fine if not totally correct before we look for a string table or
>>> table of
>>> + // contents.
>>> + Format = K_GNU;
>>> +
>>> // Get the special members.
>>> child_iterator I = child_begin(Err, false);
>>> if (Err)
>>> return;
>>> child_iterator E = child_end();
>>>
>>> - // This is at least a valid empty archive. Since an empty archive is
>>> the
>>> - // same in all formats, just claim it to be gnu to make sure Format is
>>> - // initialized.
>>> - Format = K_GNU;
>>> -
>>> + // See if this is a valid empty archive and if so return.
>>> if (I == E) {
>>> Err = Error::success();
>>> return;
>>> @@ -397,7 +523,12 @@ Archive::Archive(MemoryBufferRef Source,
>>> return false;
>>> };
>>>
>>> - StringRef Name = C->getRawName();
>>> + Expected<StringRef> NameOrErr = C->getRawName();
>>> + if (!NameOrErr) {
>>> + Err = NameOrErr.takeError();
>>> + return;
>>> + }
>>> + StringRef Name = NameOrErr.get();
>>>
>>> // Below is the pattern that is used to figure out the archive format
>>> // GNU archive format
>>> @@ -437,9 +568,9 @@ Archive::Archive(MemoryBufferRef Source,
>>> if (Name.startswith("#1/")) {
>>> Format = K_BSD;
>>> // We know this is BSD, so getName will work since there is no
>>> string table.
>>> - ErrorOr<StringRef> NameOrErr = C->getName();
>>> - if (auto ec = NameOrErr.getError()) {
>>> - Err = errorCodeToError(ec);
>>> + Expected<StringRef> NameOrErr = C->getName();
>>> + if (!NameOrErr) {
>>> + Err = NameOrErr.takeError();
>>> return;
>>> }
>>> Name = NameOrErr.get();
>>> @@ -481,7 +612,12 @@ Archive::Archive(MemoryBufferRef Source,
>>> Err = Error::success();
>>> return;
>>> }
>>> - Name = C->getRawName();
>>> + Expected<StringRef> NameOrErr = C->getRawName();
>>> + if (!NameOrErr) {
>>> + Err = NameOrErr.takeError();
>>> + return;
>>> + }
>>> + Name = NameOrErr.get();
>>> }
>>>
>>> if (Name == "//") {
>>> @@ -522,7 +658,12 @@ Archive::Archive(MemoryBufferRef Source,
>>> return;
>>> }
>>>
>>> - Name = C->getRawName();
>>> + NameOrErr = C->getRawName();
>>> + if (!NameOrErr) {
>>> + Err = NameOrErr.takeError();
>>> + return;
>>> + }
>>> + Name = NameOrErr.get();
>>>
>>> if (Name == "//") {
>>> // The string table is never an external member, so we just assert
>>> on the
>>>
>>> Modified: llvm/trunk/lib/Object/ArchiveWriter.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/ArchiveWriter.cpp?rev=277177&r1=277176&r2=277177&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/Object/ArchiveWriter.cpp (original)
>>> +++ llvm/trunk/lib/Object/ArchiveWriter.cpp Fri Jul 29 12:44:13 2016
>>> @@ -40,9 +40,9 @@ NewArchiveMember::NewArchiveMember(Memor
>>> Expected<NewArchiveMember>
>>> NewArchiveMember::getOldMember(const object::Archive::Child &OldMember,
>>> bool Deterministic) {
>>> - ErrorOr<llvm::MemoryBufferRef> BufOrErr =
>>> OldMember.getMemoryBufferRef();
>>> + Expected<llvm::MemoryBufferRef> BufOrErr =
>>> OldMember.getMemoryBufferRef();
>>> if (!BufOrErr)
>>> - return errorCodeToError(BufOrErr.getError());
>>> + return BufOrErr.takeError();
>>>
>>> NewArchiveMember M;
>>> M.Buf = MemoryBuffer::getMemBuffer(*BufOrErr, false);
>>>
>>> Added: llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus10.a
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus10.a?rev=277177&view=auto
>>>
>>> ==============================================================================
>>> --- llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus10.a (added)
>>> +++ llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus10.a Fri Jul 29
>>> 12:44:13 2016
>>> @@ -0,0 +1,13 @@
>>> +!<arch>
>>> +// 26 `
>>> +1234567890123456hello.c/
>>> +
>>> +/507 0 0 0 644 102 `
>>> +#include <stdio.h>
>>> +#include <stdlib.h>
>>> +int
>>> +main()
>>> +{
>>> + printf("Hello World\n");
>>> + return EXIT_SUCCESS;
>>> +}
>>>
>>> Added: llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus6.a
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus6.a?rev=277177&view=auto
>>>
>>> ==============================================================================
>>> Binary file - no diff available.
>>>
>>> Propchange: llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus6.a
>>>
>>> ------------------------------------------------------------------------------
>>> svn:mime-type = application/octet-stream
>>>
>>> Added: llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus7.a
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus7.a?rev=277177&view=auto
>>>
>>> ==============================================================================
>>> --- llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus7.a (added)
>>> +++ llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus7.a Fri Jul 29
>>> 12:44:13 2016
>>> @@ -0,0 +1,10 @@
>>> +!<arch>
>>> +#1/@123$ 1469564779 124 0 100644 102 `
>>> +#include <stdio.h>
>>> +#include <stdlib.h>
>>> +int
>>> +main()
>>> +{
>>> + printf("Hello World\n");
>>> + return EXIT_SUCCESS;
>>> +}
>>>
>>> Added: llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus8.a
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus8.a?rev=277177&view=auto
>>>
>>> ==============================================================================
>>> --- llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus8.a (added)
>>> +++ llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus8.a Fri Jul 29
>>> 12:44:13 2016
>>> @@ -0,0 +1,13 @@
>>> +!<arch>
>>> +foo.c 1444941645 124 0 100644 17 `
>>> +void foo(void){}
>>> +
>>> +#1/1234 1469564779 124 0 100644 126 `
>>> +1234567890123456Xhello.c#include <stdio.h>
>>> +#include <stdlib.h>
>>> +int
>>> +main()
>>> +{
>>> + printf("Hello World\n");
>>> + return EXIT_SUCCESS;
>>> +}
>>>
>>> Added: llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus9.a
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus9.a?rev=277177&view=auto
>>>
>>> ==============================================================================
>>> --- llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus9.a (added)
>>> +++ llvm/trunk/test/tools/llvm-objdump/Inputs/libbogus9.a Fri Jul 29
>>> 12:44:13 2016
>>> @@ -0,0 +1,13 @@
>>> +!<arch>
>>> +// 26 `
>>> +1234567890123456hello.c/
>>> +
>>> +/&a25* 0 0 0 644 102 `
>>> +#include <stdio.h>
>>> +#include <stdlib.h>
>>> +int
>>> +main()
>>> +{
>>> + printf("Hello World\n");
>>> + return EXIT_SUCCESS;
>>> +}
>>>
>>> Modified: llvm/trunk/test/tools/llvm-objdump/malformed-archives.test
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/malformed-archives.test?rev=277177&r1=277176&r2=277177&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/test/tools/llvm-objdump/malformed-archives.test (original)
>>> +++ llvm/trunk/test/tools/llvm-objdump/malformed-archives.test Fri Jul
>>> 29 12:44:13 2016
>>> @@ -23,10 +23,38 @@
>>> # RUN: %p/Inputs/libbogus4.a \
>>> # RUN: 2>&1 | FileCheck -check-prefix=bogus4 %s
>>>
>>> -# bogus4: libbogus4.a': truncated or malformed archive (remaining size
>>> of archive too small for next archive member header at offset 170)
>>> +# bogus4: libbogus4.a': truncated or malformed archive (remaining size
>>> of archive too small for next archive member header for foo.c)
>>>
>>> # RUN: not llvm-objdump -macho -archive-headers \
>>> # RUN: %p/Inputs/libbogus5.a \
>>> # RUN: 2>&1 | FileCheck -check-prefix=bogus5 %s
>>>
>>> -# 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)
>>> +# 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)
>>> +
>>> +# RUN: not llvm-objdump -macho -archive-headers \
>>> +# RUN: %p/Inputs/libbogus6.a \
>>> +# RUN: 2>&1 | FileCheck -check-prefix=bogus6 %s
>>> +
>>> +# bogus6: libbogus6.a': truncated or malformed archive (name contains a
>>> leading space for archive member header at offset 96)
>>> +
>>> +# RUN: not llvm-objdump -macho -archive-headers \
>>> +# RUN: %p/Inputs/libbogus7.a \
>>> +# RUN: 2>&1 | FileCheck -check-prefix=bogus7 %s
>>> +
>>> +# 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)
>>> +
>>> +# RUN: not llvm-objdump -macho -archive-headers \
>>> +# RUN: %p/Inputs/libbogus8.a \
>>> +# RUN: 2>&1 | FileCheck -check-prefix=bogus8 %s
>>> +
>>> +# 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)
>>> +
>>> +# RUN: not llvm-objdump -s %p/Inputs/libbogus9.a \
>>> +# RUN: 2>&1 | FileCheck -check-prefix=bogus9 %s
>>> +
>>> +# 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)
>>> +
>>> +# RUN: not llvm-objdump -s %p/Inputs/libbogus10.a \
>>> +# RUN: 2>&1 | FileCheck -check-prefix=bogus10 %s
>>> +
>>> +# 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)
>>>
>>> Modified: llvm/trunk/tools/dsymutil/BinaryHolder.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/BinaryHolder.cpp?rev=277177&r1=277176&r2=277177&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/tools/dsymutil/BinaryHolder.cpp (original)
>>> +++ llvm/trunk/tools/dsymutil/BinaryHolder.cpp Fri Jul 29 12:44:13 2016
>>> @@ -115,8 +115,8 @@ BinaryHolder::GetArchiveMemberBuffers(St
>>> if (Verbose)
>>> outs() << "\tfound member in current archive.\n";
>>> auto ErrOrMem = Child.getMemoryBufferRef();
>>> - if (auto Err = ErrOrMem.getError())
>>> - return Err;
>>> + if (!ErrOrMem)
>>> + return errorToErrorCode(ErrOrMem.takeError());
>>> Buffers.push_back(*ErrOrMem);
>>> }
>>> }
>>>
>>> Modified: llvm/trunk/tools/llvm-ar/llvm-ar.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-ar/llvm-ar.cpp?rev=277177&r1=277176&r2=277177&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/tools/llvm-ar/llvm-ar.cpp (original)
>>> +++ llvm/trunk/tools/llvm-ar/llvm-ar.cpp Fri Jul 29 12:44:13 2016
>>> @@ -409,8 +409,8 @@ static void performReadOperation(Archive
>>> {
>>> Error Err;
>>> for (auto &C : OldArchive->children(Err)) {
>>> - ErrorOr<StringRef> NameOrErr = C.getName();
>>> - failIfError(NameOrErr.getError());
>>> + Expected<StringRef> NameOrErr = C.getName();
>>> + failIfError(NameOrErr.takeError());
>>> StringRef Name = NameOrErr.get();
>>>
>>> if (Filter) {
>>> @@ -537,8 +537,8 @@ computeNewArchiveMembers(ArchiveOperatio
>>> Error Err;
>>> for (auto &Child : OldArchive->children(Err)) {
>>> int Pos = Ret.size();
>>> - ErrorOr<StringRef> NameOrErr = Child.getName();
>>> - failIfError(NameOrErr.getError());
>>> + Expected<StringRef> NameOrErr = Child.getName();
>>> + failIfError(NameOrErr.takeError());
>>> StringRef Name = NameOrErr.get();
>>> if (Name == PosName) {
>>> assert(AddAfter || AddBefore);
>>>
>>> Modified: llvm/trunk/tools/llvm-nm/llvm-nm.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-nm/llvm-nm.cpp?rev=277177&r1=277176&r2=277177&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/tools/llvm-nm/llvm-nm.cpp (original)
>>> +++ llvm/trunk/tools/llvm-nm/llvm-nm.cpp Fri Jul 29 12:44:13 2016
>>> @@ -198,13 +198,14 @@ static void error(llvm::Error E, StringR
>>> HadError = true;
>>> errs() << ToolName << ": " << FileName;
>>>
>>> - ErrorOr<StringRef> NameOrErr = C.getName();
>>> + Expected<StringRef> NameOrErr = C.getName();
>>> // TODO: if we have a error getting the name then it would be nice to
>>> print
>>> // the index of which archive member this is and or its offset in the
>>> // archive instead of "???" as the name.
>>> - if (NameOrErr.getError())
>>> + if (!NameOrErr) {
>>> + consumeError(NameOrErr.takeError());
>>> errs() << "(" << "???" << ")";
>>> - else
>>> + } else
>>> errs() << "(" << NameOrErr.get() << ")";
>>>
>>> if (!ArchitectureName.empty())
>>> @@ -1099,9 +1100,11 @@ static void dumpSymbolNamesFromFile(std:
>>> ErrorOr<Archive::Child> C = I->getMember();
>>> if (error(C.getError()))
>>> return;
>>> - ErrorOr<StringRef> FileNameOrErr = C->getName();
>>> - if (error(FileNameOrErr.getError()))
>>> + Expected<StringRef> FileNameOrErr = C->getName();
>>> + if (!FileNameOrErr) {
>>> + error(FileNameOrErr.takeError(), Filename);
>>> return;
>>> + }
>>> StringRef SymName = I->getName();
>>> outs() << SymName << " in " << FileNameOrErr.get() << "\n";
>>> }
>>>
>>> Modified: llvm/trunk/tools/llvm-objdump/MachODump.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/MachODump.cpp?rev=277177&r1=277176&r2=277177&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/tools/llvm-objdump/MachODump.cpp (original)
>>> +++ llvm/trunk/tools/llvm-objdump/MachODump.cpp Fri Jul 29 12:44:13 2016
>>> @@ -1521,16 +1521,23 @@ static void printArchiveChild(StringRef
>>> }
>>>
>>> if (verbose) {
>>> - ErrorOr<StringRef> NameOrErr = C.getName();
>>> - if (NameOrErr.getError()) {
>>> - StringRef RawName = C.getRawName();
>>> + Expected<StringRef> NameOrErr = C.getName();
>>> + if (!NameOrErr) {
>>> + consumeError(NameOrErr.takeError());
>>> + Expected<StringRef> NameOrErr = C.getRawName();
>>> + if (!NameOrErr)
>>> + report_error(Filename, C, NameOrErr.takeError(),
>>> ArchitectureName);
>>> + StringRef RawName = NameOrErr.get();
>>> outs() << RawName << "\n";
>>> } else {
>>> StringRef Name = NameOrErr.get();
>>> outs() << Name << "\n";
>>> }
>>> } else {
>>> - StringRef RawName = C.getRawName();
>>> + Expected<StringRef> NameOrErr = C.getRawName();
>>> + if (!NameOrErr)
>>> + report_error(Filename, C, NameOrErr.takeError(),
>>> ArchitectureName);
>>> + StringRef RawName = NameOrErr.get();
>>> outs() << RawName << "\n";
>>> }
>>> }
>>>
>>> Modified: llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp?rev=277177&r1=277176&r2=277177&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp (original)
>>> +++ llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp Fri Jul 29 12:44:13
>>> 2016
>>> @@ -312,13 +312,14 @@ LLVM_ATTRIBUTE_NORETURN void llvm::repor
>>> const
>>> object::Archive::Child &C,
>>> llvm::Error E,
>>> StringRef
>>> ArchitectureName) {
>>> - ErrorOr<StringRef> NameOrErr = C.getName();
>>> + Expected<StringRef> NameOrErr = C.getName();
>>> // TODO: if we have a error getting the name then it would be nice to
>>> print
>>> // the index of which archive member this is and or its offset in the
>>> // archive instead of "???" as the name.
>>> - if (NameOrErr.getError())
>>> + if (!NameOrErr) {
>>> + consumeError(NameOrErr.takeError());
>>> llvm::report_error(ArchiveName, "???", std::move(E),
>>> ArchitectureName);
>>> - else
>>> + } else
>>> llvm::report_error(ArchiveName, NameOrErr.get(), std::move(E),
>>> ArchitectureName);
>>> }
>>>
>>> Modified: llvm/trunk/tools/llvm-size/llvm-size.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-size/llvm-size.cpp?rev=277177&r1=277176&r2=277177&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/tools/llvm-size/llvm-size.cpp (original)
>>> +++ llvm/trunk/tools/llvm-size/llvm-size.cpp Fri Jul 29 12:44:13 2016
>>> @@ -114,13 +114,14 @@ static void error(llvm::Error E, StringR
>>> HadError = true;
>>> errs() << ToolName << ": " << FileName;
>>>
>>> - ErrorOr<StringRef> NameOrErr = C.getName();
>>> + Expected<StringRef> NameOrErr = C.getName();
>>> // TODO: if we have a error getting the name then it would be nice to
>>> print
>>> // the index of which archive member this is and or its offset in the
>>> // archive instead of "???" as the name.
>>> - if (NameOrErr.getError())
>>> + if (!NameOrErr) {
>>> + consumeError(NameOrErr.takeError());
>>> errs() << "(" << "???" << ")";
>>> - else
>>> + } else
>>> errs() << "(" << NameOrErr.get() << ")";
>>>
>>> if (!ArchitectureName.empty())
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160802/be330c15/attachment.html>
More information about the llvm-commits
mailing list