From ed6564d3853cd55ce7e06301eee524b8b66db0c9 Mon Sep 17 00:00:00 2001 From: Owen Reynolds Date: Mon, 3 Jun 2019 15:26:07 +0000 Subject: [PATCH] [llvm-ar] Fix relative thin archive path handling This fixes some thin archive relative path issues, paths are shortened where possible and paths are output correctly when using the display table command. Differential Revision: https://reviews.llvm.org/D59491 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@362407 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Object/ArchiveWriter.h | 2 +- lib/Object/ArchiveWriter.cpp | 51 ++++++++++++------- lib/ToolDrivers/llvm-lib/LibDriver.cpp | 11 ++-- test/tools/llvm-ar/reduce-thin-path.test | 10 ++++ test/tools/llvm-ar/thin-archive.test | 45 ++++++++++++++++ .../ELF/archive-unknown-members.test | 8 +-- .../llvm-readobj/thin-archive-paths.test | 6 +-- tools/llvm-ar/llvm-ar.cpp | 38 ++++++++++---- 8 files changed, 134 insertions(+), 37 deletions(-) create mode 100644 test/tools/llvm-ar/reduce-thin-path.test create mode 100644 test/tools/llvm-ar/thin-archive.test diff --git a/include/llvm/Object/ArchiveWriter.h b/include/llvm/Object/ArchiveWriter.h index cf415e92bc7..9e6daf2da36 100644 --- a/include/llvm/Object/ArchiveWriter.h +++ b/include/llvm/Object/ArchiveWriter.h @@ -36,7 +36,7 @@ struct NewArchiveMember { bool Deterministic); }; -std::string computeArchiveRelativePath(StringRef From, StringRef To); +Expected computeArchiveRelativePath(StringRef From, StringRef To); Error writeArchive(StringRef ArcName, ArrayRef NewMembers, bool WriteSymtab, object::Archive::Kind Kind, diff --git a/lib/Object/ArchiveWriter.cpp b/lib/Object/ArchiveWriter.cpp index 849d2835772..68c40054bb9 100644 --- a/lib/Object/ArchiveWriter.cpp +++ b/lib/Object/ArchiveWriter.cpp @@ -494,29 +494,46 @@ computeMemberData(raw_ostream &StringTable, raw_ostream &SymNames, } namespace llvm { + +static ErrorOr> canonicalizePath(StringRef P) { + SmallString<128> Ret = P; + std::error_code Err = sys::fs::make_absolute(Ret); + if (Err) + return Err; + sys::path::remove_dots(Ret, /*removedotdot*/ true); + return Ret; +} + // 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; - } +Expected computeArchiveRelativePath(StringRef From, StringRef To) { + ErrorOr> PathToOrErr = canonicalizePath(To); + ErrorOr> DirFromOrErr = canonicalizePath(From); + if (!PathToOrErr || !DirFromOrErr) + return errorCodeToError(std::error_code(errno, std::generic_category())); + + const SmallString<128> &PathTo = *PathToOrErr; + const SmallString<128> &DirFrom = sys::path::parent_path(*DirFromOrErr); + + // Can't construct a relative path between different roots + if (sys::path::root_name(PathTo) != sys::path::root_name(DirFrom)) + return sys::path::convert_to_slash(PathTo); + + // Skip common prefixes + auto FromTo = + std::mismatch(sys::path::begin(DirFrom), sys::path::end(DirFrom), + sys::path::begin(PathTo), sys::path::end(PathTo)); + auto FromI = FromTo.first; + auto ToI = FromTo.second; + // Construct relative path SmallString<128> Relative; for (auto FromE = sys::path::end(DirFrom); FromI != FromE; ++FromI) - sys::path::append(Relative, ".."); + sys::path::append(Relative, sys::path::Style::posix, ".."); - for (auto ToE = sys::path::end(To); ToI != ToE; ++ToI) - sys::path::append(Relative, *ToI); + for (auto ToE = sys::path::end(PathTo); ToI != ToE; ++ToI) + sys::path::append(Relative, sys::path::Style::posix, *ToI); - // Replace backslashes with slashes so that the path is portable between *nix - // and Windows. - return sys::path::convert_to_slash(Relative); + return Relative.str(); } Error writeArchive(StringRef ArcName, ArrayRef NewMembers, diff --git a/lib/ToolDrivers/llvm-lib/LibDriver.cpp b/lib/ToolDrivers/llvm-lib/LibDriver.cpp index 34a83147a3a..2d44686dd28 100644 --- a/lib/ToolDrivers/llvm-lib/LibDriver.cpp +++ b/lib/ToolDrivers/llvm-lib/LibDriver.cpp @@ -211,9 +211,14 @@ int llvm::libDriverMain(ArrayRef ArgsArr) { // 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)); + for (NewArchiveMember &Member : Members) { + if (sys::path::is_relative(Member.MemberName)) { + Expected PathOrErr = + computeArchiveRelativePath(OutputPath, Member.MemberName); + if (PathOrErr) + Member.MemberName = Saver.save(*PathOrErr); + } + } if (Error E = writeArchive(OutputPath, Members, diff --git a/test/tools/llvm-ar/reduce-thin-path.test b/test/tools/llvm-ar/reduce-thin-path.test new file mode 100644 index 00000000000..aea6101ce9b --- /dev/null +++ b/test/tools/llvm-ar/reduce-thin-path.test @@ -0,0 +1,10 @@ +RUN: rm -rf %t && mkdir -p %t/foo/bar/ +RUN: mkdir -p %t/baz/ +RUN: yaml2obj %S/Inputs/elf.yaml -o %t/elf.o + +RUN: cd %t && llvm-ar rTc %t/baz/internal.ar elf.o +RUN: cd %t/foo && llvm-ar rTc %t/foo/bar/external.ar ../baz/internal.ar + +RUN: FileCheck -input-file=%t/foo/bar/external.ar %s + +CHECK: {{^}}../../elf.o/ diff --git a/test/tools/llvm-ar/thin-archive.test b/test/tools/llvm-ar/thin-archive.test new file mode 100644 index 00000000000..8d9543b6869 --- /dev/null +++ b/test/tools/llvm-ar/thin-archive.test @@ -0,0 +1,45 @@ +RUN: rm -rf %t && mkdir -p %t/foo/bar/ + +RUN: yaml2obj %S/Inputs/elf.yaml -o %t/foo/elf.o +RUN: cp %t/foo/elf.o %t/foo/bar/elf.o +RUN: cp %t/foo/bar/elf.o %t/delete.o + +Test that modules can be added with absolute paths when the archive is created using an absolute path + +RUN: llvm-ar rTc %t/absolute-1.ar %t/foo/elf.o %t/delete.o %t/foo/bar/elf.o +RUN: llvm-ar dT %t/absolute-1.ar delete.o + +RUN: FileCheck -input-file=%t/absolute-1.ar --check-prefixes=THIN,CHECK %s -DPATH=%/t/ +RUN: llvm-ar t %t/absolute-1.ar | FileCheck %s -DPATH=%/t/ + +Test that modules can be added with absolute paths when the archive is created using a relative path + +RUN: llvm-ar rTc Output/%basename_t.tmp/absolute-2.ar %t/foo/elf.o %t/delete.o %t/foo/bar/elf.o +RUN: llvm-ar dT Output/%basename_t.tmp/absolute-2.ar %t/delete.o + +RUN: FileCheck -input-file=%t/absolute-2.ar --check-prefixes=THIN,CHECK %s -DPATH=%/t/ +RUN: llvm-ar t %t/absolute-2.ar | FileCheck %s -DPATH=%/t/ + +These tests must be run in %t/foo. cd %t is included on each line to make debugging this test case easier. + +Test that modules can be added with relative paths when the archive is created using a relative path + +RUN: cd %t/foo && llvm-ar rTc ../relative-1.ar elf.o ../delete.o bar/elf.o +RUN: cd %t/foo && llvm-ar dT ../relative-1.ar delete.o + +RUN: FileCheck -input-file=%t/relative-1.ar --check-prefixes=THIN,CHECK %s -DPATH= +RUN: llvm-ar t %t/relative-1.ar | FileCheck %s -DPATH=%/t/ + +Test that modules can be added with relative paths when the archive is created using a absolute path + +RUN: cd %t/foo && llvm-ar rTc %t/relative-2.ar elf.o ../delete.o bar/elf.o +RUN: cd %t/foo && llvm-ar dT %t/relative-2.ar delete.o + +RUN: FileCheck -input-file=%t/relative-2.ar --check-prefixes=THIN,CHECK %s -DPATH= +RUN: llvm-ar t %t/relative-2.ar | FileCheck %s -DPATH=%/t/ + +THIN: ! + +CHECK-NOT: delete.o +CHECK: {{^}}[[PATH]]foo/elf.o +CHECK: {{^}}[[PATH]]foo/bar/elf.o diff --git a/test/tools/llvm-objcopy/ELF/archive-unknown-members.test b/test/tools/llvm-objcopy/ELF/archive-unknown-members.test index 6540b630f7d..39a6597a83b 100644 --- a/test/tools/llvm-objcopy/ELF/archive-unknown-members.test +++ b/test/tools/llvm-objcopy/ELF/archive-unknown-members.test @@ -23,10 +23,10 @@ # RUN: llvm-ar rcT %t.thin1.a %t1.o %s # RUN: llvm-ar rcT %t.thin2.a %t2.o %s -# RUN: not llvm-objcopy --strip-debug %t.thin1.a 2>&1 \ -# RUN: | FileCheck %s --check-prefix=THIN -DARCHIVE=%t.thin1.a -DMEMBER=%s -# RUN: not llvm-strip --strip-debug %t.thin2.a 2>&1 \ -# RUN: | FileCheck %s --check-prefix=THIN -DARCHIVE=%t.thin2.a -DMEMBER=%s +# RUN: not llvm-objcopy --strip-debug %/t.thin1.a 2>&1 \ +# RUN: | FileCheck %s --check-prefix=THIN -DARCHIVE=%/t.thin1.a -DMEMBER=%/s +# RUN: not llvm-strip --strip-debug %/t.thin2.a 2>&1 \ +# RUN: | FileCheck %s --check-prefix=THIN -DARCHIVE=%/t.thin2.a -DMEMBER=%/s ## Verify that the first member was not modified, if a later member could not ## be recognized. # RUN: cmp %t.o %t1.o diff --git a/test/tools/llvm-readobj/thin-archive-paths.test b/test/tools/llvm-readobj/thin-archive-paths.test index f1952c739cc..d7a971eb303 100644 --- a/test/tools/llvm-readobj/thin-archive-paths.test +++ b/test/tools/llvm-readobj/thin-archive-paths.test @@ -23,11 +23,11 @@ # RUN: llvm-ar rcT c/absolute.a %t/a/b/1.o # Show that absolute paths in the file header printing are correct. -# RUN: llvm-readobj --file-headers c/absolute.a | FileCheck %s --check-prefix=ABS -DDIR=%t +# RUN: llvm-readobj --file-headers c/absolute.a | FileCheck %s --check-prefix=ABS -DDIR=%/t # ABS: File: [[DIR]]/a/b/1.o # Show that absolute paths in an error message for both archive and member are correct. # RUN: rm a/b/1.o -# RUN: not llvm-readobj --file-headers %t/c/absolute.a 2>&1 | FileCheck %s --check-prefix=ERR2 -DDIR=%t -# RUN: not llvm-readelf --file-headers %t/c/absolute.a 2>&1 | FileCheck %s --check-prefix=ERR2 -DDIR=%t +# RUN: not llvm-readobj --file-headers %/t/c/absolute.a 2>&1 | FileCheck %s --check-prefix=ERR2 -DDIR=%/t +# RUN: not llvm-readelf --file-headers %/t/c/absolute.a 2>&1 | FileCheck %s --check-prefix=ERR2 -DDIR=%/t # ERR2: error: '[[DIR]]/c/absolute.a': '[[DIR]]/a/b/1.o': {{[Nn]}}o such file or directory diff --git a/tools/llvm-ar/llvm-ar.cpp b/tools/llvm-ar/llvm-ar.cpp index 04c2396a4fa..0731f35ac45 100644 --- a/tools/llvm-ar/llvm-ar.cpp +++ b/tools/llvm-ar/llvm-ar.cpp @@ -464,9 +464,11 @@ static void doDisplayTable(StringRef Name, const object::Archive::Child &C) { } if (C.getParent()->isThin()) { - StringRef ParentDir = sys::path::parent_path(ArchiveName); - if (!ParentDir.empty()) - outs() << ParentDir << '/'; + if (!sys::path::is_absolute(Name)) { + StringRef ParentDir = sys::path::parent_path(ArchiveName); + if (!ParentDir.empty()) + outs() << sys::path::convert_to_slash(ParentDir) << '/'; + } } outs() << Name << "\n"; } @@ -593,10 +595,18 @@ static void addChildMember(std::vector &Members, // the archive it's in, so the file resolves correctly. if (Thin && FlattenArchive) { StringSaver Saver(Alloc); - Expected FileNameOrErr = M.getFullName(); + Expected FileNameOrErr = M.getName(); failIfError(FileNameOrErr.takeError()); - NMOrErr->MemberName = - Saver.save(computeArchiveRelativePath(ArchiveName, *FileNameOrErr)); + if (sys::path::is_absolute(*FileNameOrErr)) { + NMOrErr->MemberName = Saver.save(sys::path::convert_to_slash(*FileNameOrErr)); + } else { + FileNameOrErr = M.getFullName(); + failIfError(FileNameOrErr.takeError()); + Expected PathOrErr = + computeArchiveRelativePath(ArchiveName, *FileNameOrErr); + NMOrErr->MemberName = Saver.save( + PathOrErr ? *PathOrErr : sys::path::convert_to_slash(*FileNameOrErr)); + } } if (FlattenArchive && identify_magic(NMOrErr->Buf->getBuffer()) == file_magic::archive) { @@ -625,9 +635,19 @@ static void addMember(std::vector &Members, // 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 (!Thin) { + NMOrErr->MemberName = sys::path::filename(NMOrErr->MemberName); + } else { + if (sys::path::is_absolute(FileName)) + NMOrErr->MemberName = Saver.save(sys::path::convert_to_slash(FileName)); + else { + Expected PathOrErr = + computeArchiveRelativePath(ArchiveName, FileName); + NMOrErr->MemberName = Saver.save( + PathOrErr ? *PathOrErr : sys::path::convert_to_slash(FileName)); + } + } + if (FlattenArchive && identify_magic(NMOrErr->Buf->getBuffer()) == file_magic::archive) { object::Archive &Lib = readLibrary(FileName); -- 2.40.0