Skip to content

Commit c09f6d0

Browse files
committed
[llvm][cas] Fix llvm-cas --ingest symlink issues
* Allow ingesting a symlink to a non-existent file. While this may be unexpected, it is not a filesystem error and we should not assume it will not occur in practice in llvm-cas. * Avoid walking into directories repeatedly when there are symlinks, which could cause stack overflow from recursion or a filesystem error about a path with too many symlinks. (cherry picked from commit 23a1a10)
1 parent 42c0375 commit c09f6d0

File tree

5 files changed

+32
-6
lines changed

5 files changed

+32
-6
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
missing

llvm/test/tools/llvm-cas/Inputs/self

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
.

llvm/test/tools/llvm-cas/ingest-remap.test

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ RUN: rm -rf %t && mkdir -p %t
33
RUN: llvm-cas --cas %t/cas --ingest --prefix-map %S/Inputs=/^input --data %S/Inputs > %t/cas.id
44
RUN: llvm-cas --cas %t/cas --ls-tree-recursive @%t/cas.id | FileCheck %s
55

6+
CHECK: syml
7+
CHECK-SAME: /^input/broken_symlink -> missing
68
CHECK: file
79
CHECK-SAME: /^input/directory/file
810
CHECK: syml
@@ -12,4 +14,6 @@ CHECK-SAME: /^input/oneline
1214
CHECK: file
1315
CHECK-SAME: /^input/oneline-nonewline
1416
CHECK: syml
15-
CHECK-SAME: /^input/sym_dir
17+
CHECK-SAME: /^input/self -> .
18+
CHECK: syml
19+
CHECK-SAME: /^input/sym_dir -> directory

llvm/test/tools/llvm-cas/ingest.test

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ RUN: mkdir %t
44
RUN: llvm-cas --cas %t/cas --ingest --data %S/Inputs > %t/cas.id
55
RUN: llvm-cas --cas %t/cas --ls-tree-recursive @%t/cas.id | FileCheck %s
66

7+
CHECK: syml
8+
CHECK-SAME: broken_symlink -> missing
79
CHECK: file
810
CHECK-SAME: directory/file
911
CHECK: syml
@@ -13,7 +15,9 @@ CHECK-SAME: oneline
1315
CHECK: file
1416
CHECK-SAME: oneline-nonewline
1517
CHECK: syml
16-
CHECK-SAME: sym_dir
18+
CHECK-SAME: self -> .
19+
CHECK: syml
20+
CHECK-SAME: sym_dir -> directory
1721

1822
RUN: llvm-cas --cas %t/cas --get-cas-id --data %S/Inputs/directory/file @%t/cas.id > %t/file.casid
1923
RUN: llvm-cas --cas %t/cas --cat-blob @%t/file.casid | FileCheck %s --check-prefix=CHECK-TEST-FILE

llvm/tools/llvm-cas/llvm-cas.cpp

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -426,16 +426,31 @@ int traverseGraph(ObjectStore &CAS, const CASID &ID) {
426426
return 0;
427427
}
428428

429-
static Error recursiveAccess(CachingOnDiskFileSystem &FS, StringRef Path) {
429+
static Error
430+
recursiveAccess(CachingOnDiskFileSystem &FS, StringRef Path,
431+
llvm::DenseSet<llvm::sys::fs::UniqueID> &SeenDirectories) {
430432
auto ST = FS.status(Path);
433+
434+
// Ignore missing entries, which can be a symlink to a missing file, which is
435+
// not an error in the filesystem itself.
436+
// FIXME: add status(follow=false) to VFS instead, which would let us detect
437+
// this case directly.
438+
if (ST.getError() == llvm::errc::no_such_file_or_directory)
439+
return Error::success();
440+
431441
if (!ST)
432442
return createFileError(Path, ST.getError());
433443

434-
if (ST->isDirectory()) {
444+
// Check that this is the first time we see the directory to prevent infinite
445+
// recursion into symlinks. The status() above will ensure all symlinks are
446+
// ingested.
447+
// FIXME: add status(follow=false) to VFS instead, and then only traverse
448+
// a directory and not a symlink to a directory.
449+
if (ST->isDirectory() && SeenDirectories.insert(ST->getUniqueID()).second) {
435450
std::error_code EC;
436451
for (llvm::vfs::directory_iterator I = FS.dir_begin(Path, EC), IE;
437452
!EC && I != IE; I.increment(EC)) {
438-
auto Err = recursiveAccess(FS, I->path());
453+
auto Err = recursiveAccess(FS, I->path(), SeenDirectories);
439454
if (Err)
440455
return Err;
441456
}
@@ -460,7 +475,8 @@ static Expected<ObjectProxy> ingestFileSystemImpl(ObjectStore &CAS,
460475

461476
(*FS)->trackNewAccesses();
462477

463-
if (Error E = recursiveAccess(**FS, Path))
478+
llvm::DenseSet<llvm::sys::fs::UniqueID> SeenDirectories;
479+
if (Error E = recursiveAccess(**FS, Path, SeenDirectories))
464480
return std::move(E);
465481

466482
return (*FS)->createTreeFromNewAccesses(

0 commit comments

Comments
 (0)