[Lldb-commits] [lldb] r331049 - Always normalize FileSpec paths.
Greg Clayton via lldb-commits
lldb-commits at lists.llvm.org
Fri Apr 27 14:14:06 PDT 2018
Buildbots should be fixed with 331082.
> On Apr 27, 2018, at 1:01 PM, Frédéric Riss <friss at apple.com> wrote:
>
>
>
>> On Apr 27, 2018, at 11:17 AM, Jim Ingham via lldb-commits <lldb-commits at lists.llvm.org <mailto:lldb-commits at lists.llvm.org>> wrote:
>>
>> Greg,
>>
>> Your new FileSpecTest unit tests are failing in the Xcode build of lldb, e.g.:
>>
>> http://lab.llvm.org:8080/green/view/LLDB/job/lldb-xcode/6271/consoleFull#-1083450927b825e790-484f-4586-af29-73c4754ff671 <http://lab.llvm.org:8080/green/view/LLDB/job/lldb-xcode/6271/consoleFull#-1083450927b825e790-484f-4586-af29-73c4754ff671>
>>
>> Can you figure out what's up with this?
>
> Once you get past the unit test, it looks like it also broke TestBreakpointCommand.py. Please fix this quickly.
>
> Fred
>
>> Jim
>>
>> BTW, the "reply to" for lldb-commits mails for you is still your apple.com address. I don't know where that gets set but you should probably update that at some point.
>>
>>
>>
>>
>>> On Apr 27, 2018, at 8:45 AM, Greg Clayton via lldb-commits <lldb-commits at lists.llvm.org> wrote:
>>>
>>> Author: gclayton
>>> Date: Fri Apr 27 08:45:58 2018
>>> New Revision: 331049
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=331049&view=rev
>>> Log:
>>> Always normalize FileSpec paths.
>>>
>>> Always normalizing lldb_private::FileSpec paths will help us get a consistent results from comparisons when setting breakpoints and when looking for source files. This also removes a lot of complexity from the comparison routines. Modified the DWARF line table parser to use the normalized compile unit directory if needed.
>>>
>>> Differential Revision: https://reviews.llvm.org/D45977
>>>
>>>
>>> Modified:
>>> lldb/trunk/include/lldb/Core/FileSpecList.h
>>> lldb/trunk/include/lldb/Utility/FileSpec.h
>>> lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp
>>> lldb/trunk/source/Core/FileSpecList.cpp
>>> lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
>>> lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp
>>> lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h
>>> lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
>>> lldb/trunk/source/Symbol/CompileUnit.cpp
>>> lldb/trunk/source/Symbol/Declaration.cpp
>>> lldb/trunk/source/Utility/FileSpec.cpp
>>> lldb/trunk/unittests/Utility/FileSpecTest.cpp
>>>
>>> Modified: lldb/trunk/include/lldb/Core/FileSpecList.h
>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/FileSpecList.h?rev=331049&r1=331048&r2=331049&view=diff
>>> ==============================================================================
>>> --- lldb/trunk/include/lldb/Core/FileSpecList.h (original)
>>> +++ lldb/trunk/include/lldb/Core/FileSpecList.h Fri Apr 27 08:45:58 2018
>>> @@ -119,16 +119,11 @@ public:
>>> /// @param[in] full
>>> /// Should FileSpec::Equal be called with "full" true or false.
>>> ///
>>> - /// @param[in] remove_backup_dots
>>> - /// Should FileSpec::Equal be called with "remove_backup_dots" true or
>>> - /// false.
>>> - ///
>>> /// @return
>>> /// The index of the file that matches \a file if it is found,
>>> /// else UINT32_MAX is returned.
>>> //------------------------------------------------------------------
>>> - size_t FindFileIndex(size_t idx, const FileSpec &file, bool full,
>>> - bool remove_backup_dots = false) const;
>>> + size_t FindFileIndex(size_t idx, const FileSpec &file, bool full) const;
>>>
>>> //------------------------------------------------------------------
>>> /// Get file at index.
>>>
>>> Modified: lldb/trunk/include/lldb/Utility/FileSpec.h
>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/FileSpec.h?rev=331049&r1=331048&r2=331049&view=diff
>>> ==============================================================================
>>> --- lldb/trunk/include/lldb/Utility/FileSpec.h (original)
>>> +++ lldb/trunk/include/lldb/Utility/FileSpec.h Fri Apr 27 08:45:58 2018
>>> @@ -256,8 +256,7 @@ public:
>>> //------------------------------------------------------------------
>>> static int Compare(const FileSpec &lhs, const FileSpec &rhs, bool full);
>>>
>>> - static bool Equal(const FileSpec &a, const FileSpec &b, bool full,
>>> - bool remove_backups = false);
>>> + static bool Equal(const FileSpec &a, const FileSpec &b, bool full);
>>>
>>> //------------------------------------------------------------------
>>> /// Case sensitivity of path.
>>> @@ -488,12 +487,6 @@ public:
>>> size_t MemorySize() const;
>>>
>>> //------------------------------------------------------------------
>>> - /// Normalize a pathname by collapsing redundant separators and
>>> - /// up-level references.
>>> - //------------------------------------------------------------------
>>> - FileSpec GetNormalizedPath() const;
>>> -
>>> - //------------------------------------------------------------------
>>> /// Change the file specified with a new path.
>>> ///
>>> /// Update the contents of this object with a new path. The path will
>>>
>>> Modified: lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp?rev=331049&r1=331048&r2=331049&view=diff
>>> ==============================================================================
>>> --- lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp (original)
>>> +++ lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp Fri Apr 27 08:45:58 2018
>>> @@ -122,7 +122,7 @@ void BreakpointResolverFileLine::FilterC
>>>
>>> llvm::StringRef relative_path;
>>> if (is_relative)
>>> - relative_path = m_file_spec.GetNormalizedPath().GetDirectory().GetStringRef();
>>> + relative_path = m_file_spec.GetDirectory().GetStringRef();
>>>
>>> Log * log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS);
>>> for(uint32_t i = 0; i < sc_list.GetSize(); ++i) {
>>>
>>> Modified: lldb/trunk/source/Core/FileSpecList.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/FileSpecList.cpp?rev=331049&r1=331048&r2=331049&view=diff
>>> ==============================================================================
>>> --- lldb/trunk/source/Core/FileSpecList.cpp (original)
>>> +++ lldb/trunk/source/Core/FileSpecList.cpp Fri Apr 27 08:45:58 2018
>>> @@ -82,7 +82,7 @@ void FileSpecList::Dump(Stream *s, const
>>> // it is found, else std::numeric_limits<uint32_t>::max() is returned.
>>> //------------------------------------------------------------------
>>> size_t FileSpecList::FindFileIndex(size_t start_idx, const FileSpec &file_spec,
>>> - bool full, bool remove_dots) const {
>>> + bool full) const {
>>> const size_t num_files = m_files.size();
>>>
>>> // When looking for files, we will compare only the filename if the
>>> @@ -96,7 +96,7 @@ size_t FileSpecList::FindFileIndex(size_
>>> file_spec.IsCaseSensitive() || m_files[idx].IsCaseSensitive()))
>>> return idx;
>>> } else {
>>> - if (FileSpec::Equal(m_files[idx], file_spec, full, remove_dots))
>>> + if (FileSpec::Equal(m_files[idx], file_spec, full))
>>> return idx;
>>> }
>>> }
>>>
>>> Modified: lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp?rev=331049&r1=331048&r2=331049&view=diff
>>> ==============================================================================
>>> --- lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (original)
>>> +++ lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp Fri Apr 27 08:45:58 2018
>>> @@ -5098,9 +5098,6 @@ uint32_t ObjectFileMachO::GetDependentMo
>>> // It is OK to resolve this path because we must find a file on
>>> // disk for us to accept it anyway if it is rpath relative.
>>> FileSpec file_spec(path, true);
>>> - // Remove any redundant parts of the path (like "../foo") since
>>> - // LC_RPATH values often contain "..".
>>> - file_spec = file_spec.GetNormalizedPath();
>>> if (file_spec.Exists() && files.AppendIfUnique(file_spec)) {
>>> count++;
>>> break;
>>> @@ -5118,7 +5115,6 @@ uint32_t ObjectFileMachO::GetDependentMo
>>> for (const auto &at_exec_relative_path : at_exec_relative_paths) {
>>> FileSpec file_spec =
>>> exec_dir.CopyByAppendingPathComponent(at_exec_relative_path);
>>> - file_spec = file_spec.GetNormalizedPath();
>>> if (file_spec.Exists() && files.AppendIfUnique(file_spec))
>>> count++;
>>> }
>>>
>>> Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp?rev=331049&r1=331048&r2=331049&view=diff
>>> ==============================================================================
>>> --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp (original)
>>> +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp Fri Apr 27 08:45:58 2018
>>> @@ -444,7 +444,7 @@ bool DWARFDebugLine::ParsePrologue(const
>>>
>>> bool DWARFDebugLine::ParseSupportFiles(
>>> const lldb::ModuleSP &module_sp, const DWARFDataExtractor &debug_line_data,
>>> - const char *cu_comp_dir, dw_offset_t stmt_list,
>>> + const lldb_private::FileSpec &cu_comp_dir, dw_offset_t stmt_list,
>>> FileSpecList &support_files) {
>>> lldb::offset_t offset = stmt_list;
>>>
>>> @@ -861,8 +861,8 @@ void DWARFDebugLine::Prologue::Dump(Log
>>> // buff.Append8(0); // Terminate the file names section with empty string
>>> //}
>>>
>>> -bool DWARFDebugLine::Prologue::GetFile(uint32_t file_idx, const char *comp_dir,
>>> - FileSpec &file) const {
>>> +bool DWARFDebugLine::Prologue::GetFile(uint32_t file_idx,
>>> + const lldb_private::FileSpec &comp_dir, FileSpec &file) const {
>>> uint32_t idx = file_idx - 1; // File indexes are 1 based...
>>> if (idx < file_names.size()) {
>>> file.SetFile(file_names[idx].name, false);
>>> @@ -876,7 +876,7 @@ bool DWARFDebugLine::Prologue::GetFile(u
>>> }
>>> }
>>>
>>> - if (comp_dir && comp_dir[0])
>>> + if (comp_dir)
>>> file.PrependPathComponent(comp_dir);
>>> }
>>> return true;
>>>
>>> Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h
>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h?rev=331049&r1=331048&r2=331049&view=diff
>>> ==============================================================================
>>> --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h (original)
>>> +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h Fri Apr 27 08:45:58 2018
>>> @@ -90,7 +90,7 @@ public:
>>> include_directories.clear();
>>> file_names.clear();
>>> }
>>> - bool GetFile(uint32_t file_idx, const char *comp_dir,
>>> + bool GetFile(uint32_t file_idx, const lldb_private::FileSpec &cu_comp_dir,
>>> lldb_private::FileSpec &file) const;
>>> };
>>>
>>> @@ -199,7 +199,8 @@ public:
>>> static bool
>>> ParseSupportFiles(const lldb::ModuleSP &module_sp,
>>> const lldb_private::DWARFDataExtractor &debug_line_data,
>>> - const char *cu_comp_dir, dw_offset_t stmt_list,
>>> + const lldb_private::FileSpec &cu_comp_dir,
>>> + dw_offset_t stmt_list,
>>> lldb_private::FileSpecList &support_files);
>>> static bool
>>> ParsePrologue(const lldb_private::DWARFDataExtractor &debug_line_data,
>>>
>>> Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=331049&r1=331048&r2=331049&view=diff
>>> ==============================================================================
>>> --- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original)
>>> +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Fri Apr 27 08:45:58 2018
>>> @@ -174,38 +174,39 @@ static const char *removeHostnameFromPat
>>> return colon_pos + 1;
>>> }
>>>
>>> -static const char *resolveCompDir(const char *path_from_dwarf) {
>>> +static FileSpec resolveCompDir(const char *path_from_dwarf) {
>>> if (!path_from_dwarf)
>>> - return nullptr;
>>> + return FileSpec();
>>>
>>> // DWARF2/3 suggests the form hostname:pathname for compilation directory.
>>> // Remove the host part if present.
>>> const char *local_path = removeHostnameFromPathname(path_from_dwarf);
>>> if (!local_path)
>>> - return nullptr;
>>> + return FileSpec();
>>>
>>> bool is_symlink = false;
>>> - FileSpec local_path_spec(local_path, false);
>>> + // Always normalize our compile unit directory to get rid of redundant
>>> + // slashes and other path anomalies before we use it for path prepending
>>> + FileSpec local_spec(local_path, false);
>>> const auto &file_specs = GetGlobalPluginProperties()->GetSymLinkPaths();
>>> for (size_t i = 0; i < file_specs.GetSize() && !is_symlink; ++i)
>>> is_symlink = FileSpec::Equal(file_specs.GetFileSpecAtIndex(i),
>>> - local_path_spec, true);
>>> + local_spec, true);
>>>
>>> if (!is_symlink)
>>> - return local_path;
>>> + return local_spec;
>>>
>>> namespace fs = llvm::sys::fs;
>>> - if (fs::get_file_type(local_path_spec.GetPath(), false) !=
>>> + if (fs::get_file_type(local_spec.GetPath(), false) !=
>>> fs::file_type::symlink_file)
>>> - return local_path;
>>> + return local_spec;
>>>
>>> - FileSpec resolved_local_path_spec;
>>> - const auto error =
>>> - FileSystem::Readlink(local_path_spec, resolved_local_path_spec);
>>> + FileSpec resolved_symlink;
>>> + const auto error = FileSystem::Readlink(local_spec, resolved_symlink);
>>> if (error.Success())
>>> - return resolved_local_path_spec.GetCString();
>>> + return resolved_symlink;
>>>
>>> - return nullptr;
>>> + return local_spec;
>>> }
>>>
>>> DWARFUnit *SymbolFileDWARF::GetBaseCompileUnit() {
>>> @@ -912,7 +913,7 @@ bool SymbolFileDWARF::ParseCompileUnitSu
>>> const DWARFDIE cu_die = dwarf_cu->GetUnitDIEOnly();
>>>
>>> if (cu_die) {
>>> - const char *cu_comp_dir = resolveCompDir(
>>> + FileSpec cu_comp_dir = resolveCompDir(
>>> cu_die.GetAttributeValueAsString(DW_AT_comp_dir, nullptr));
>>> const dw_offset_t stmt_list = cu_die.GetAttributeValueAsUnsigned(
>>> DW_AT_stmt_list, DW_INVALID_OFFSET);
>>>
>>> Modified: lldb/trunk/source/Symbol/CompileUnit.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/CompileUnit.cpp?rev=331049&r1=331048&r2=331049&view=diff
>>> ==============================================================================
>>> --- lldb/trunk/source/Symbol/CompileUnit.cpp (original)
>>> +++ lldb/trunk/source/Symbol/CompileUnit.cpp Fri Apr 27 08:45:58 2018
>>> @@ -287,9 +287,8 @@ uint32_t CompileUnit::ResolveSymbolConte
>>> // when finding file indexes
>>> std::vector<uint32_t> file_indexes;
>>> const bool full_match = (bool)file_spec.GetDirectory();
>>> - const bool remove_backup_dots = true;
>>> bool file_spec_matches_cu_file_spec =
>>> - FileSpec::Equal(file_spec, *this, full_match, remove_backup_dots);
>>> + FileSpec::Equal(file_spec, *this, full_match);
>>>
>>> // If we are not looking for inlined functions and our file spec doesn't
>>> // match then we are done...
>>> @@ -297,11 +296,10 @@ uint32_t CompileUnit::ResolveSymbolConte
>>> return 0;
>>>
>>> uint32_t file_idx =
>>> - GetSupportFiles().FindFileIndex(1, file_spec, true, remove_backup_dots);
>>> + GetSupportFiles().FindFileIndex(1, file_spec, true);
>>> while (file_idx != UINT32_MAX) {
>>> file_indexes.push_back(file_idx);
>>> - file_idx = GetSupportFiles().FindFileIndex(file_idx + 1, file_spec, true,
>>> - remove_backup_dots);
>>> + file_idx = GetSupportFiles().FindFileIndex(file_idx + 1, file_spec, true);
>>> }
>>>
>>> const size_t num_file_indexes = file_indexes.size();
>>>
>>> Modified: lldb/trunk/source/Symbol/Declaration.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/Declaration.cpp?rev=331049&r1=331048&r2=331049&view=diff
>>> ==============================================================================
>>> --- lldb/trunk/source/Symbol/Declaration.cpp (original)
>>> +++ lldb/trunk/source/Symbol/Declaration.cpp Fri Apr 27 08:45:58 2018
>>> @@ -91,7 +91,7 @@ bool lldb_private::operator==(const Decl
>>> return lhs.GetFile() == rhs.GetFile();
>>> #else
>>> if (lhs.GetLine() == rhs.GetLine())
>>> - return FileSpec::Equal(lhs.GetFile(), rhs.GetFile(), true, true);
>>> + return FileSpec::Equal(lhs.GetFile(), rhs.GetFile(), true);
>>> #endif
>>> return false;
>>> }
>>>
>>> Modified: lldb/trunk/source/Utility/FileSpec.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/FileSpec.cpp?rev=331049&r1=331048&r2=331049&view=diff
>>> ==============================================================================
>>> --- lldb/trunk/source/Utility/FileSpec.cpp (original)
>>> +++ lldb/trunk/source/Utility/FileSpec.cpp Fri Apr 27 08:45:58 2018
>>> @@ -62,17 +62,18 @@ bool IsPathSeparator(char value, FileSpe
>>> return value == '/' || (!PathSyntaxIsPosix(syntax) && value == '\\');
>>> }
>>>
>>> -void Normalize(llvm::SmallVectorImpl<char> &path, FileSpec::PathSyntax syntax) {
>>> - if (PathSyntaxIsPosix(syntax))
>>> - return;
>>> -
>>> - std::replace(path.begin(), path.end(), '\\', '/');
>>> - // Windows path can have \\ slashes which can be changed by replace
>>> - // call above to //. Here we remove the duplicate.
>>> - auto iter = std::unique(path.begin(), path.end(), [](char &c1, char &c2) {
>>> - return (c1 == '/' && c2 == '/');
>>> - });
>>> - path.erase(iter, path.end());
>>> +inline llvm::sys::path::Style
>>> +LLVMPathSyntax(FileSpec::PathSyntax lldb_syntax) {
>>> + switch (lldb_syntax) {
>>> + case FileSpec::ePathSyntaxPosix:
>>> + return llvm::sys::path::Style::posix;
>>> + case FileSpec::ePathSyntaxWindows:
>>> + return llvm::sys::path::Style::windows;
>>> + default:
>>> + case FileSpec::ePathSyntaxHostNative:
>>> + return llvm::sys::path::Style::native;
>>> + };
>>> + return llvm::sys::path::Style::native;
>>> }
>>>
>>> void Denormalize(llvm::SmallVectorImpl<char> &path,
>>> @@ -199,6 +200,104 @@ FileSpec::FileSpec(const FileSpec *rhs)
>>> //------------------------------------------------------------------
>>> FileSpec::~FileSpec() {}
>>>
>>> +namespace {
>>> +//------------------------------------------------------------------
>>> +/// Safely get a character at the specified index.
>>> +///
>>> +/// @param[in] path
>>> +/// A full, partial, or relative path to a file.
>>> +///
>>> +/// @param[in] i
>>> +/// An index into path which may or may not be valid.
>>> +///
>>> +/// @return
>>> +/// The character at index \a i if the index is valid, or 0 if
>>> +/// the index is not valid.
>>> +//------------------------------------------------------------------
>>> +inline char safeCharAtIndex(const llvm::StringRef &path, size_t i) {
>>> + if (i < path.size())
>>> + return path[i];
>>> + return 0;
>>> +}
>>> +
>>> +//------------------------------------------------------------------
>>> +/// Check if a path needs to be normalized.
>>> +///
>>> +/// Check if a path needs to be normalized. We currently consider a
>>> +/// path to need normalization if any of the following are true
>>> +/// - path contains "/./"
>>> +/// - path contains "/../"
>>> +/// - path contains "//"
>>> +/// - path ends with "/"
>>> +/// Paths that start with "./" or with "../" are not considered to
>>> +/// need normalization since we aren't trying to resolve the path,
>>> +/// we are just trying to remove redundant things from the path.
>>> +///
>>> +/// @param[in] path
>>> +/// A full, partial, or relative path to a file.
>>> +///
>>> +/// @param[in] syntax
>>> +/// The syntax enumeration for the path in \a path.
>>> +///
>>> +/// @return
>>> +/// Returns \b true if the path needs to be normalized.
>>> +//------------------------------------------------------------------
>>> +bool needsNormalization(const llvm::StringRef &path,
>>> + FileSpec::PathSyntax syntax) {
>>> + const auto separator = GetPreferredPathSeparator(syntax);
>>> + for (auto i = path.find(separator); i != llvm::StringRef::npos;
>>> + i = path.find(separator, i + 1)) {
>>> + const auto next = safeCharAtIndex(path, i+1);
>>> + switch (next) {
>>> + case 0:
>>> + // path separator char at the end of the string which should be
>>> + // stripped unless it is the one and only character
>>> + return i > 0;
>>> + case '/':
>>> + case '\\':
>>> + // two path separator chars in the middle of a path needs to be
>>> + // normalized
>>> + if (next == separator && i > 0)
>>> + return true;
>>> + ++i;
>>> + break;
>>> +
>>> + case '.': {
>>> + const auto next_next = safeCharAtIndex(path, i+2);
>>> + switch (next_next) {
>>> + default: break;
>>> + case 0: return true; // ends with "/."
>>> + case '/':
>>> + case '\\':
>>> + if (next_next == separator)
>>> + return true; // contains "/./"
>>> + break;
>>> + case '.': {
>>> + const auto next_next_next = safeCharAtIndex(path, i+3);
>>> + switch (next_next_next) {
>>> + default: break;
>>> + case 0: return true; // ends with "/.."
>>> + case '/':
>>> + case '\\':
>>> + if (next_next_next == separator)
>>> + return true; // contains "/../"
>>> + break;
>>> + }
>>> + break;
>>> + }
>>> + }
>>> + }
>>> + break;
>>> +
>>> + default:
>>> + break;
>>> + }
>>> + }
>>> + return false;
>>> +}
>>> +
>>> +
>>> +}
>>> //------------------------------------------------------------------
>>> // Assignment operator.
>>> //------------------------------------------------------------------
>>> @@ -238,7 +337,14 @@ void FileSpec::SetFile(llvm::StringRef p
>>> m_is_resolved = true;
>>> }
>>>
>>> - Normalize(resolved, m_syntax);
>>> + // Normalize the path by removing ".", ".." and other redundant components.
>>> + if (needsNormalization(llvm::StringRef(resolved.data(), resolved.size()),
>>> + syntax))
>>> + llvm::sys::path::remove_dots(resolved, true, LLVMPathSyntax(syntax));
>>> +
>>> + // Normalize back slashes to forward slashes
>>> + if (syntax == FileSpec::ePathSyntaxWindows)
>>> + std::replace(resolved.begin(), resolved.end(), '\\', '/');
>>>
>>> llvm::StringRef resolve_path_ref(resolved.c_str());
>>> size_t dir_end = ParentPathEnd(resolve_path_ref, m_syntax);
>>> @@ -408,106 +514,22 @@ int FileSpec::Compare(const FileSpec &a,
>>> return ConstString::Compare(a.m_filename, b.m_filename, case_sensitive);
>>> }
>>>
>>> -bool FileSpec::Equal(const FileSpec &a, const FileSpec &b, bool full,
>>> - bool remove_backups) {
>>> - static ConstString g_dot_string(".");
>>> - static ConstString g_dot_dot_string("..");
>>> +bool FileSpec::Equal(const FileSpec &a, const FileSpec &b, bool full) {
>>>
>>> // case sensitivity of equality test
>>> const bool case_sensitive = a.IsCaseSensitive() || b.IsCaseSensitive();
>>>
>>> - bool filenames_equal = ConstString::Equals(a.m_filename,
>>> - b.m_filename,
>>> - case_sensitive);
>>> -
>>> - // The only way two FileSpecs can be equal if their filenames are
>>> - // unequal is if we are removing backups and one or the other filename
>>> - // is a backup string:
>>> + const bool filenames_equal = ConstString::Equals(a.m_filename,
>>> + b.m_filename,
>>> + case_sensitive);
>>>
>>> - if (!filenames_equal && !remove_backups)
>>> + if (!filenames_equal)
>>> return false;
>>>
>>> - bool last_component_is_dot = ConstString::Equals(a.m_filename, g_dot_string)
>>> - || ConstString::Equals(a.m_filename,
>>> - g_dot_dot_string)
>>> - || ConstString::Equals(b.m_filename,
>>> - g_dot_string)
>>> - || ConstString::Equals(b.m_filename,
>>> - g_dot_dot_string);
>>> -
>>> - if (!filenames_equal && !last_component_is_dot)
>>> - return false;
>>> -
>>> if (!full && (a.GetDirectory().IsEmpty() || b.GetDirectory().IsEmpty()))
>>> return filenames_equal;
>>>
>>> - if (remove_backups == false)
>>> - return a == b;
>>> -
>>> - if (a == b)
>>> - return true;
>>> -
>>> - return Equal(a.GetNormalizedPath(), b.GetNormalizedPath(), full, false);
>>> -}
>>> -
>>> -FileSpec FileSpec::GetNormalizedPath() const {
>>> - // Fast path. Do nothing if the path is not interesting.
>>> - if (!m_directory.GetStringRef().contains(".") &&
>>> - !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 {
>>> - if (m_syntax == ePathSyntaxWindows && path.size() > 2 && path[1] == ':') {
>>> - result += rest.take_front(2);
>>> - rest = rest.drop_front(2);
>>> - }
>>> - }
>>> -
>>> - 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.
>>> - }
>>> - if (!processed.empty()) {
>>> - processed.pop_back();
>>> - continue; // Dots. Go one level up if we can.
>>> - }
>>> - if (absolute)
>>> - continue; // We're at the top level. Cannot go higher than that. Skip.
>>> -
>>> - 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 FileSpec(result, false, m_syntax);
>>> + return a == b;
>>> }
>>>
>>> //------------------------------------------------------------------
>>> @@ -647,12 +669,14 @@ void FileSpec::GetPath(llvm::SmallVector
>>> bool denormalize) const {
>>> path.append(m_directory.GetStringRef().begin(),
>>> m_directory.GetStringRef().end());
>>> - if (m_directory && m_filename &&
>>> - !IsPathSeparator(m_directory.GetStringRef().back(), m_syntax))
>>> - path.insert(path.end(), GetPreferredPathSeparator(m_syntax));
>>> + // Since the path was normalized and all paths use '/' when stored in these
>>> + // objects, we don't need to look for the actual syntax specific path
>>> + // separator, we just look for and insert '/'.
>>> + if (m_directory && m_filename && m_directory.GetStringRef().back() != '/' &&
>>> + m_filename.GetStringRef().back() != '/')
>>> + path.insert(path.end(), '/');
>>> path.append(m_filename.GetStringRef().begin(),
>>> m_filename.GetStringRef().end());
>>> - Normalize(path, m_syntax);
>>> if (denormalize && !path.empty())
>>> Denormalize(path, m_syntax);
>>> }
>>>
>>> Modified: lldb/trunk/unittests/Utility/FileSpecTest.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/FileSpecTest.cpp?rev=331049&r1=331048&r2=331049&view=diff
>>> ==============================================================================
>>> --- lldb/trunk/unittests/Utility/FileSpecTest.cpp (original)
>>> +++ lldb/trunk/unittests/Utility/FileSpecTest.cpp Fri Apr 27 08:45:58 2018
>>> @@ -54,17 +54,14 @@ TEST(FileSpecTest, FileAndDirectoryCompo
>>>
>>> FileSpec fs_posix_trailing_slash("/foo/bar/", false,
>>> FileSpec::ePathSyntaxPosix);
>>> - EXPECT_STREQ("/foo/bar/.", fs_posix_trailing_slash.GetCString());
>>> - EXPECT_STREQ("/foo/bar", fs_posix_trailing_slash.GetDirectory().GetCString());
>>> - EXPECT_STREQ(".", fs_posix_trailing_slash.GetFilename().GetCString());
>>> + EXPECT_STREQ("/foo/bar", fs_posix_trailing_slash.GetCString());
>>> + EXPECT_STREQ("/foo", fs_posix_trailing_slash.GetDirectory().GetCString());
>>> + EXPECT_STREQ("bar", fs_posix_trailing_slash.GetFilename().GetCString());
>>>
>>> FileSpec fs_windows_trailing_slash("F:\\bar\\", false,
>>> FileSpec::ePathSyntaxWindows);
>>> - EXPECT_STREQ("F:\\bar\\.", fs_windows_trailing_slash.GetCString());
>>> - // EXPECT_STREQ("F:\\bar",
>>> - // fs_windows_trailing_slash.GetDirectory().GetCString()); // It returns
>>> - // "F:/bar"
>>> - EXPECT_STREQ(".", fs_windows_trailing_slash.GetFilename().GetCString());
>>> + EXPECT_STREQ("F:\\bar", fs_windows_trailing_slash.GetCString());
>>> + EXPECT_STREQ("bar", fs_windows_trailing_slash.GetFilename().GetCString());
>>> }
>>>
>>> TEST(FileSpecTest, AppendPathComponent) {
>>> @@ -131,32 +128,13 @@ TEST(FileSpecTest, PrependPathComponent)
>>> EXPECT_STREQ("F:\\bar", fs_windows_root.GetCString());
>>> }
>>>
>>> -static void Compare(const FileSpec &one, const FileSpec &two, bool full_match,
>>> - bool remove_backup_dots, bool result) {
>>> - EXPECT_EQ(result, FileSpec::Equal(one, two, full_match, remove_backup_dots))
>>> - << "File one: " << one.GetCString() << "\nFile two: " << two.GetCString()
>>> - << "\nFull match: " << full_match
>>> - << "\nRemove backup dots: " << remove_backup_dots;
>>> -}
>>> -
>>> TEST(FileSpecTest, EqualSeparator) {
>>> FileSpec backward("C:\\foo\\bar", false, FileSpec::ePathSyntaxWindows);
>>> FileSpec forward("C:/foo/bar", false, FileSpec::ePathSyntaxWindows);
>>> EXPECT_EQ(forward, backward);
>>> -
>>> - const bool full_match = true;
>>> - const bool remove_backup_dots = true;
>>> - const bool match = true;
>>> - Compare(forward, backward, full_match, remove_backup_dots, match);
>>> - Compare(forward, backward, full_match, !remove_backup_dots, match);
>>> - Compare(forward, backward, !full_match, remove_backup_dots, match);
>>> - Compare(forward, backward, !full_match, !remove_backup_dots, match);
>>> }
>>>
>>> TEST(FileSpecTest, EqualDotsWindows) {
>>> - const bool full_match = true;
>>> - const bool remove_backup_dots = true;
>>> - const bool match = true;
>>> std::pair<const char *, const char *> tests[] = {
>>> {R"(C:\foo\bar\baz)", R"(C:\foo\foo\..\bar\baz)"},
>>> {R"(C:\bar\baz)", R"(C:\foo\..\bar\baz)"},
>>> @@ -170,18 +148,11 @@ TEST(FileSpecTest, EqualDotsWindows) {
>>> for (const auto &test : tests) {
>>> FileSpec one(test.first, false, FileSpec::ePathSyntaxWindows);
>>> FileSpec two(test.second, false, FileSpec::ePathSyntaxWindows);
>>> - 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);
>>> + EXPECT_EQ(one, two);
>>> }
>>> }
>>>
>>> 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)"},
>>> @@ -193,18 +164,11 @@ TEST(FileSpecTest, EqualDotsPosix) {
>>> 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);
>>> + EXPECT_EQ(one, two);
>>> }
>>> }
>>>
>>> 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"(/.)"},
>>> @@ -214,11 +178,7 @@ TEST(FileSpecTest, EqualDotsPosixRoot) {
>>> 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);
>>> + EXPECT_EQ(one, two);
>>> }
>>> }
>>>
>>> @@ -235,22 +195,25 @@ TEST(FileSpecTest, GetNormalizedPath) {
>>> {"/foo//bar/./baz", "/foo/bar/baz"},
>>> {"/./foo", "/foo"},
>>> {"/", "/"},
>>> - {"//", "//"},
>>> + // TODO: fix llvm::sys::path::remove_dots() to return "//" below.
>>> + //{"//", "//"},
>>> {"//net", "//net"},
>>> {"/..", "/"},
>>> {"/.", "/"},
>>> {"..", ".."},
>>> - {".", "."},
>>> + {".", ""},
>>> {"../..", "../.."},
>>> - {"foo/..", "."},
>>> + {"foo/..", ""},
>>> {"foo/../bar", "bar"},
>>> {"../foo/..", ".."},
>>> {"./foo", "foo"},
>>> + {"././foo", "foo"},
>>> + {"../foo", "../foo"},
>>> + {"../../foo", "../../foo"},
>>> };
>>> for (auto test : posix_tests) {
>>> EXPECT_EQ(test.second,
>>> FileSpec(test.first, false, FileSpec::ePathSyntaxPosix)
>>> - .GetNormalizedPath()
>>> .GetPath());
>>> }
>>>
>>> @@ -265,21 +228,25 @@ TEST(FileSpecTest, GetNormalizedPath) {
>>> // {R"(\\net)", R"(\\net)"},
>>> {R"(c:\..)", R"(c:\)"},
>>> {R"(c:\.)", R"(c:\)"},
>>> - {R"(\..)", R"(\)"},
>>> + // TODO: fix llvm::sys::path::remove_dots() to return "\" below.
>>> + {R"(\..)", R"(\..)"},
>>> // {R"(c:..)", R"(c:..)"},
>>> {R"(..)", R"(..)"},
>>> - {R"(.)", R"(.)"},
>>> - {R"(c:..\..)", R"(c:..\..)"},
>>> + {R"(.)", R"()"},
>>> + // TODO: fix llvm::sys::path::remove_dots() to return "c:\" below.
>>> + {R"(c:..\..)", R"(c:\..\..)"},
>>> {R"(..\..)", R"(..\..)"},
>>> - {R"(foo\..)", R"(.)"},
>>> + {R"(foo\..)", R"()"},
>>> {R"(foo\..\bar)", R"(bar)"},
>>> {R"(..\foo\..)", R"(..)"},
>>> {R"(.\foo)", R"(foo)"},
>>> + {R"(.\.\foo)", R"(foo)"},
>>> + {R"(..\foo)", R"(..\foo)"},
>>> + {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;
>>> }
>>> @@ -308,3 +275,4 @@ TEST(FileSpecTest, FormatFileSpec) {
>>> EXPECT_EQ("foo", llvm::formatv("{0:F}", F).str());
>>> EXPECT_EQ("(empty)", llvm::formatv("{0:D}", F).str());
>>> }
>>> +
>>>
>>>
>>> _______________________________________________
>>> lldb-commits mailing list
>>> lldb-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at lists.llvm.org <mailto:lldb-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20180427/bcce83ef/attachment-0001.html>
More information about the lldb-commits
mailing list