[llvm-commits] [lld] Supporting archive libraries in lld
Michael Spencer
bigcheesegs at gmail.com
Wed Nov 7 12:11:16 PST 2012
On Wed, Nov 7, 2012 at 10:07 AM, Shankar Easwaran
<shankare at codeaurora.org> wrote:
> Hi Nick, Michael,
>
> I have attached the complete diff after the changes you mentioned.
>
> Note: the indentation issue earlier was caused by the diff tool earlier.
>
> Ok to commit ?
>
> Thanks
>
> Shankar Easwaran
LLVM part lgtm.
- Michael Spencer
>
> On 11/6/2012 5:10 PM, Shankar Easwaran wrote:
>
> Hi Michael,
>
> Thanks for the review.
>
> The indentation in the source looked fine but the diff option -uw made the
> diff appear differently than the source.
>
> Fixed the diff output and resending the llvm diff.
>
> Thanks
>
> Shankar Easwaran
>
>
> On 11/6/2012 4:51 PM, Michael Spencer wrote:
>
> On Tue, Nov 6, 2012 at 8:30 AM, Shankar Easwaran
> <shankare at codeaurora.org> wrote:
>
> Hi Nick, Michael,
>
> I incorporated your comments in lld / llvm for supporting reading archive
> libraries.
>
> Please look at the new changes and let me know if you are ok to commit this
> in.
>
> Thanks
>
> Shankar Easwaran
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
> the Linux Foundation
>
> Index: lib/Object/Archive.cpp
> ===================================================================
> --- lib/Object/Archive.cpp (revision 167054)
> +++ lib/Object/Archive.cpp (working copy)
> @@ -122,7 +122,15 @@
> + sizeof(ArchiveMemberHeader)
> + Parent->StringTable->getSize()))
> return object_error::parse_failed;
> +
> + // The filename would exist in the StringTable only if the filename
> + // is a long filename. The filename be terminated by a /
>
> This should read:
>
> GNU long file names end with a /.
>
> + if (Parent->kind() == K_GNU) {
> + StringRef::size_type end = StringRef(addr).find('/');
>
> End
>
> + Result = StringRef(addr, end);
> + } else {
> Result = addr;
>
> This needs to be indented.
>
> + }
> return object_error::success;
> } else if (name.startswith("#1/")) {
> APInt name_size;
> @@ -187,7 +195,40 @@
> child_iterator i = begin_children(false);
> child_iterator e = end_children();
>
> - if (i != e) ++i; // Nobody cares about the first member.
> + StringRef name;
> + if ((ec = i->getName(name)))
> + return;
> +
> + // Below is the pattern that is used to figure out the archive format
> + // GNU archive format
> + // First member : / (points to the symbol table )
> + // Second member : // (may exist, if it exists, points to the string
> table)
> + // Note : The string table is used if the filename exceeds 15 characters
> + // BSD archive format
> + // First member : __.SYMDEF (points to the symbol table)
> + // There is no string table, if the filename exceeds 15 characters or
> has a
> + // embedded space, the filename has #1/<size>, The size represents the
> size
> + // of the filename that needs to be read after the archive header
> + // COFF archive format
>
> This should be spaced the same as GNU archive format and BSD archive format.
>
> + // First member : /
> + // Second member : / (provides a directory of symbols)
> + // Third member : // contains the string table, this is present even if
> the
> + // string table is empty
> + if (name == "/") {
> + SymbolTable = i;
> + StringTable = e;
> + if (i != e) ++i;
> + if ((ec = i->getName(name)))
> + return;
> + const char *data = name.data();
> + if (data[0] != '/') {
>
> You don't need to convert to a const char * to do this. name[0] is the same.
>
> + Format = K_GNU;
> + } else if (data[1] == '/') {
> + Format = K_GNU;
> + StringTable = i;
> + ++i;
> + } else {
> + Format = K_COFF;
> if (i != e) {
> SymbolTable = i;
> ++i;
> @@ -195,7 +236,12 @@
> if (i != e) {
> StringTable = i;
> }
>
> This needs to be indented.
>
> -
> + }
> + } else if (name == "__.SYMDEF") {
> + Format = K_BSD;
> + SymbolTable = i;
> + StringTable = e;
> + }
> ec = object_error::success;
> }
>
> @@ -222,17 +268,28 @@
>
> error_code Archive::Symbol::getMember(child_iterator &Result) const {
> const char *buf = Parent->SymbolTable->getBuffer()->getBufferStart();
> - uint32_t member_count = *reinterpret_cast<const
> support::ulittle32_t*>(buf);
> const char *offsets = buf + 4;
> + uint32_t offset = 0;
> + if (Parent->kind() == K_GNU) {
> + offset = *(reinterpret_cast<const support::ubig32_t*>(offsets)
> + + SymbolIndex);
> + } else if (Parent->kind() == K_BSD) {
> + assert("BSD format is not supported");
> + } else {
> + uint32_t member_count = 0;
> + member_count = *reinterpret_cast<const support::ulittle32_t*>(buf);
> buf += 4 + (member_count * 4); // Skip offsets.
> const char *indicies = buf + 4;
> -
> uint16_t offsetindex =
> *(reinterpret_cast<const support::ulittle16_t*>(indicies)
> + SymbolIndex);
>
> Indentation is wrong.
>
> -
> - uint32_t offset = *(reinterpret_cast<const
> support::ulittle32_t*>(offsets)
> + uint32_t *offsetaddr =
> + (uint32_t *)(reinterpret_cast<const
> support::ulittle32_t*>(offsets)
> + (offsetindex - 1));
> + assert((const char *)offsetaddr <
> + Parent->SymbolTable->getBuffer()->getBufferEnd());
> + offset = *(offsetaddr);
> + }
>
> const char *Loc = Parent->getData().begin() + offset;
> size_t Size = sizeof(ArchiveMemberHeader) +
> @@ -253,10 +310,20 @@
>
> Archive::symbol_iterator Archive::begin_symbols() const {
> const char *buf = SymbolTable->getBuffer()->getBufferStart();
> - uint32_t member_count = *reinterpret_cast<const
> support::ulittle32_t*>(buf);
> + if (kind() == K_GNU) {
> + uint32_t symbol_count = 0;
> + symbol_count = *reinterpret_cast<const support::ubig32_t*>(buf);
> + buf += sizeof(uint32_t) + (symbol_count * (sizeof(uint32_t)));
> + } else if (kind() == K_BSD) {
> + assert("BSD archive format is not supported");
> + } else {
> + uint32_t member_count = 0;
> + uint32_t symbol_count = 0;
> + member_count = *reinterpret_cast<const support::ulittle32_t*>(buf);
> buf += 4 + (member_count * 4); // Skip offsets.
>
> Indentation.
>
> - uint32_t symbol_count = *reinterpret_cast<const
> support::ulittle32_t*>(buf);
> + symbol_count = *reinterpret_cast<const support::ulittle32_t*>(buf);
> buf += 4 + (symbol_count * 2); // Skip indices.
>
> Indentation.
>
> + }
> uint32_t string_start_offset =
> buf - SymbolTable->getBuffer()->getBufferStart();
> return symbol_iterator(Symbol(this, 0, string_start_offset));
> @@ -264,9 +331,37 @@
>
> Archive::symbol_iterator Archive::end_symbols() const {
> const char *buf = SymbolTable->getBuffer()->getBufferStart();
> - uint32_t member_count = *reinterpret_cast<const
> support::ulittle32_t*>(buf);
> + uint32_t symbol_count = 0;
> + if (kind() == K_GNU) {
> + symbol_count = *reinterpret_cast<const support::ubig32_t*>(buf);
> + buf += sizeof(uint32_t) + (symbol_count * (sizeof(uint32_t)));
> + } else if (kind() == K_BSD) {
> + assert("BSD archive format is not supported");
> + } else {
> + uint32_t member_count = 0;
> + member_count = *reinterpret_cast<const support::ulittle32_t*>(buf);
> buf += 4 + (member_count * 4); // Skip offsets.
>
> Indentation.
>
> - uint32_t symbol_count = *reinterpret_cast<const
> support::ulittle32_t*>(buf);
> + symbol_count = *reinterpret_cast<const support::ulittle32_t*>(buf);
> + }
> return symbol_iterator(
> Symbol(this, symbol_count, 0));
> }
> +
> +error_code Archive::findSym(StringRef name, child_iterator &result) const {
> + error_code ec;
> + StringRef symname;
> + Archive::symbol_iterator bs = begin_symbols();
> + Archive::symbol_iterator es = end_symbols();
> +
> + for (; bs != es; ++bs)
> + {
>
> No newline before {.
>
> + if ((ec = bs->getName(symname)))
> + return ec;
> + if (symname == name) {
> + if ((ec = bs->getMember(result)))
> + return ec;
> + return error_code::success();
> + }
> + }
> + return object_error::parse_failed;
>
> This is not the right error code. parse_failed means that the file is
> corrupt. This should return a child_iterator.
>
> +}
>
> - Michael Spencer
>
>
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
> the Linux Foundation
More information about the llvm-commits
mailing list