[Lldb-commits] [lldb] r222075 - For some reason, sometimes the directory paths that clang emits have internal

jingham at apple.com jingham at apple.com
Mon Nov 17 11:36:56 PST 2014


Thanks for the reminder.  I did check llvm::sys::path before I headed into this.  It doesn't have a routine that did exactly that I wanted, though it did have a little tokenizer/iterator that I could have used.  But the path utility in llvm currently hard-codes the assumption that the paths you are manipulating are all Host paths - it's pretty free with the assumption that they exist.  You can't make either of those assumptions about include paths in DWARF.  Of course, the llvm::sys::path code could be reworked to remove these assumptions, etc.  But that would have turned a fairly simple patch into a much bigger deal, and I've spend all the time I can afford on this bug for now.

Jim


> On Nov 14, 2014, at 6:55 PM, Zachary Turner <zturner at google.com> wrote:
> 
> You might check if llvm has a method to remove all the dots and stuff already.  I think it does, but I could be wrong.  
> 
> On Fri Nov 14 2014 at 5:58:15 PM Jim Ingham <jingham at apple.com> wrote:
> Author: jingham
> Date: Fri Nov 14 19:54:26 2014
> New Revision: 222075
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=222075&view=rev
> Log:
> For some reason, sometimes the directory paths that clang emits have internal
> relative paths, like:
> 
> /whatever/llvm/lib/Sema/../../include/llvm/Sema/
> 
> That causes problems with our type uniquing, since we use the declaration file
> and line as one component of the uniquing, and different ways of getting to the
> same file will have different directory spellings, though they are functionally
> equivalent.  We end up with two copies of the exact same type because of this,
> and that makes the expression parser give "duplicate type" errors.
> 
> I added a method to resolve paths with ../ in them and used that in the FileSpec::Equals,
> for comparing Declarations and for doing Breakpoint compares as well, since they also
> suffer from this if you specify breakpoints by full path (since nobody knows what
> ../'s to insert...)
> 
> <rdar://problem/18765814>
> 
> Modified:
>     lldb/trunk/include/lldb/Core/FileSpecList.h
>     lldb/trunk/include/lldb/Host/FileSpec.h
>     lldb/trunk/source/Core/FileSpecList.cpp
>     lldb/trunk/source/Host/common/FileSpec.cpp
>     lldb/trunk/source/Symbol/CompileUnit.cpp
>     lldb/trunk/source/Symbol/Declaration.cpp
> 
> Modified: lldb/trunk/include/lldb/Core/FileSpecList.h
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/FileSpecList.h?rev=222075&r1=222074&r2=222075&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Core/FileSpecList.h (original)
> +++ lldb/trunk/include/lldb/Core/FileSpecList.h Fri Nov 14 19:54:26 2014
> @@ -119,12 +119,15 @@ 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) const;
> +    FindFileIndex (size_t idx, const FileSpec &file, bool full, bool remove_backup_dots = false) const;
> 
>      //------------------------------------------------------------------
>      /// Get file at index.
> 
> Modified: lldb/trunk/include/lldb/Host/FileSpec.h
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/FileSpec.h?rev=222075&r1=222074&r2=222075&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Host/FileSpec.h (original)
> +++ lldb/trunk/include/lldb/Host/FileSpec.h Fri Nov 14 19:54:26 2014
> @@ -246,7 +246,7 @@ public:
>      Compare (const FileSpec& lhs, const FileSpec& rhs, bool full);
> 
>      static bool
> -    Equal (const FileSpec& a, const FileSpec& b, bool full);
> +    Equal (const FileSpec& a, const FileSpec& b, bool full, bool remove_backups = false);
> 
>      //------------------------------------------------------------------
>      /// Dump this object to a Stream.
> @@ -582,6 +582,20 @@ public:
>      static void Normalize(llvm::SmallVectorImpl<char> &path, PathSyntax syntax = ePathSyntaxHostNative);
>      static void DeNormalize(llvm::SmallVectorImpl<char> &path, PathSyntax syntax = ePathSyntaxHostNative);
> 
> +
> +    //------------------------------------------------------------------
> +    /// 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);
> +
>      //------------------------------------------------------------------
>      /// Change the file specified with a new path.
>      ///
> 
> Modified: lldb/trunk/source/Core/FileSpecList.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/FileSpecList.cpp?rev=222075&r1=222074&r2=222075&view=diff
> ==============================================================================
> --- lldb/trunk/source/Core/FileSpecList.cpp (original)
> +++ lldb/trunk/source/Core/FileSpecList.cpp Fri Nov 14 19:54:26 2014
> @@ -107,7 +107,7 @@ FileSpecList::Dump(Stream *s, const char
>  // it is found, else UINT32_MAX is returned.
>  //------------------------------------------------------------------
>  size_t
> -FileSpecList::FindFileIndex (size_t start_idx, const FileSpec &file_spec, bool full) const
> +FileSpecList::FindFileIndex (size_t start_idx, const FileSpec &file_spec, bool full, bool remove_dots) const
>  {
>      const size_t num_files = m_files.size();
> 
> @@ -124,7 +124,7 @@ FileSpecList::FindFileIndex (size_t star
>          }
>          else
>          {
> -            if (FileSpec::Equal (m_files[idx], file_spec, full))
> +            if (FileSpec::Equal (m_files[idx], file_spec, full, remove_dots))
>                  return idx;
>          }
>      }
> 
> Modified: lldb/trunk/source/Host/common/FileSpec.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/FileSpec.cpp?rev=222075&r1=222074&r2=222075&view=diff
> ==============================================================================
> --- lldb/trunk/source/Host/common/FileSpec.cpp (original)
> +++ lldb/trunk/source/Host/common/FileSpec.cpp Fri Nov 14 19:54:26 2014
> @@ -448,15 +448,129 @@ FileSpec::Compare(const FileSpec& a, con
>  }
> 
>  bool
> -FileSpec::Equal (const FileSpec& a, const FileSpec& b, bool full)
> +FileSpec::Equal (const FileSpec& a, const FileSpec& b, bool full, bool remove_backups)
>  {
>      if (!full && (a.GetDirectory().IsEmpty() || b.GetDirectory().IsEmpty()))
>          return a.m_filename == b.m_filename;
> -    else
> +    else if (remove_backups == false)
>          return a == b;
> +    else
> +    {
> +        if (a.m_filename != b.m_filename)
> +            return false;
> +        if (a.m_directory == b.m_directory)
> +            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 a_without_dots == b_without_dots;
> +    }
>  }
> 
> +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;
> +    }
> +    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)
> +            {
> +                if (!before_sep.empty())
> +                {
> +                    result.append(before_sep.data(), before_sep.size());
> +                }
> +            }
> +            break;
> +        }
> +        had_dots = true;
> +
> +        unsigned num_backups = 1;
> +        while (curpos.startswith(backup_sep))
> +        {
> +            num_backups++;
> +            curpos = curpos.slice(backup_sep.size(), curpos.size());
> +        }
> +
> +        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;
> +            }
> +        }
> +        result.append(before_sep.data(), end_pos);
> +    }
> +
> +    if (had_dots)
> +        result_const_str.SetCString(result.c_str());
> +    else
> +        result_const_str = input_const_str;
> +
> +    return;
> +}
> 
>  //------------------------------------------------------------------
>  // Dump the object to the supplied stream. If the object contains
> 
> Modified: lldb/trunk/source/Symbol/CompileUnit.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/CompileUnit.cpp?rev=222075&r1=222074&r2=222075&view=diff
> ==============================================================================
> --- lldb/trunk/source/Symbol/CompileUnit.cpp (original)
> +++ lldb/trunk/source/Symbol/CompileUnit.cpp Fri Nov 14 19:54:26 2014
> @@ -325,18 +325,19 @@ CompileUnit::ResolveSymbolContext
>      // when finding file indexes
>      std::vector<uint32_t> file_indexes;
>      const bool full_match = (bool)file_spec.GetDirectory();
> -    bool file_spec_matches_cu_file_spec = FileSpec::Equal(file_spec, *this, full_match);
> +    const bool remove_backup_dots = true;
> +    bool file_spec_matches_cu_file_spec = FileSpec::Equal(file_spec, *this, full_match, remove_backup_dots);
> 
>      // If we are not looking for inlined functions and our file spec doesn't
>      // match then we are done...
>      if (file_spec_matches_cu_file_spec == false && check_inlines == false)
>          return 0;
> 
> -    uint32_t file_idx = GetSupportFiles().FindFileIndex (1, file_spec, true);
> +    uint32_t file_idx = GetSupportFiles().FindFileIndex (1, file_spec, true, remove_backup_dots);
>      while (file_idx != UINT32_MAX)
>      {
>          file_indexes.push_back (file_idx);
> -        file_idx = GetSupportFiles().FindFileIndex (file_idx + 1, file_spec, true);
> +        file_idx = GetSupportFiles().FindFileIndex (file_idx + 1, file_spec, true, remove_backup_dots);
>      }
> 
>      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=222075&r1=222074&r2=222075&view=diff
> ==============================================================================
> --- lldb/trunk/source/Symbol/Declaration.cpp (original)
> +++ lldb/trunk/source/Symbol/Declaration.cpp Fri Nov 14 19:54:26 2014
> @@ -110,7 +110,7 @@ lldb_private::operator == (const Declara
>              return lhs.GetFile() == rhs.GetFile();
>  #else
>      if (lhs.GetLine () == rhs.GetLine ())
> -        return lhs.GetFile() == rhs.GetFile();
> +        return FileSpec::Equal(lhs.GetFile(),rhs.GetFile(), true, true);
>  #endif
>      return false;
>  }
> 
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits





More information about the lldb-commits mailing list