Skip to content

Commit

Permalink
[llvm-ar][libObject] Fix relative paths when nesting thin archives.
Browse files Browse the repository at this point in the history 
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

llvm-svn: 353424
  • Loading branch information
@rupprecht
rupprecht committed Feb 7, 2019
1 parent b88144e commit bf990ab
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 67 deletions.
1 change: 0 additions & 1 deletion llvm/include/llvm/Object/ArchiveWriter.h
View file
Expand Up @@ -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);

Expand Down
71 changes: 12 additions & 59 deletions llvm/lib/Object/ArchiveWriter.cpp
View file
Expand Up @@ -48,7 +48,6 @@ NewArchiveMember::getOldMember(const object::Archive::Child &OldMember,
return BufOrErr.takeError();

NewArchiveMember M;
assert(M.IsNew == false);
M.Buf = MemoryBuffer::getMemBuffer(*BufOrErr, false);
M.MemberName = M.Buf->getBufferIdentifier();
if (!Deterministic) {
Expand Down Expand Up @@ -98,7 +97,6 @@ Expected<NewArchiveMember> NewArchiveMember::getFile(StringRef FileName,
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) {
Expand Down Expand Up @@ -191,35 +189,6 @@ static bool useStringTable(bool Thin, StringRef Name) {
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:
Expand All @@ -234,27 +203,11 @@ static bool is64BitKind(object::Archive::Kind Kind) {
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);
Expand All @@ -265,12 +218,12 @@ static void printMemberHeader(raw_ostream &Out, uint64_t Pos,
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;
}
Expand Down Expand Up @@ -432,8 +385,8 @@ getSymbols(MemoryBufferRef Buf, raw_ostream &SymNames, bool &HasObject) {

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
Expand Down Expand Up @@ -520,8 +473,8 @@ computeMemberData(raw_ostream &StringTable, raw_ostream &SymNames,
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 =
Expand Down Expand Up @@ -553,7 +506,7 @@ Error llvm::writeArchive(StringRef ArcName,
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;
Expand Down
15 changes: 15 additions & 0 deletions llvm/test/tools/llvm-ar/flatten-thin-archive-directories.test
View file
@@ -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
2 changes: 1 addition & 1 deletion llvm/test/tools/llvm-ar/flatten-thin-archive.test
View file
Expand Up @@ -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
Expand Down
57 changes: 51 additions & 6 deletions llvm/tools/llvm-ar/llvm-ar.cpp
View file
Expand Up @@ -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() {
Expand Down Expand Up @@ -537,6 +540,35 @@ static void performReadOperation(ArchiveOperation Operation,
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) {
Expand All @@ -545,6 +577,15 @@ static void addChildMember(std::vector<NewArchiveMember> &Members,
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();
Expand All @@ -568,6 +609,13 @@ static void addMember(std::vector<NewArchiveMember> &Members,
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);
Expand All @@ -581,8 +629,6 @@ static void addMember(std::vector<NewArchiveMember> &Members,
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));
}

Expand Down Expand Up @@ -672,15 +718,15 @@ computeNewArchiveMembers(ArchiveOperation Operation,
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);
break;
case IA_Delete:
break;
case IA_MoveOldMember:
addChildMember(Moved, Child);
addChildMember(Moved, Child, /*FlattenArchive=*/Thin);
break;
case IA_MoveNewMember:
addMember(Moved, *MemberI);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -951,7 +997,6 @@ static bool handleGenericOption(StringRef arg) {

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) {
Expand Down

0 comments on commit bf990ab

Please sign in to comment.