This revision was automatically updated to reflect the committed changes.
Closed by commit rL285593: Improve ".." handling in FileSpec normalization
(authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D26081?vs=76428&id=76432#toc
Repository:
rL LLVM
https://reviews.llvm.org/D26081
Files:
lldb/trunk/include/lldb/Host/FileSpec.h
lldb/trunk/source/Host/common/FileSpec.cpp
lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/trunk/unittests/Host/FileSpecTest.cpp
Index: lldb/trunk/include/lldb/Host/FileSpec.h
===================================================================
--- lldb/trunk/include/lldb/Host/FileSpec.h
+++ lldb/trunk/include/lldb/Host/FileSpec.h
@@ -620,22 +620,7 @@
/// Normalize a pathname by collapsing redundant separators and
/// up-level references.
//------------------------------------------------------------------
- void NormalizePath();
-
- //------------------------------------------------------------------
- /// Run through the input string, replaying the effect of any ".." and produce
- /// the resultant path. The input path is not required to be in the host file
- /// system
- /// format, but it is required to be normalized to that system.
- ///
- /// @param[in] input
- /// The input path to analyze.
- ///
- /// @param[out] result
- /// The backup-resolved path will be written here.
- //------------------------------------------------------------------
- static void RemoveBackupDots(const ConstString &input_const_str,
- ConstString &result_const_str);
+ FileSpec GetNormalizedPath() const;
//------------------------------------------------------------------
/// Change the file specified with a new path.
Index: lldb/trunk/unittests/Host/FileSpecTest.cpp
===================================================================
--- lldb/trunk/unittests/Host/FileSpecTest.cpp
+++ lldb/trunk/unittests/Host/FileSpecTest.cpp
@@ -131,7 +131,6 @@
Compare(forward, backward, !full_match, !remove_backup_dots, match);
}
-#if 0
TEST(FileSpecTest, EqualDotsWindows) {
const bool full_match = true;
const bool remove_backup_dots = true;
@@ -142,6 +141,7 @@
{R"(C:\bar\baz)", R"(C:/foo/../bar/baz)"},
{R"(C:/bar/baz)", R"(C:\foo\..\bar\baz)"},
{R"(C:\bar)", R"(C:\foo\..\bar)"},
+ {R"(C:\foo\bar)", R"(C:\foo\.\bar)"},
};
for(const auto &test: tests) {
@@ -151,20 +151,20 @@
Compare(one, two, full_match, remove_backup_dots, match);
Compare(one, two, full_match, !remove_backup_dots, !match);
Compare(one, two, !full_match, remove_backup_dots, match);
- Compare(one, two, !full_match, !remove_backup_dots, match);
+ Compare(one, two, !full_match, !remove_backup_dots, !match);
}
}
-#endif
TEST(FileSpecTest, EqualDotsPosix) {
const bool full_match = true;
const bool remove_backup_dots = true;
const bool match = true;
std::pair<const char *, const char *> tests[] = {
{R"(/foo/bar/baz)", R"(/foo/foo/../bar/baz)"},
{R"(/bar/baz)", R"(/foo/../bar/baz)"},
-// {R"(/bar)", R"(/foo/../bar)"},
+ {R"(/bar)", R"(/foo/../bar)"},
+ {R"(/foo/bar)", R"(/foo/./bar)"},
};
for(const auto &test: tests) {
@@ -174,27 +174,84 @@
Compare(one, two, full_match, remove_backup_dots, match);
Compare(one, two, full_match, !remove_backup_dots, !match);
Compare(one, two, !full_match, remove_backup_dots, match);
-// Compare(one, two, !full_match, !remove_backup_dots, match);
+ Compare(one, two, !full_match, !remove_backup_dots, !match);
}
}
-#if 0
TEST(FileSpecTest, EqualDotsPosixRoot) {
const bool full_match = true;
const bool remove_backup_dots = true;
const bool match = true;
std::pair<const char *, const char *> tests[] = {
- {R"(/)", R"(/..)"},
- {R"(/)", R"(/foo/..)"},
+ {R"(/)", R"(/..)"}, {R"(/)", R"(/.)"}, {R"(/)", R"(/foo/..)"},
};
for(const auto &test: tests) {
FileSpec one(test.first, false, FileSpec::ePathSyntaxPosix);
FileSpec two(test.second, false, FileSpec::ePathSyntaxPosix);
EXPECT_NE(one, two);
Compare(one, two, full_match, remove_backup_dots, match);
- Compare(one, two, !full_match, remove_backup_dots, match);
+ Compare(one, two, full_match, !remove_backup_dots, !match);
+ Compare(one, two, !full_match, remove_backup_dots, !match);
+ Compare(one, two, !full_match, !remove_backup_dots, !match);
+ }
+}
+
+TEST(FileSpecTest, GetNormalizedPath) {
+ std::pair<const char *, const char *> posix_tests[] = {
+ {"/foo/../bar", "/bar"},
+ {"/foo/./bar", "/foo/bar"},
+ {"/foo/..", "/"},
+ {"/foo/.", "/foo"},
+ {"/./foo", "/foo"},
+ {"/", "/"},
+ {"//", "//"},
+ {"//net", "//net"},
+ {"/..", "/"},
+ {"/.", "/"},
+ {"..", ".."},
+ {".", "."},
+ {"../..", "../.."},
+ {"foo/..", "."},
+ {"foo/../bar", "bar"},
+ {"../foo/..", ".."},
+ {"./foo", "foo"},
+ };
+ for (auto test : posix_tests) {
+ EXPECT_EQ(test.second,
+ FileSpec(test.first, false, FileSpec::ePathSyntaxPosix)
+ .GetNormalizedPath()
+ .GetPath());
+ }
+
+ std::pair<const char *, const char *> windows_tests[] = {
+ {R"(c:\bar\..\bar)", R"(c:\bar)"},
+ {R"(c:\bar\.\bar)", R"(c:\bar\bar)"},
+ {R"(c:\bar\..)", R"(c:\)"},
+ {R"(c:\bar\.)", R"(c:\bar)"},
+ {R"(c:\.\bar)", R"(c:\bar)"},
+ {R"(\)", R"(\)"},
+ // {R"(\\)", R"(\\)"},
+ // {R"(\\net)", R"(\\net)"},
+ {R"(c:\..)", R"(c:\)"},
+ {R"(c:\.)", R"(c:\)"},
+ {R"(\..)", R"(\)"},
+ // {R"(c:..)", R"(c:..)"},
+ {R"(..)", R"(..)"},
+ {R"(.)", R"(.)"},
+ {R"(c:..\..)", R"(c:..\..)"},
+ {R"(..\..)", R"(..\..)"},
+ {R"(foo\..)", R"(.)"},
+ {R"(foo\..\bar)", R"(bar)"},
+ {R"(..\foo\..)", R"(..)"},
+ {R"(.\foo)", R"(foo)"},
+ };
+ for (auto test : windows_tests) {
+ EXPECT_EQ(test.second,
+ FileSpec(test.first, false, FileSpec::ePathSyntaxWindows)
+ .GetNormalizedPath()
+ .GetPath())
+ << "Original path: " << test.first;
}
}
-#endif
Index: lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===================================================================
--- lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -5095,7 +5095,7 @@
FileSpec file_spec(path, true);
// Remove any redundant parts of the path (like "../foo") since
// LC_RPATH values often contain "..".
- file_spec.NormalizePath();
+ file_spec = file_spec.GetNormalizedPath();
if (file_spec.Exists() && files.AppendIfUnique(file_spec)) {
count++;
break;
Index: lldb/trunk/source/Host/common/FileSpec.cpp
===================================================================
--- lldb/trunk/source/Host/common/FileSpec.cpp
+++ lldb/trunk/source/Host/common/FileSpec.cpp
@@ -533,120 +533,73 @@
if (!full && (a.GetDirectory().IsEmpty() || b.GetDirectory().IsEmpty()))
return ConstString::Equals(a.m_filename, b.m_filename, case_sensitive);
- else if (remove_backups == false)
+
+ if (remove_backups == false)
return a == b;
- else {
- if (!ConstString::Equals(a.m_filename, b.m_filename, case_sensitive))
- return false;
- if (ConstString::Equals(a.m_directory, b.m_directory, case_sensitive))
- return true;
- ConstString a_without_dots;
- ConstString b_without_dots;
- RemoveBackupDots(a.m_directory, a_without_dots);
- RemoveBackupDots(b.m_directory, b_without_dots);
- return ConstString::Equals(a_without_dots, b_without_dots, case_sensitive);
- }
-}
+ if (a == b)
+ return true;
-void FileSpec::NormalizePath() {
- ConstString normalized_directory;
- FileSpec::RemoveBackupDots(m_directory, normalized_directory);
- m_directory = normalized_directory;
+ return Equal(a.GetNormalizedPath(), b.GetNormalizedPath(), full, false);
}
-void FileSpec::RemoveBackupDots(const ConstString &input_const_str,
- ConstString &result_const_str) {
- const char *input = input_const_str.GetCString();
- result_const_str.Clear();
- if (!input || input[0] == '\0')
- return;
-
- const char win_sep = '\\';
- const char unix_sep = '/';
- char found_sep;
- const char *win_backup = "\\..";
- const char *unix_backup = "/..";
-
- bool is_win = false;
-
- // Determine the platform for the path (win or unix):
-
- if (input[0] == win_sep)
- is_win = true;
- else if (input[0] == unix_sep)
- is_win = false;
- else if (input[1] == ':')
- is_win = true;
- else if (strchr(input, unix_sep) != nullptr)
- is_win = false;
- else if (strchr(input, win_sep) != nullptr)
- is_win = true;
- else {
- // No separators at all, no reason to do any work here.
- result_const_str = input_const_str;
- return;
- }
-
- llvm::StringRef backup_sep;
- if (is_win) {
- found_sep = win_sep;
- backup_sep = win_backup;
+FileSpec FileSpec::GetNormalizedPath() const {
+ // Fast path. Do nothing if the path is not interesting.
+ if (!m_directory.GetStringRef().contains(".") &&
+ (m_filename.GetStringRef() != ".." && m_filename.GetStringRef() != "."))
+ return *this;
+
+ llvm::SmallString<64> path, result;
+ const bool normalize = false;
+ GetPath(path, normalize);
+ llvm::StringRef rest(path);
+
+ // We will not go below root dir.
+ size_t root_dir_start = RootDirStart(path, m_syntax);
+ const bool absolute = root_dir_start != llvm::StringRef::npos;
+ if (absolute) {
+ result += rest.take_front(root_dir_start + 1);
+ rest = rest.drop_front(root_dir_start + 1);
} else {
- found_sep = unix_sep;
- backup_sep = unix_backup;
- }
-
- llvm::StringRef input_ref(input);
- llvm::StringRef curpos(input);
-
- bool had_dots = false;
- std::string result;
-
- while (1) {
- // Start of loop
- llvm::StringRef before_sep;
- std::pair<llvm::StringRef, llvm::StringRef> around_sep =
- curpos.split(backup_sep);
-
- before_sep = around_sep.first;
- curpos = around_sep.second;
-
- if (curpos.empty()) {
- if (had_dots) {
- while (before_sep.startswith("//"))
- before_sep = before_sep.substr(1);
- if (!before_sep.empty()) {
- result.append(before_sep.data(), before_sep.size());
- }
- }
- break;
+ if (m_syntax == ePathSyntaxWindows && path.size() > 2 && path[1] == ':') {
+ result += rest.take_front(2);
+ rest = rest.drop_front(2);
}
- had_dots = true;
+ }
- unsigned num_backups = 1;
- while (curpos.startswith(backup_sep)) {
- num_backups++;
- curpos = curpos.slice(backup_sep.size(), curpos.size());
+ bool anything_added = false;
+ llvm::SmallVector<llvm::StringRef, 0> components, processed;
+ rest.split(components, '/', -1, false);
+ processed.reserve(components.size());
+ for (auto component : components) {
+ if (component == ".")
+ continue; // Skip these.
+ if (component != "..") {
+ processed.push_back(component);
+ continue; // Regular file name.
}
-
- size_t end_pos = before_sep.size();
- while (num_backups-- > 0) {
- end_pos = before_sep.rfind(found_sep, end_pos);
- if (end_pos == llvm::StringRef::npos) {
- result_const_str = input_const_str;
- return;
- }
+ if (!processed.empty()) {
+ processed.pop_back();
+ continue; // Dots. Go one level up if we can.
}
- result.append(before_sep.data(), end_pos);
- }
+ if (absolute)
+ continue; // We're at the top level. Cannot go higher than that. Skip.
- if (had_dots)
- result_const_str.SetCString(result.c_str());
- else
- result_const_str = input_const_str;
+ result += component; // We're a relative path. We need to keep these.
+ result += '/';
+ anything_added = true;
+ }
+ for (auto component : processed) {
+ result += component;
+ result += '/';
+ anything_added = true;
+ }
+ if (anything_added)
+ result.pop_back(); // Pop last '/'.
+ else if (result.empty())
+ result = ".";
- return;
+ return FileSpec(result, false, m_syntax);
}
//------------------------------------------------------------------
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
