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