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