<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Feb 21, 2017 at 12:40 PM, Rafael Espindola via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rafael<br>
Date: Tue Feb 21 14:40:54 2017<br>
New Revision: 295765<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=295765&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=295765&view=rev</a><br>
Log:<br>
Don't modify archive members unless really needed.<br>
<br>
For whatever reason ld64 requires that member headers (not the member<br>
themselves) should be aligned. The only way to do that is to edit the<br>
previous member so that it ends at an aligned boundary.<br>
<br>
Since modifying data put in an archive is an undesirable property,<br>
llvm-ar should only do it when it is absolutely necessary.<br>
<br>
Added:<br>
llvm/trunk/test/Object/<wbr>archive-pad.test<br>
Modified:<br>
llvm/trunk/include/llvm/<wbr>Object/Archive.h<br>
llvm/trunk/lib/Object/<wbr>ArchiveWriter.cpp<br>
llvm/trunk/test/Object/<wbr>archive-extract.test<br>
llvm/trunk/test/Object/<wbr>archive-format.test<br>
llvm/trunk/tools/llvm-ar/llvm-<wbr>ar.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/<wbr>Object/Archive.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/Archive.h?rev=295765&r1=295764&r2=295765&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/include/<wbr>llvm/Object/Archive.h?rev=<wbr>295765&r1=295764&r2=295765&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/include/llvm/<wbr>Object/Archive.h (original)<br>
+++ llvm/trunk/include/llvm/<wbr>Object/Archive.h Tue Feb 21 14:40:54 2017<br>
@@ -212,6 +212,7 @@ public:<br>
K_GNU,<br>
K_MIPS64,<br>
K_BSD,<br>
+ K_DARWIN,<br>
K_DARWIN64,<br>
K_COFF<br>
};<br>
<br>
Modified: llvm/trunk/lib/Object/<wbr>ArchiveWriter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/ArchiveWriter.cpp?rev=295765&r1=295764&r2=295765&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/Object/<wbr>ArchiveWriter.cpp?rev=295765&<wbr>r1=295764&r2=295765&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Object/<wbr>ArchiveWriter.cpp (original)<br>
+++ llvm/trunk/lib/Object/<wbr>ArchiveWriter.cpp Tue Feb 21 14:40:54 2017<br>
@@ -122,12 +122,26 @@ static void printWithSpacePadding(raw_fd<br>
}<br>
}<br>
<br>
+static bool isBSDLike(object::Archive::<wbr>Kind Kind) {<br>
+ switch (Kind) {<br>
+ case object::Archive::K_GNU:<br>
+ return false;<br>
+ case object::Archive::K_BSD:<br>
+ case object::Archive::K_DARWIN:<br>
+ return true;<br>
+ case object::Archive::K_MIPS64:<br>
+ case object::Archive::K_DARWIN64:<br>
+ case object::Archive::K_COFF:<br>
+ llvm_unreachable("not supported for writting");<br>
+ }<br>
+}<br></blockquote><div><br></div><div>This `switch` doesn't cover all Kind enums, which is bad because this function should return a value.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
static void print32(raw_ostream &Out, object::Archive::Kind Kind,<br>
uint32_t Val) {<br>
- if (Kind == object::Archive::K_GNU)<br>
- support::endian::Writer<<wbr>support::big>(Out).write(Val);<br>
- else<br>
+ if (isBSDLike(Kind))<br>
support::endian::Writer<<wbr>support::little>(Out).write(<wbr>Val);<br>
+ else<br>
+ support::endian::Writer<<wbr>support::big>(Out).write(Val);<br>
}<br>
<br>
static void printRestOfMemberHeader(<br>
@@ -178,7 +192,7 @@ printMemberHeader(raw_fd_<wbr>ostream &Out, o<br>
std::vector<unsigned>::<wbr>iterator &StringMapIndexIter,<br>
const sys::TimePoint<std::chrono::<wbr>seconds> &ModTime,<br>
unsigned UID, unsigned GID, unsigned Perms, unsigned Size) {<br>
- if (Kind == object::Archive::K_BSD)<br>
+ if (isBSDLike(Kind))<br>
return printBSDMemberHeader(Out, Name, ModTime, UID, GID, Perms, Size);<br>
if (!useStringTable(Thin, Name))<br>
return printGNUSmallMemberHeader(Out, Name, ModTime, UID, GID, Perms, Size);<br>
@@ -285,10 +299,10 @@ writeSymbolTable(raw_fd_<wbr>ostream &Out, ob<br>
<br>
if (!HeaderStartOffset) {<br>
HeaderStartOffset = Out.tell();<br>
- if (Kind == object::Archive::K_GNU)<br>
- printGNUSmallMemberHeader(Out, "", now(Deterministic), 0, 0, 0, 0);<br>
- else<br>
+ if (isBSDLike(Kind))<br>
printBSDMemberHeader(Out, "__.SYMDEF", now(Deterministic), 0, 0, 0, 0);<br>
+ else<br>
+ printGNUSmallMemberHeader(Out, "", now(Deterministic), 0, 0, 0, 0);<br>
BodyStartOffset = Out.tell();<br>
print32(Out, Kind, 0); // number of entries or bytes<br>
}<br>
@@ -307,7 +321,7 @@ writeSymbolTable(raw_fd_<wbr>ostream &Out, ob<br>
return EC;<br>
NameOS << '\0';<br>
MemberOffsetRefs.push_back(<wbr>MemberNum);<br>
- if (Kind == object::Archive::K_BSD)<br>
+ if (isBSDLike(Kind))<br>
print32(Out, Kind, NameOffset);<br>
print32(Out, Kind, 0); // member offset<br>
}<br>
@@ -318,12 +332,12 @@ writeSymbolTable(raw_fd_<wbr>ostream &Out, ob<br>
<br>
// ld64 prefers the cctools type archive which pads its string table to a<br>
// boundary of sizeof(int32_t).<br>
- if (Kind == object::Archive::K_BSD)<br>
+ if (isBSDLike(Kind))<br>
for (unsigned P = OffsetToAlignment(NameOS.tell(<wbr>), sizeof(int32_t)); P--;)<br>
NameOS << '\0';<br>
<br>
StringRef StringTable = NameOS.str();<br>
- if (Kind == object::Archive::K_BSD)<br>
+ if (isBSDLike(Kind))<br>
print32(Out, Kind, StringTable.size()); // byte count of the string table<br>
Out << StringTable;<br>
<br>
@@ -342,10 +356,10 @@ writeSymbolTable(raw_fd_<wbr>ostream &Out, ob<br>
// Patch up the number of symbols.<br>
Out.seek(BodyStartOffset);<br>
unsigned NumSyms = MemberOffsetRefs.size();<br>
- if (Kind == object::Archive::K_GNU)<br>
- print32(Out, Kind, NumSyms);<br>
- else<br>
+ if (isBSDLike(Kind))<br>
print32(Out, Kind, NumSyms * 8);<br>
+ else<br>
+ print32(Out, Kind, NumSyms);<br>
<br>
Out.seek(Pos);<br>
return BodyStartOffset + 4;<br>
@@ -357,8 +371,7 @@ llvm::writeArchive(StringRef ArcName,<br>
bool WriteSymtab, object::Archive::Kind Kind,<br>
bool Deterministic, bool Thin,<br>
std::unique_ptr<MemoryBuffer> OldArchiveBuf) {<br>
- assert((!Thin || Kind == object::Archive::K_GNU) &&<br>
- "Only the gnu format has a thin mode");<br>
+ assert((!Thin || !isBSDLike(Kind)) && "Only the gnu format has a thin mode");<br>
SmallString<128> TmpArchive;<br>
int TmpArchiveFD;<br>
if (auto EC = sys::fs::createUniqueFile(<wbr>ArcName + ".temp-archive-%%%%%%%.a",<br>
@@ -388,7 +401,7 @@ llvm::writeArchive(StringRef ArcName,<br>
}<br>
<br>
std::vector<unsigned> StringMapIndexes;<br>
- if (Kind != object::Archive::K_BSD)<br>
+ if (!isBSDLike(Kind))<br>
writeStringTable(Out, ArcName, NewMembers, StringMapIndexes, Thin);<br>
<br>
std::vector<unsigned>::<wbr>iterator StringMapIndexIter = StringMapIndexes.begin();<br>
@@ -404,7 +417,7 @@ llvm::writeArchive(StringRef ArcName,<br>
// least 4-byte aligned for 32-bit content. Opt for the larger encoding<br>
// uniformly. This matches the behaviour with cctools and ensures that ld64<br>
// is happy with archives that we generate.<br>
- if (Kind == object::Archive::K_BSD)<br>
+ if (Kind == object::Archive::K_DARWIN)<br>
Padding = OffsetToAlignment(M.Buf-><wbr>getBufferSize(), 8);<br>
<br>
printMemberHeader(Out, Kind, Thin,<br>
@@ -424,7 +437,7 @@ llvm::writeArchive(StringRef ArcName,<br>
if (MemberReferenceOffset) {<br>
Out.seek(<wbr>MemberReferenceOffset);<br>
for (unsigned MemberNum : MemberOffsetRefs) {<br>
- if (Kind == object::Archive::K_BSD)<br>
+ if (isBSDLike(Kind))<br>
Out.seek(Out.tell() + 4); // skip over the string offset<br>
print32(Out, Kind, MemberOffset[MemberNum]);<br>
}<br>
<br>
Modified: llvm/trunk/test/Object/<wbr>archive-extract.test<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/archive-extract.test?rev=295765&r1=295764&r2=295765&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/<wbr>Object/archive-extract.test?<wbr>rev=295765&r1=295764&r2=<wbr>295765&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/Object/<wbr>archive-extract.test (original)<br>
+++ llvm/trunk/test/Object/<wbr>archive-extract.test Tue Feb 21 14:40:54 2017<br>
@@ -43,10 +43,10 @@<br>
CHECK-GNU: 1465 2004-11-19 03:01:31.000000000 very_long_bytecode_file_name.<wbr>bc<br>
<br>
; RUN: rm -f %t.a<br>
-; RUN: llvm-ar -format bsd rcU %t.a very_long_bytecode_file_name.<wbr>bc<br>
-; RUN: env TZ=GMT llvm-ar tv %t.a | FileCheck %s -check-prefix CHECK-BSD<br>
+; RUN: llvm-ar -format darwin rcU %t.a very_long_bytecode_file_name.<wbr>bc<br>
+; RUN: env TZ=GMT llvm-ar tv %t.a | FileCheck %s -check-prefix CHECK-DARWIN<br>
<br>
-CHECK-BSD: 1472 2004-11-19 03:01:31.000000000 very_long_bytecode_file_name.<wbr>bc<br>
+CHECK-DARWIN: 1472 2004-11-19 03:01:31.000000000 very_long_bytecode_file_name.<wbr>bc<br>
<br>
RUN: not llvm-ar x %p/Inputs/GNU.a foo.o 2>&1 | FileCheck --check-prefix=NOTFOUND %s<br>
NOTFOUND: foo.o was not found<br>
<br>
Modified: llvm/trunk/test/Object/<wbr>archive-format.test<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/archive-format.test?rev=295765&r1=295764&r2=295765&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/<wbr>Object/archive-format.test?<wbr>rev=295765&r1=295764&r2=<wbr>295765&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/Object/<wbr>archive-format.test (original)<br>
+++ llvm/trunk/test/Object/<wbr>archive-format.test Tue Feb 21 14:40:54 2017<br>
@@ -32,12 +32,23 @@ RUN: llvm-ar --format=bsd rc %t.a 012345<br>
RUN: cat %t.a | FileCheck -strict-whitespace --check-prefix=BSD %s<br>
<br>
BSD: !<arch><br>
-BSD-NEXT: #1/20 0 0 0 644 28 `<br>
+BSD-NEXT: #1/20 0 0 0 644 24 `<br>
+BSD-NEXT: 0123456789abcde{{.....}}bar.<br>
+BSD-SAME: #1/16 0 0 0 644 20 `<br>
+BSD-NEXT: 0123456789abcdefzed.<br>
+<br>
+RUN: rm -f %t.a<br>
+RUN: llvm-ar --format=darwin rc %t.a 0123456789abcde 0123456789abcdef<br>
+RUN: cat %t.a | FileCheck -strict-whitespace --check-prefix=DARWIN %s<br>
+<br>
+DARWIN: !<arch><br>
+DARWIN-NEXT: #1/20 0 0 0 644 28 `<br>
Each [[:space:]] matches a newline. We explicitly match 3 newlines, as the<br>
fourth newline is implicitly consumed by FileCheck and cannot be matched.<br>
-BSD-NEXT: 0123456789abcde{{.....}}bar.{{<wbr>[[:space:]][[:space:]][[:<wbr>space:]]}}<br>
-BSD-NEXT: #1/20 0 0 0 644 28 `<br>
-BSD-NEXT: 0123456789abcdef{{....}}zed.<br>
+DARWIN-NEXT: 0123456789abcde{{.....}}bar.{{<wbr>[[:space:]][[:space:]][[:<wbr>space:]]}}<br>
+DARWIN-NEXT: #1/20 0 0 0 644 28 `<br>
+DARWIN-NEXT: 0123456789abcdef{{....}}zed.<br>
+<br>
<br>
RUN: rm -f test.a<br>
RUN: llvm-ar --format=gnu rcT test.a 0123456789abcde 0123456789abcdef<br>
<br>
Added: llvm/trunk/test/Object/<wbr>archive-pad.test<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/archive-pad.test?rev=295765&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/<wbr>Object/archive-pad.test?rev=<wbr>295765&view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/Object/<wbr>archive-pad.test (added)<br>
+++ llvm/trunk/test/Object/<wbr>archive-pad.test Tue Feb 21 14:40:54 2017<br>
@@ -0,0 +1,19 @@<br>
+Test that only the darwin format needs to modify archive members to<br>
+avoid a ld64 bug.<br>
+<br>
+RUN: echo foo > %t.o<br>
+<br>
+RUN: rm -f %t.a<br>
+RUN: llvm-ar -format=bsd rc %t.a %t.o<br>
+RUN: llvm-ar p %t.a > %bsd.o<br>
+RUN: cmp %bsd.o %t.o<br>
+<br>
+RUN: rm -f %t.a<br>
+RUN: llvm-ar -format=gnu rc %t.a %t.o<br>
+RUN: llvm-ar p %t.a > %gnu.o<br>
+RUN: cmp %gnu.o %t.o<br>
+<br>
+RUN: rm -f %t.a<br>
+RUN: llvm-ar -format=darwin rc %t.a %t.o<br>
+RUN: llvm-ar p %t.a > %darwin.o<br>
+RUN: not cmp %darwin.o %t.o<br>
<br>
Modified: llvm/trunk/tools/llvm-ar/llvm-<wbr>ar.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-ar/llvm-ar.cpp?rev=295765&r1=295764&r2=295765&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/tools/llvm-<wbr>ar/llvm-ar.cpp?rev=295765&r1=<wbr>295764&r2=295765&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/tools/llvm-ar/llvm-<wbr>ar.cpp (original)<br>
+++ llvm/trunk/tools/llvm-ar/llvm-<wbr>ar.cpp Tue Feb 21 14:40:54 2017<br>
@@ -87,13 +87,14 @@ static cl::opt<bool> MRI("M", cl::desc("<br>
static cl::opt<std::string> Plugin("plugin", cl::desc("plugin (ignored for compatibility"));<br>
<br>
namespace {<br>
-enum Format { Default, GNU, BSD };<br>
+enum Format { Default, GNU, BSD, DARWIN };<br>
}<br>
<br>
static cl::opt<Format><br>
FormatOpt("format", cl::desc("Archive format to create"),<br>
cl::values(clEnumValN(Default, "default", "default"),<br>
clEnumValN(GNU, "gnu", "gnu"),<br>
+ clEnumValN(DARWIN, "darwin", "darwin"),<br>
clEnumValN(BSD, "bsd", "bsd")));<br>
<br>
static std::string Options;<br>
@@ -623,8 +624,9 @@ computeNewArchiveMembers(<wbr>ArchiveOperatio<br>
}<br>
<br>
static object::Archive::Kind getDefaultForHost() {<br>
- return Triple(sys::getProcessTriple()<wbr>).isOSDarwin() ? object::Archive::K_BSD<br>
- : object::Archive::K_GNU;<br>
+ return Triple(sys::getProcessTriple()<wbr>).isOSDarwin()<br>
+ ? object::Archive::K_DARWIN<br>
+ : object::Archive::K_GNU;<br>
}<br>
<br>
static object::Archive::Kind getKindFromMember(const NewArchiveMember &Member) {<br>
@@ -633,7 +635,7 @@ static object::Archive::Kind getKindFrom<br>
<br>
if (OptionalObject)<br>
return isa<object::MachOObjectFile>(*<wbr>*OptionalObject)<br>
- ? object::Archive::K_BSD<br>
+ ? object::Archive::K_DARWIN<br>
: object::Archive::K_GNU;<br>
<br>
// squelch the error in case we had a non-object file<br>
@@ -672,6 +674,11 @@ performWriteOperation(<wbr>ArchiveOperation O<br>
fail("Only the gnu format has a thin mode");<br>
Kind = object::Archive::K_BSD;<br>
break;<br>
+ case DARWIN:<br>
+ if (Thin)<br>
+ fail("Only the gnu format has a thin mode");<br>
+ Kind = object::Archive::K_DARWIN;<br>
+ break;<br>
}<br>
<br>
std::pair<StringRef, std::error_code> Result =<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>