[llvm] r277177 - The next step along the way to getting good error messages for bad archives.
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 1 10:32:34 PDT 2016
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(...)
...
}
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)
Also "Twine(NameOrErr.get())" doesn't need the Twine() wrapper 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".
(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?
> + } else if (Name.startswith("#1/")) {
>
Drop the else-after-return
http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
> + 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
> + // 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.
> + }
> + // 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)
> + }
> + }
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160801/f9e86fea/attachment.html>
More information about the llvm-commits
mailing list