[llvm] r329338 - [llvm-cov] Prevent llvm-cov from hanging when a symblink doesn't exist.
Max Moroz via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 5 12:43:24 PDT 2018
Author: dor1s
Date: Thu Apr 5 12:43:24 2018
New Revision: 329338
URL: http://llvm.org/viewvc/llvm-project?rev=329338&view=rev
Log:
[llvm-cov] Prevent llvm-cov from hanging when a symblink doesn't exist.
Summary:
Previous code hangs indefinitely when trying to iterate through a
symbol link file that points to an non-exist directory. This change
fixes the bug to make the addCollectedPath function exit ealier and
print out correct warning messages.
Patch by Yuke Liao (@liaoyuke).
Reviewers: Dor1s, vsk
Reviewed By: vsk
Subscribers: bruno, mgrang, llvm-commits
Differential Revision: https://reviews.llvm.org/D44960
Modified:
llvm/trunk/include/llvm/Support/FileSystem.h
llvm/trunk/tools/llvm-cov/CodeCoverage.cpp
llvm/trunk/unittests/Support/Path.cpp
Modified: llvm/trunk/include/llvm/Support/FileSystem.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FileSystem.h?rev=329338&r1=329337&r2=329338&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/FileSystem.h (original)
+++ llvm/trunk/include/llvm/Support/FileSystem.h Thu Apr 5 12:43:24 2018
@@ -956,14 +956,16 @@ public:
SmallString<128> path_storage;
ec = detail::directory_iterator_construct(
*State, path.toStringRef(path_storage), FollowSymlinks);
+ update_error_code_for_current_entry(ec);
}
explicit directory_iterator(const directory_entry &de, std::error_code &ec,
bool follow_symlinks = true)
: FollowSymlinks(follow_symlinks) {
State = std::make_shared<detail::DirIterState>();
- ec =
- detail::directory_iterator_construct(*State, de.path(), FollowSymlinks);
+ ec = detail::directory_iterator_construct(
+ *State, de.path(), FollowSymlinks);
+ update_error_code_for_current_entry(ec);
}
/// Construct end iterator.
@@ -972,6 +974,7 @@ public:
// No operator++ because we need error_code.
directory_iterator &increment(std::error_code &ec) {
ec = directory_iterator_increment(*State);
+ update_error_code_for_current_entry(ec);
return *this;
}
@@ -993,6 +996,24 @@ public:
}
// Other members as required by
// C++ Std, 24.1.1 Input iterators [input.iterators]
+
+private:
+ // Checks if current entry is valid and populates error code. For example,
+ // current entry may not exist due to broken symbol links.
+ void update_error_code_for_current_entry(std::error_code &ec) {
+ // Bail out if error has already occured earlier to avoid overwriting it.
+ if (ec)
+ return;
+
+ // Empty directory entry is used to mark the end of an interation, it's not
+ // an error.
+ if (State->CurrentEntry == directory_entry())
+ return;
+
+ ErrorOr<basic_file_status> status = State->CurrentEntry.status();
+ if (!status)
+ ec = status.getError();
+ }
};
namespace detail {
@@ -1030,11 +1051,9 @@ public:
if (State->HasNoPushRequest)
State->HasNoPushRequest = false;
else {
- ErrorOr<basic_file_status> st = State->Stack.top()->status();
- if (!st) return *this;
- if (is_directory(*st)) {
+ ErrorOr<basic_file_status> status = State->Stack.top()->status();
+ if (status && is_directory(*status)) {
State->Stack.push(directory_iterator(*State->Stack.top(), ec, Follow));
- if (ec) return *this;
if (State->Stack.top() != end_itr) {
++State->Level;
return *this;
Modified: llvm/trunk/tools/llvm-cov/CodeCoverage.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-cov/CodeCoverage.cpp?rev=329338&r1=329337&r2=329338&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-cov/CodeCoverage.cpp (original)
+++ llvm/trunk/tools/llvm-cov/CodeCoverage.cpp Thu Apr 5 12:43:24 2018
@@ -199,7 +199,7 @@ void CodeCoverageTool::collectPaths(cons
if (PathRemapping)
addCollectedPath(Path);
else
- error("Missing source file", Path);
+ warning("Source file doesn't exist, proceeded by ignoring it.", Path);
return;
}
@@ -211,12 +211,16 @@ void CodeCoverageTool::collectPaths(cons
if (llvm::sys::fs::is_directory(Status)) {
std::error_code EC;
for (llvm::sys::fs::recursive_directory_iterator F(Path, EC), E;
- F != E && !EC; F.increment(EC)) {
+ F != E; F.increment(EC)) {
+
+ if (EC) {
+ warning(EC.message(), F->path());
+ continue;
+ }
+
if (llvm::sys::fs::is_regular_file(F->path()))
addCollectedPath(F->path());
}
- if (EC)
- warning(EC.message(), Path);
}
}
Modified: llvm/trunk/unittests/Support/Path.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/Path.cpp?rev=329338&r1=329337&r2=329338&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/Path.cpp (original)
+++ llvm/trunk/unittests/Support/Path.cpp Thu Apr 5 12:43:24 2018
@@ -885,58 +885,91 @@ TEST_F(FileSystemTest, BrokenSymlinkDire
fs::create_link("no_such_file", Twine(TestDirectory) + "/symlink/e"));
typedef std::vector<std::string> v_t;
- v_t visited;
-
- // The directory iterator doesn't stat the file, so we should be able to
- // iterate over the whole directory.
+ v_t VisitedNonBrokenSymlinks;
+ v_t VisitedBrokenSymlinks;
std::error_code ec;
+
+ // Broken symbol links are expected to throw an error.
for (fs::directory_iterator i(Twine(TestDirectory) + "/symlink", ec), e;
i != e; i.increment(ec)) {
+ if (ec == std::make_error_code(std::errc::no_such_file_or_directory)) {
+ VisitedBrokenSymlinks.push_back(path::filename(i->path()));
+ continue;
+ }
+
ASSERT_NO_ERROR(ec);
- visited.push_back(path::filename(i->path()));
+ VisitedNonBrokenSymlinks.push_back(path::filename(i->path()));
}
- std::sort(visited.begin(), visited.end());
- v_t expected = {"a", "b", "c", "d", "e"};
- ASSERT_TRUE(visited.size() == expected.size());
- ASSERT_TRUE(std::equal(visited.begin(), visited.end(), expected.begin()));
- visited.clear();
-
- // The recursive directory iterator has to stat the file, so we need to skip
- // the broken symlinks.
- for (fs::recursive_directory_iterator
- i(Twine(TestDirectory) + "/symlink", ec),
- e;
- i != e; i.increment(ec)) {
- ASSERT_NO_ERROR(ec);
-
- ErrorOr<fs::basic_file_status> status = i->status();
- if (status.getError() ==
- std::make_error_code(std::errc::no_such_file_or_directory)) {
- i.no_push();
+ std::sort(VisitedNonBrokenSymlinks.begin(), VisitedNonBrokenSymlinks.end());
+ std::sort(VisitedBrokenSymlinks.begin(), VisitedBrokenSymlinks.end());
+ v_t ExpectedNonBrokenSymlinks = {"b", "d"};
+ ASSERT_EQ(ExpectedNonBrokenSymlinks.size(), VisitedNonBrokenSymlinks.size());
+ ASSERT_TRUE(std::equal(VisitedNonBrokenSymlinks.begin(),
+ VisitedNonBrokenSymlinks.end(),
+ ExpectedNonBrokenSymlinks.begin()));
+ VisitedNonBrokenSymlinks.clear();
+
+ v_t ExpectedBrokenSymlinks = {"a", "c", "e"};
+ ASSERT_EQ(ExpectedBrokenSymlinks.size(), VisitedBrokenSymlinks.size());
+ ASSERT_TRUE(std::equal(VisitedBrokenSymlinks.begin(),
+ VisitedBrokenSymlinks.end(),
+ ExpectedBrokenSymlinks.begin()));
+ VisitedBrokenSymlinks.clear();
+
+ // Broken symbol links are expected to throw an error.
+ for (fs::recursive_directory_iterator i(
+ Twine(TestDirectory) + "/symlink", ec), e; i != e; i.increment(ec)) {
+ if (ec == std::make_error_code(std::errc::no_such_file_or_directory)) {
+ VisitedBrokenSymlinks.push_back(path::filename(i->path()));
continue;
}
- visited.push_back(path::filename(i->path()));
+ ASSERT_NO_ERROR(ec);
+ VisitedNonBrokenSymlinks.push_back(path::filename(i->path()));
}
- std::sort(visited.begin(), visited.end());
- expected = {"b", "bb", "d", "da", "dd", "ddd", "ddd"};
- ASSERT_TRUE(visited.size() == expected.size());
- ASSERT_TRUE(std::equal(visited.begin(), visited.end(), expected.begin()));
- visited.clear();
-
- // This recursive directory iterator doesn't follow symlinks, so we don't need
- // to skip them.
- for (fs::recursive_directory_iterator
- i(Twine(TestDirectory) + "/symlink", ec, /*follow_symlinks=*/false),
- e;
+ std::sort(VisitedNonBrokenSymlinks.begin(), VisitedNonBrokenSymlinks.end());
+ std::sort(VisitedBrokenSymlinks.begin(), VisitedBrokenSymlinks.end());
+ ExpectedNonBrokenSymlinks = {"b", "bb", "d", "da", "dd", "ddd", "ddd"};
+ ASSERT_EQ(ExpectedNonBrokenSymlinks.size(), VisitedNonBrokenSymlinks.size());
+ ASSERT_TRUE(std::equal(VisitedNonBrokenSymlinks.begin(),
+ VisitedNonBrokenSymlinks.end(),
+ ExpectedNonBrokenSymlinks.begin()));
+ VisitedNonBrokenSymlinks.clear();
+
+ ExpectedBrokenSymlinks = {"a", "ba", "bc", "c", "e"};
+ ASSERT_EQ(ExpectedBrokenSymlinks.size(), VisitedBrokenSymlinks.size());
+ ASSERT_TRUE(std::equal(VisitedBrokenSymlinks.begin(),
+ VisitedBrokenSymlinks.end(),
+ ExpectedBrokenSymlinks.begin()));
+ VisitedBrokenSymlinks.clear();
+
+ for (fs::recursive_directory_iterator i(
+ Twine(TestDirectory) + "/symlink", ec, /*follow_symlinks=*/false), e;
i != e; i.increment(ec)) {
+ if (ec == std::make_error_code(std::errc::no_such_file_or_directory)) {
+ VisitedBrokenSymlinks.push_back(path::filename(i->path()));
+ continue;
+ }
+
ASSERT_NO_ERROR(ec);
- visited.push_back(path::filename(i->path()));
+ VisitedNonBrokenSymlinks.push_back(path::filename(i->path()));
}
- std::sort(visited.begin(), visited.end());
- expected = {"a", "b", "ba", "bb", "bc", "c", "d", "da", "dd", "ddd", "e"};
- ASSERT_TRUE(visited.size() == expected.size());
- ASSERT_TRUE(std::equal(visited.begin(), visited.end(), expected.begin()));
+ std::sort(VisitedNonBrokenSymlinks.begin(), VisitedNonBrokenSymlinks.end());
+ std::sort(VisitedBrokenSymlinks.begin(), VisitedBrokenSymlinks.end());
+ ExpectedNonBrokenSymlinks = {"a", "b", "ba", "bb", "bc", "c", "d", "da", "dd",
+ "ddd", "e"};
+ ASSERT_EQ(ExpectedNonBrokenSymlinks.size(), VisitedNonBrokenSymlinks.size());
+ ASSERT_TRUE(std::equal(VisitedNonBrokenSymlinks.begin(),
+ VisitedNonBrokenSymlinks.end(),
+ ExpectedNonBrokenSymlinks.begin()));
+ VisitedNonBrokenSymlinks.clear();
+
+ ExpectedBrokenSymlinks = {};
+ ASSERT_EQ(ExpectedBrokenSymlinks.size(), VisitedBrokenSymlinks.size());
+ ASSERT_TRUE(std::equal(VisitedBrokenSymlinks.begin(),
+ VisitedBrokenSymlinks.end(),
+ ExpectedBrokenSymlinks.begin()));
+ VisitedBrokenSymlinks.clear();
ASSERT_NO_ERROR(fs::remove_directories(Twine(TestDirectory) + "/symlink"));
}
More information about the llvm-commits
mailing list