[llvm] r353424 - [llvm-ar][libObject] Fix relative paths when nesting thin archives.

Jordan Rupprecht via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 7 08:41:06 PST 2019


Author: rupprecht
Date: Thu Feb  7 08:41:06 2019
New Revision: 353424

URL: http://llvm.org/viewvc/llvm-project?rev=353424&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

Subscribers: hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D57842

Added:
    llvm/trunk/test/tools/llvm-ar/flatten-thin-archive-directories.test
Modified:
    llvm/trunk/include/llvm/Object/ArchiveWriter.h
    llvm/trunk/lib/Object/ArchiveWriter.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=353424&r1=353423&r2=353424&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Object/ArchiveWriter.h (original)
+++ llvm/trunk/include/llvm/Object/ArchiveWriter.h Thu Feb  7 08:41:06 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);
 

Modified: llvm/trunk/lib/Object/ArchiveWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/ArchiveWriter.cpp?rev=353424&r1=353423&r2=353424&view=diff
==============================================================================
--- llvm/trunk/lib/Object/ArchiveWriter.cpp (original)
+++ llvm/trunk/lib/Object/ArchiveWriter.cpp Thu Feb  7 08:41:06 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 =
@@ -553,7 +506,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;

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=353424&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 Thu Feb  7 08:41:06 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=353424&r1=353423&r2=353424&view=diff
==============================================================================
--- llvm/trunk/test/tools/llvm-ar/flatten-thin-archive.test (original)
+++ llvm/trunk/test/tools/llvm-ar/flatten-thin-archive.test Thu Feb  7 08:41:06 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

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=353424&r1=353423&r2=353424&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-ar/llvm-ar.cpp (original)
+++ llvm/trunk/tools/llvm-ar/llvm-ar.cpp Thu Feb  7 08:41:06 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() {
@@ -537,6 +540,35 @@ static void performReadOperation(Archive
   exit(1);
 }
 
+// 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 void addChildMember(std::vector<NewArchiveMember> &Members,
                            const object::Archive::Child &M,
                            bool FlattenArchive = false) {
@@ -545,6 +577,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(computeRelativePath(ArchiveName, *FileNameOrErr));
+  }
   if (FlattenArchive &&
       identify_magic(NMOrErr->Buf->getBuffer()) == file_magic::archive) {
     Expected<std::string> FileNameOrErr = M.getFullName();
@@ -568,6 +609,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(computeRelativePath(ArchiveName, FileName))
+           : sys::path::filename(NMOrErr->MemberName);
   if (FlattenArchive &&
       identify_magic(NMOrErr->Buf->getBuffer()) == file_magic::archive) {
     object::Archive &Lib = readLibrary(FileName);
@@ -581,8 +629,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 +718,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 +726,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 +945,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 +997,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) {




More information about the llvm-commits mailing list