[llvm] r353995 - [llvm-ar][libObject] Fix relative paths when nesting thin archives.
Jordan Rupprecht via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 13 15:39:41 PST 2019
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) {
More information about the llvm-commits
mailing list