[llvm-commits] [lld] Supporting archive libraries in lld
Nick Kledzik
kledzik at apple.com
Wed Nov 7 11:47:35 PST 2012
Shankar,
The lld diffs can be committed after two fixes:
1) Put a comment in the archive test cases saying that they will be changed to not required binary files to be checked in once we have a way to generate binary archives from textual (yaml) input.
2) FileArchive should not be a public class in lld/Core. It should be a local class in ReaderArchive.cpp.
-Nick
On Nov 7, 2012, at 10:07 AM, Shankar Easwaran 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
>
> 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
> <llvm.diff><lld.diff>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121107/fb4e55bc/attachment.html>
More information about the llvm-commits
mailing list