[llvm] r353995 - [llvm-ar][libObject] Fix relative paths when nesting thin archives.
Saleem Abdulrasool via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 14 10:57:03 PST 2019
Is it not possible to use `sys::path::convert_to_slash` instead of the
`#ifdef _WIN32`? I think at the very least that should be `LLVM_ON_WIN32`
(due to Cygwin's behaviours).
On Wed, Feb 13, 2019 at 3:39 PM Jordan Rupprecht via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: rupprecht
> Date: Wed Feb 13 15:39:41 2019
> New Revision: 353995
>
> URL: http://llvm.org/viewvc/llvm-project?rev=353995&view=rev
> Log:
> [llvm-ar][libObject] Fix relative paths when nesting thin archives.
>
> Summary:
> When adding one thin archive to another, we currently chop off the
> relative path to the flattened members. For instance, when adding
> `foo/child.a` (which contains `x.txt`) to `parent.a`, when flattening it we
> should add it as `foo/x.txt` (which exists) instead of `x.txt` (which does
> not exist).
>
> As a note, this also undoes the `IsNew` parameter of handling relative
> paths in r288280. The unit test there still passes.
>
> This was reported as part of testing the kernel build with llvm-ar:
> https://patchwork.kernel.org/patch/10767545/ (see the second point).
>
> Reviewers: mstorsjo, pcc, ruiu, davide, david2050, inglorion
>
> Reviewed By: ruiu
>
> Subscribers: void, jdoerfert, tpimh, mgorny, hans, nickdesaulniers,
> hiraditya, llvm-commits
>
> Tags: #llvm
>
> Differential Revision: https://reviews.llvm.org/D57842
>
> Added:
> llvm/trunk/test/tools/llvm-ar/flatten-thin-archive-directories.test
> llvm/trunk/test/tools/llvm-lib/thin-relative.test
> Modified:
> llvm/trunk/include/llvm/Object/ArchiveWriter.h
> llvm/trunk/lib/Object/ArchiveWriter.cpp
> llvm/trunk/lib/ToolDrivers/llvm-lib/CMakeLists.txt
> llvm/trunk/lib/ToolDrivers/llvm-lib/LibDriver.cpp
> llvm/trunk/test/tools/llvm-ar/flatten-thin-archive.test
> llvm/trunk/tools/llvm-ar/llvm-ar.cpp
>
> Modified: llvm/trunk/include/llvm/Object/ArchiveWriter.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/ArchiveWriter.h?rev=353995&r1=353994&r2=353995&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Object/ArchiveWriter.h (original)
> +++ llvm/trunk/include/llvm/Object/ArchiveWriter.h Wed Feb 13 15:39:41 2019
> @@ -26,7 +26,6 @@ struct NewArchiveMember {
> sys::TimePoint<std::chrono::seconds> ModTime;
> unsigned UID = 0, GID = 0, Perms = 0644;
>
> - bool IsNew = false;
> NewArchiveMember() = default;
> NewArchiveMember(MemoryBufferRef BufRef);
>
> @@ -37,6 +36,8 @@ struct NewArchiveMember {
> bool Deterministic);
> };
>
> +std::string computeArchiveRelativePath(StringRef From, StringRef To);
> +
> Error writeArchive(StringRef ArcName, ArrayRef<NewArchiveMember>
> NewMembers,
> bool WriteSymtab, object::Archive::Kind Kind,
> bool Deterministic, bool Thin,
>
> Modified: llvm/trunk/lib/Object/ArchiveWriter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/ArchiveWriter.cpp?rev=353995&r1=353994&r2=353995&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Object/ArchiveWriter.cpp (original)
> +++ llvm/trunk/lib/Object/ArchiveWriter.cpp Wed Feb 13 15:39:41 2019
> @@ -48,7 +48,6 @@ NewArchiveMember::getOldMember(const obj
> return BufOrErr.takeError();
>
> NewArchiveMember M;
> - assert(M.IsNew == false);
> M.Buf = MemoryBuffer::getMemBuffer(*BufOrErr, false);
> M.MemberName = M.Buf->getBufferIdentifier();
> if (!Deterministic) {
> @@ -98,7 +97,6 @@ Expected<NewArchiveMember> NewArchiveMem
> return errorCodeToError(std::error_code(errno,
> std::generic_category()));
>
> NewArchiveMember M;
> - M.IsNew = true;
> M.Buf = std::move(*MemberBufferOrErr);
> M.MemberName = M.Buf->getBufferIdentifier();
> if (!Deterministic) {
> @@ -191,35 +189,6 @@ static bool useStringTable(bool Thin, St
> return Thin || Name.size() >= 16 || Name.contains('/');
> }
>
> -// Compute the relative path from From to To.
> -static std::string computeRelativePath(StringRef From, StringRef To) {
> - if (sys::path::is_absolute(From) || sys::path::is_absolute(To))
> - return To;
> -
> - StringRef DirFrom = sys::path::parent_path(From);
> - auto FromI = sys::path::begin(DirFrom);
> - auto ToI = sys::path::begin(To);
> - while (*FromI == *ToI) {
> - ++FromI;
> - ++ToI;
> - }
> -
> - SmallString<128> Relative;
> - for (auto FromE = sys::path::end(DirFrom); FromI != FromE; ++FromI)
> - sys::path::append(Relative, "..");
> -
> - for (auto ToE = sys::path::end(To); ToI != ToE; ++ToI)
> - sys::path::append(Relative, *ToI);
> -
> -#ifdef _WIN32
> - // Replace backslashes with slashes so that the path is portable
> between *nix
> - // and Windows.
> - std::replace(Relative.begin(), Relative.end(), '\\', '/');
> -#endif
> -
> - return Relative.str();
> -}
> -
> static bool is64BitKind(object::Archive::Kind Kind) {
> switch (Kind) {
> case object::Archive::K_GNU:
> @@ -234,27 +203,11 @@ static bool is64BitKind(object::Archive:
> llvm_unreachable("not supported for writting");
> }
>
> -static void addToStringTable(raw_ostream &Out, StringRef ArcName,
> - const NewArchiveMember &M, bool Thin) {
> - StringRef ID = M.Buf->getBufferIdentifier();
> - if (Thin) {
> - if (M.IsNew)
> - Out << computeRelativePath(ArcName, ID);
> - else
> - Out << ID;
> - } else
> - Out << M.MemberName;
> - Out << "/\n";
> -}
> -
> -static void printMemberHeader(raw_ostream &Out, uint64_t Pos,
> - raw_ostream &StringTable,
> - StringMap<uint64_t> &MemberNames,
> - object::Archive::Kind Kind, bool Thin,
> - StringRef ArcName, const NewArchiveMember
> &M,
> - sys::TimePoint<std::chrono::seconds>
> ModTime,
> - unsigned Size) {
> -
> +static void
> +printMemberHeader(raw_ostream &Out, uint64_t Pos, raw_ostream
> &StringTable,
> + StringMap<uint64_t> &MemberNames, object::Archive::Kind
> Kind,
> + bool Thin, const NewArchiveMember &M,
> + sys::TimePoint<std::chrono::seconds> ModTime, unsigned
> Size) {
> if (isBSDLike(Kind))
> return printBSDMemberHeader(Out, Pos, M.MemberName, ModTime, M.UID,
> M.GID,
> M.Perms, Size);
> @@ -265,12 +218,12 @@ static void printMemberHeader(raw_ostrea
> uint64_t NamePos;
> if (Thin) {
> NamePos = StringTable.tell();
> - addToStringTable(StringTable, ArcName, M, Thin);
> + StringTable << M.MemberName << "/\n";
> } else {
> auto Insertion = MemberNames.insert({M.MemberName, uint64_t(0)});
> if (Insertion.second) {
> Insertion.first->second = StringTable.tell();
> - addToStringTable(StringTable, ArcName, M, Thin);
> + StringTable << M.MemberName << "/\n";
> }
> NamePos = Insertion.first->second;
> }
> @@ -432,8 +385,8 @@ getSymbols(MemoryBufferRef Buf, raw_ostr
>
> static Expected<std::vector<MemberData>>
> computeMemberData(raw_ostream &StringTable, raw_ostream &SymNames,
> - object::Archive::Kind Kind, bool Thin, StringRef
> ArcName,
> - bool Deterministic, ArrayRef<NewArchiveMember>
> NewMembers) {
> + object::Archive::Kind Kind, bool Thin, bool
> Deterministic,
> + ArrayRef<NewArchiveMember> NewMembers) {
> static char PaddingData[8] = {'\n', '\n', '\n', '\n', '\n', '\n', '\n',
> '\n'};
>
> // This ignores the symbol table, but we only need the value mod 8 and
> the
> @@ -520,8 +473,8 @@ computeMemberData(raw_ostream &StringTab
> ModTime = sys::toTimePoint(FilenameCount[M.MemberName]++);
> else
> ModTime = M.ModTime;
> - printMemberHeader(Out, Pos, StringTable, MemberNames, Kind, Thin,
> ArcName,
> - M, ModTime, Buf.getBufferSize() + MemberPadding);
> + printMemberHeader(Out, Pos, StringTable, MemberNames, Kind, Thin, M,
> + ModTime, Buf.getBufferSize() + MemberPadding);
> Out.flush();
>
> Expected<std::vector<unsigned>> Symbols =
> @@ -540,11 +493,40 @@ computeMemberData(raw_ostream &StringTab
> return Ret;
> }
>
> -Error llvm::writeArchive(StringRef ArcName,
> - ArrayRef<NewArchiveMember> NewMembers,
> - bool WriteSymtab, object::Archive::Kind Kind,
> - bool Deterministic, bool Thin,
> - std::unique_ptr<MemoryBuffer> OldArchiveBuf) {
> +namespace llvm {
> +// Compute the relative path from From to To.
> +std::string computeArchiveRelativePath(StringRef From, StringRef To) {
> + if (sys::path::is_absolute(From) || sys::path::is_absolute(To))
> + return To;
> +
> + StringRef DirFrom = sys::path::parent_path(From);
> + auto FromI = sys::path::begin(DirFrom);
> + auto ToI = sys::path::begin(To);
> + while (*FromI == *ToI) {
> + ++FromI;
> + ++ToI;
> + }
> +
> + SmallString<128> Relative;
> + for (auto FromE = sys::path::end(DirFrom); FromI != FromE; ++FromI)
> + sys::path::append(Relative, "..");
> +
> + for (auto ToE = sys::path::end(To); ToI != ToE; ++ToI)
> + sys::path::append(Relative, *ToI);
> +
> +#ifdef _WIN32
> + // Replace backslashes with slashes so that the path is portable
> between *nix
> + // and Windows.
> + std::replace(Relative.begin(), Relative.end(), '\\', '/');
> +#endif
> +
> + return Relative.str();
> +}
> +
> +Error writeArchive(StringRef ArcName, ArrayRef<NewArchiveMember>
> NewMembers,
> + bool WriteSymtab, object::Archive::Kind Kind,
> + bool Deterministic, bool Thin,
> + std::unique_ptr<MemoryBuffer> OldArchiveBuf) {
> assert((!Thin || !isBSDLike(Kind)) && "Only the gnu format has a thin
> mode");
>
> SmallString<0> SymNamesBuf;
> @@ -553,7 +535,7 @@ Error llvm::writeArchive(StringRef ArcNa
> raw_svector_ostream StringTable(StringTableBuf);
>
> Expected<std::vector<MemberData>> DataOrErr = computeMemberData(
> - StringTable, SymNames, Kind, Thin, ArcName, Deterministic,
> NewMembers);
> + StringTable, SymNames, Kind, Thin, Deterministic, NewMembers);
> if (Error E = DataOrErr.takeError())
> return E;
> std::vector<MemberData> &Data = *DataOrErr;
> @@ -630,3 +612,5 @@ Error llvm::writeArchive(StringRef ArcNa
>
> return Temp->keep(ArcName);
> }
> +
> +} // namespace llvm
>
> Modified: llvm/trunk/lib/ToolDrivers/llvm-lib/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ToolDrivers/llvm-lib/CMakeLists.txt?rev=353995&r1=353994&r2=353995&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/ToolDrivers/llvm-lib/CMakeLists.txt (original)
> +++ llvm/trunk/lib/ToolDrivers/llvm-lib/CMakeLists.txt Wed Feb 13 15:39:41
> 2019
> @@ -1,3 +1,10 @@
> +set(LLVM_LINK_COMPONENTS
> + BinaryFormat
> + Object
> + Option
> + Support
> + )
> +
> set(LLVM_TARGET_DEFINITIONS Options.td)
> tablegen(LLVM Options.inc -gen-opt-parser-defs)
> add_public_tablegen_target(LibOptionsTableGen)
>
> Modified: llvm/trunk/lib/ToolDrivers/llvm-lib/LibDriver.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ToolDrivers/llvm-lib/LibDriver.cpp?rev=353995&r1=353994&r2=353995&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/ToolDrivers/llvm-lib/LibDriver.cpp (original)
> +++ llvm/trunk/lib/ToolDrivers/llvm-lib/LibDriver.cpp Wed Feb 13 15:39:41
> 2019
> @@ -208,6 +208,13 @@ int llvm::libDriverMain(ArrayRef<const c
>
> // Create an archive file.
> std::string OutputPath = getOutputPath(&Args, Members[0]);
> + // llvm-lib uses relative paths for both regular and thin archives,
> unlike
> + // standard GNU ar, which only uses relative paths for thin archives and
> + // basenames for regular archives.
> + for (NewArchiveMember &Member : Members)
> + Member.MemberName =
> + Saver.save(computeArchiveRelativePath(OutputPath,
> Member.MemberName));
> +
> if (Error E =
> writeArchive(OutputPath, Members,
> /*WriteSymtab=*/true, object::Archive::K_GNU,
>
> Added: llvm/trunk/test/tools/llvm-ar/flatten-thin-archive-directories.test
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-ar/flatten-thin-archive-directories.test?rev=353995&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/tools/llvm-ar/flatten-thin-archive-directories.test
> (added)
> +++ llvm/trunk/test/tools/llvm-ar/flatten-thin-archive-directories.test
> Wed Feb 13 15:39:41 2019
> @@ -0,0 +1,15 @@
> +# This test creates a thin archive in a directory and adds it to a thin
> archive
> +# in the parent directory. The relative path should be included when
> flattening
> +# the archive.
> +
> +RUN: mkdir -p %t/foo
> +RUN: touch %t/foo/a.txt
> +RUN: rm -f %t/archive.a %t/foo/archive.a
> +
> +# These tests must be run in the same directory as %t/archive.a. cd %t is
> +# included on each line to make debugging this test case easier.
> +RUN: cd %t && llvm-ar rcST foo/archive.a foo/a.txt
> +RUN: cd %t && llvm-ar rcST archive.a foo/archive.a
> +RUN: cd %t && llvm-ar t archive.a | FileCheck %s --match-full-lines
> +
> +CHECK: foo/a.txt
>
> Modified: llvm/trunk/test/tools/llvm-ar/flatten-thin-archive.test
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-ar/flatten-thin-archive.test?rev=353995&r1=353994&r2=353995&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/tools/llvm-ar/flatten-thin-archive.test (original)
> +++ llvm/trunk/test/tools/llvm-ar/flatten-thin-archive.test Wed Feb 13
> 15:39:41 2019
> @@ -6,7 +6,7 @@
> # flattened members appearing together.
>
> RUN: touch %t-a.txt %t-b.txt %t-c.txt %t-d.txt %t-e.txt
> -RUN: rm -f %t-a-plus-b.a %t.a
> +RUN: rm -f %t-a-plus-b.a %t-d-plus-e.a %t.a
> RUN: llvm-ar rcsT %t-a-plus-b.a %t-a.txt %t-b.txt
> RUN: llvm-ar rcs %t-d-plus-e.a %t-d.txt %t-e.txt
> RUN: llvm-ar rcsT %t.a %t-a-plus-b.a %t-c.txt %t-d-plus-e.a
>
> Added: llvm/trunk/test/tools/llvm-lib/thin-relative.test
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-lib/thin-relative.test?rev=353995&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/tools/llvm-lib/thin-relative.test (added)
> +++ llvm/trunk/test/tools/llvm-lib/thin-relative.test Wed Feb 13 15:39:41
> 2019
> @@ -0,0 +1,13 @@
> +RUN: rm -rf %t
> +RUN: mkdir -p %t/foo
> +RUN: cd %t
> +
> +RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o foo/obj.o
> %S/Inputs/a.s
> +
> +RUN: llvm-lib -out:archive.a -llvmlibthin foo/obj.o
> +RUN: llvm-ar t archive.a | FileCheck %s --check-prefix=PARENT-DIR
> --match-full-lines
> +PARENT-DIR: foo/obj.o
> +
> +RUN: llvm-lib -out:foo/archive.a -llvmlibthin foo/obj.o
> +RUN: llvm-ar t foo/archive.a | FileCheck %s --check-prefix=SAME-DIR
> --match-full-lines
> +SAME-DIR: foo/obj.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=353995&r1=353994&r2=353995&view=diff
>
> ==============================================================================
> --- llvm/trunk/tools/llvm-ar/llvm-ar.cpp (original)
> +++ llvm/trunk/tools/llvm-ar/llvm-ar.cpp Wed Feb 13 15:39:41 2019
> @@ -193,6 +193,9 @@ static std::string ArchiveName;
> // on the command line.
> static std::vector<StringRef> Members;
>
> +// Static buffer to hold StringRefs.
> +static BumpPtrAllocator Alloc;
> +
> // Extract the member filename from the command line for the [relpos]
> argument
> // associated with a, b, and i modifiers
> static void getRelPos() {
> @@ -545,6 +548,15 @@ static void addChildMember(std::vector<N
> Expected<NewArchiveMember> NMOrErr =
> NewArchiveMember::getOldMember(M, Deterministic);
> failIfError(NMOrErr.takeError());
> + // If the child member we're trying to add is thin, use the path
> relative to
> + // the archive it's in, so the file resolves correctly.
> + if (Thin && FlattenArchive) {
> + StringSaver Saver(Alloc);
> + Expected<std::string> FileNameOrErr = M.getFullName();
> + failIfError(FileNameOrErr.takeError());
> + NMOrErr->MemberName =
> + Saver.save(computeArchiveRelativePath(ArchiveName,
> *FileNameOrErr));
> + }
> if (FlattenArchive &&
> identify_magic(NMOrErr->Buf->getBuffer()) == file_magic::archive) {
> Expected<std::string> FileNameOrErr = M.getFullName();
> @@ -568,6 +580,13 @@ static void addMember(std::vector<NewArc
> Expected<NewArchiveMember> NMOrErr =
> NewArchiveMember::getFile(FileName, Deterministic);
> failIfError(NMOrErr.takeError(), FileName);
> + StringSaver Saver(Alloc);
> + // For regular archives, use the basename of the object path for the
> member
> + // name. For thin archives, use the full relative paths so the file
> resolves
> + // correctly.
> + NMOrErr->MemberName =
> + Thin ? Saver.save(computeArchiveRelativePath(ArchiveName, FileName))
> + : sys::path::filename(NMOrErr->MemberName);
> if (FlattenArchive &&
> identify_magic(NMOrErr->Buf->getBuffer()) == file_magic::archive) {
> object::Archive &Lib = readLibrary(FileName);
> @@ -581,8 +600,6 @@ static void addMember(std::vector<NewArc
> return;
> }
> }
> - // Use the basename of the object path for the member name.
> - NMOrErr->MemberName = sys::path::filename(NMOrErr->MemberName);
> Members.push_back(std::move(*NMOrErr));
> }
>
> @@ -672,7 +689,7 @@ computeNewArchiveMembers(ArchiveOperatio
> computeInsertAction(Operation, Child, Name, MemberI);
> switch (Action) {
> case IA_AddOldMember:
> - addChildMember(Ret, Child);
> + addChildMember(Ret, Child, /*FlattenArchive=*/Thin);
> break;
> case IA_AddNewMember:
> addMember(Ret, *MemberI);
> @@ -680,7 +697,7 @@ computeNewArchiveMembers(ArchiveOperatio
> case IA_Delete:
> break;
> case IA_MoveOldMember:
> - addChildMember(Moved, Child);
> + addChildMember(Moved, Child, /*FlattenArchive=*/Thin);
> break;
> case IA_MoveNewMember:
> addMember(Moved, *MemberI);
> @@ -899,7 +916,7 @@ static void runMRIScript() {
> {
> Error Err = Error::success();
> for (auto &Member : Lib.children(Err))
> - addChildMember(NewMembers, Member);
> + addChildMember(NewMembers, Member, /*FlattenArchive=*/Thin);
> failIfError(std::move(Err));
> }
> break;
> @@ -951,7 +968,6 @@ static bool handleGenericOption(StringRe
>
> static int ar_main(int argc, char **argv) {
> SmallVector<const char *, 0> Argv(argv, argv + argc);
> - BumpPtrAllocator Alloc;
> StringSaver Saver(Alloc);
> cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv);
> for (size_t i = 1; i < Argv.size(); ++i) {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
--
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190214/6e424c6f/attachment.html>
More information about the llvm-commits
mailing list