[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

llvm-svn: 362407

include/llvm/Object/ArchiveWriter.h

@@ -36,7 +36,7 @@ struct NewArchiveMember {
                                             bool Deterministic);
 };
 
-std::string computeArchiveRelativePath(StringRef From, StringRef To);
+Expected<std::string> computeArchiveRelativePath(StringRef From, StringRef To);
 
 Error writeArchive(StringRef ArcName, ArrayRef<NewArchiveMember> NewMembers,
                    bool WriteSymtab, object::Archive::Kind Kind,

lib/Object/ArchiveWriter.cpp

@@ -494,29 +494,46 @@ computeMemberData(raw_ostream &StringTable, raw_ostream &SymNames,
 }
 
 namespace llvm {
+
+static ErrorOr<SmallString<128>> 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;
+Expected<std::string> computeArchiveRelativePath(StringRef From, StringRef To) {
+  ErrorOr<SmallString<128>> PathToOrErr = canonicalizePath(To);
+  ErrorOr<SmallString<128>> DirFromOrErr = canonicalizePath(From);
+  if (!PathToOrErr || !DirFromOrErr)
+    return errorCodeToError(std::error_code(errno, std::generic_category()));
 
-  StringRef DirFrom = sys::path::parent_path(From);
-  auto FromI = sys::path::begin(DirFrom);
-  auto ToI = sys::path::begin(To);
-  while (*FromI == *ToI) {
-    ++FromI;
-    ++ToI;
-  }
+  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<NewArchiveMember> NewMembers,

lib/ToolDrivers/llvm-lib/LibDriver.cpp

@@ -211,9 +211,14 @@ int llvm::libDriverMain(ArrayRef<const char *> 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<std::string> PathOrErr =
+          computeArchiveRelativePath(OutputPath, Member.MemberName);
+      if (PathOrErr)
+        Member.MemberName = Saver.save(*PathOrErr);
+    }
+  }
 
   if (Error E =
           writeArchive(OutputPath, Members,

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/

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: !<thin>
+
+CHECK-NOT: delete.o
+CHECK: {{^}}[[PATH]]foo/elf.o
+CHECK: {{^}}[[PATH]]foo/bar/elf.o

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

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

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<NewArchiveMember> &Members,
   // the archive it's in, so the file resolves correctly.
   if (Thin && FlattenArchive) {
     StringSaver Saver(Alloc);
-    Expected<std::string> FileNameOrErr = M.getFullName();
+    Expected<std::string> 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<std::string> 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<NewArchiveMember> &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<std::string> 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);