[2/2] Fix Path support on Windows

[zturner]

Summary

This gives FileSpec the notion of a path syntax, of which there are 3. Unix Native, Windows Native, and Host Native, which chooses between Unix and Windows automatically. This allows cross-platform support (e.g. storing a Unix Path on a Windows Host, and vice versa).

There are remaining Path issues in PathMappingList. These should be fixed independently by storing FileSpecs instead of ConstStrings, and using the fixes in this patch. Alternatively, llvm::sys::path contains path methods that might help here.

[zturner]

Fixed some issues related to normalization and de-normalization.

[zturner]

Accidentally reversed the diff on the last upload.

[zturner]

Removed dead function.

[deepak2427]

Thanks for fixing this. I haven't had a chance to test this patch with our cross-platform issues yet, have to look at some other issues first.
Just a couple of doubts I had.

• Are the paths always going to default as ePathSyntaxHostNative and then converted based on the Host?
• In 733 llvm::SmallString<64> denormalized; , is the length of 64 enough?
• I'm not that familiar with LLVM data structures, however would it be better to keep llvm::SmallString itself for the Normalize/Denormalize functions to keep it consistent?

[zturner]

In D4675#11, @deepak2427 wrote:

Thanks for fixing this. I haven't had a chance to test this patch with our cross-platform issues yet, have to look at some other issues first.
Just a couple of doubts I had.

• Are the paths always going to default as ePathSyntaxHostNative and then converted based on the Host?
• In 733 llvm::SmallString<64> denormalized; , is the length of 64 enough?
• I'm not that familiar with LLVM data structures, however would it be better to keep llvm::SmallString itself for the Normalize/Denormalize functions to keep it consistent?

Paths default to SyntaxHostNative because that makes the semantics post-patch equivalent to pre-patch. Normalized form is basically unix form, so if the syntax ends up as ePathSyntaxPosix (either because it was explicitly specified or because host was specified and your host is posix), then normalization and de-normalization are both no-ops.

SmallString<64> just means that it keeps 64 bytes on the stack, and if it grows longer than that, it will fallback to help allocation. It still supports arbitrarily long strings, but paths tend to be small, so that's why a small number was chosen.

include/lldb/Host/FileSpec.h

typedef enum FileType
         eFileTypeDirectory,
         eFileTypePipe,
         eFileTypeRegular,
         eFileTypeSocket,
         eFileTypeSymbolicLink,
         eFileTypeOther
     } FileType;
 
+    enum PathSyntax
+    {
+        ePathSyntaxPosix,
+        ePathSyntaxWindows,
+        ePathSyntaxHostNative
+    };
+
     FileSpec();
 
     //------------------------------------------------------------------
     /// Constructor with path.
     ///
     /// Takes a path to a file which can be just a filename, or a full
     /// path. If \a path is not NULL or empty, this function will call
     /// FileSpec::SetFile (const char *path, bool resolve).
     ///
     /// @param[in] path
     ///     The full or partial path to a file.
     ///
     /// @param[in] resolve_path
     ///     If \b true, then we resolve the path with realpath,
     ///     if \b false we trust the path is in canonical form already.
     ///
     /// @see FileSpec::SetFile (const char *path, bool resolve)
     //------------------------------------------------------------------
-    explicit FileSpec (const char *path, bool resolve_path);
+    explicit FileSpec (const char *path, bool resolve_path, PathSyntax syntax = ePathSyntaxHostNative);
 
     //------------------------------------------------------------------
     /// Copy constructor
     ///
     /// Makes a copy of the uniqued directory and filename strings from
     /// \a rhs.
     ///
     /// @param[in] rhs
public:
     /// resolve the file path when you set the file.
     //------------------------------------------------------------------
     bool
     ResolvePath ();
 
     uint64_t
     GetByteSize() const;
 
+    PathSyntax
+    GetPathSyntax() const;
+
     //------------------------------------------------------------------
     /// Directory string get accessor.
     ///
     /// @return
     ///     A reference to the directory string object.
     //------------------------------------------------------------------
     ConstString &
     GetDirectory ();
public:
     ///     Returns the number of characters that would be needed to 
     ///     properly copy the full path into \a path. If the returned
     ///     number is less than \a max_path_length, then the path is
     ///     properly copied and terminated. If the return value is 
     ///     >= \a max_path_length, then the path was truncated (but is
     ///     still NULL terminated).
     //------------------------------------------------------------------
     size_t
-    GetPath (char *path, size_t max_path_length) const;
+    GetPath (char *path, size_t max_path_length, bool denormalize = true) const;
 
     //------------------------------------------------------------------
     /// Extract the full path to the file.
     ///
     /// Extract the directory and path into a std::string, which is returned.
     ///
     /// @return
     ///     Returns a std::string with the directory and filename 
     ///     concatenated.
     //------------------------------------------------------------------
     std::string
-    GetPath () const;
+    GetPath (bool denormalize = true) const;
 
     //------------------------------------------------------------------
     /// Extract the extension of the file.
     ///
     /// Returns a ConstString that represents the extension of the filename
     /// for this FileSpec object. If this object does not represent a file,
     /// or the filename has no extension, ConstString(NULL) is returned.
     /// The dot ('.') character is not returned as part of the extension
public:
     ///
     /// @return
     ///     A shared pointer to the data. This shared pointer can
     ///     contain a NULL DataBuffer pointer, so the contained pointer
     ///     must be checked prior to using it.
     //------------------------------------------------------------------
     lldb::DataBufferSP
     ReadFileContentsAsCString(Error *error_ptr = NULL);
+
+    static void Normalize(llvm::StringRef path, llvm::SmallVectorImpl<char> &result, PathSyntax syntax);
+    static void DeNormalize(llvm::StringRef path, llvm::SmallVectorImpl<char> &result, PathSyntax syntax);
+
     //------------------------------------------------------------------
     /// Change the file specified with a new path.
     ///
     /// Update the contents of this object with a new path. The path will
     /// be split up into a directory and filename and stored as uniqued
     /// string values for quick comparison and efficient memory usage.
     ///
     /// @param[in] path
     ///     A full, partial, or relative path to a file.
     ///
     /// @param[in] resolve_path
     ///     If \b true, then we will try to resolve links the path using
     ///     the static FileSpec::Resolve.
     //------------------------------------------------------------------
     void
-    SetFile (const char *path, bool resolve_path);
+    SetFile (const char *path, bool resolve_path, PathSyntax syntax = ePathSyntaxHostNative);
 
     bool
     IsResolved () const
     {
         return m_is_resolved;
     }
 
     //------------------------------------------------------------------

 
 protected:
     //------------------------------------------------------------------
     // Member variables
     //------------------------------------------------------------------
     ConstString m_directory;    ///< The uniqued directory path
     ConstString m_filename;     ///< The uniqued filename path
     mutable bool m_is_resolved; ///< True if this path has been resolved.
+    PathSyntax m_syntax;        ///< The syntax that this path uses (e.g. Windows / Posix)
 };
 
 //----------------------------------------------------------------------
 /// Dump a FileSpec object to a stream
 //----------------------------------------------------------------------
 Stream& operator << (Stream& s, const FileSpec& f);
 
 } // namespace lldb_private
 
 #endif  // #if defined(__cplusplus)
 #endif  // liblldb_FileSpec_h_

include/lldb/Host/Host.h

public:
     ///
     /// @return
     ///     \b true if the thread name was able to be set, \b false
     ///     otherwise.
     static bool
     SetShortThreadName (lldb::pid_t pid, lldb::tid_t tid, const char *name, size_t len);
 
     //------------------------------------------------------------------
+    /// Gets the host environment's native path syntax (Windows / Posix).
+    ///
+    /// @return
+    ///     \b One of {FileSpec::ePathSyntaxWindows, FileSpec::ePathSyntaxPosix}
+    //------------------------------------------------------------------
+    static FileSpec::PathSyntax
+    GetHostPathSyntax();
+
+    //------------------------------------------------------------------
     /// Gets the FileSpec of the user profile directory.  On Posix-platforms
     /// this is ~, and on windows this is generally something like
     /// C:\Users\Alice.
     ///
     /// @return
     ///     \b A file spec with the path to the user's home directory.
     //------------------------------------------------------------------
     static FileSpec

source/Host/common/FileSpec.cpp

FileSpec::FileSpec() :
     m_filename()
 {
 }
 
 //------------------------------------------------------------------
 // Default constructor that can take an optional full path to a
 // file on disk.
 //------------------------------------------------------------------
-FileSpec::FileSpec(const char *pathname, bool resolve_path) :
+FileSpec::FileSpec(const char *pathname, bool resolve_path, PathSyntax syntax) :
     m_directory(),
     m_filename(),
     m_is_resolved(false)
 {
     if (pathname && pathname[0])
-        SetFile(pathname, resolve_path);
+        SetFile(pathname, resolve_path, syntax);
 }
 
 //------------------------------------------------------------------
 // Copy constructor
 //------------------------------------------------------------------
 FileSpec::FileSpec(const FileSpec& rhs) :
     m_directory (rhs.m_directory),
     m_filename (rhs.m_filename),
FileSpec::operator= (const FileSpec& rhs)
     {
         m_directory = rhs.m_directory;
         m_filename = rhs.m_filename;
         m_is_resolved = rhs.m_is_resolved;
     }
     return *this;
 }
 
+void FileSpec::Normalize(llvm::StringRef path, llvm::SmallVectorImpl<char> &result, PathSyntax syntax)
+{
+    result.clear();
+    result.append(path.begin(), path.end());
+    if (syntax == ePathSyntaxPosix || (syntax == ePathSyntaxHostNative && Host::GetHostPathSyntax() == ePathSyntaxPosix))
+        return;
+
+    std::replace(result.begin(), result.end(), '\\', '/');
+}
+
+void FileSpec::DeNormalize(llvm::StringRef path, llvm::SmallVectorImpl<char> &result, PathSyntax syntax)
+{
+    result.clear();
+    result.append(path.begin(), path.end());
+    if (syntax == ePathSyntaxPosix || (syntax == ePathSyntaxHostNative && Host::GetHostPathSyntax() == ePathSyntaxPosix))
+        return;
+
+    std::replace(result.begin(), result.end(), '/', '\\');
+}
+
 //------------------------------------------------------------------
 // Update the contents of this object with a new path. The path will
 // be split up into a directory and filename and stored as uniqued
 // string values for quick comparison and efficient memory usage.
 //------------------------------------------------------------------
 void
-FileSpec::SetFile (const char *pathname, bool resolve)
+FileSpec::SetFile (const char *pathname, bool resolve, PathSyntax syntax)
 {
     m_filename.Clear();
     m_directory.Clear();
     m_is_resolved = false;
+    m_syntax = (syntax == ePathSyntaxHostNative) ? Host::GetHostPathSyntax() : syntax;
+
     if (pathname == NULL || pathname[0] == '\0')
         return;
 
-    char resolved_path[PATH_MAX];
+    llvm::SmallString<64> normalized;
+    Normalize(pathname, normalized, syntax);
+
+    std::vector<char> resolved_path(PATH_MAX);
     bool path_fit = true;
     
     if (resolve)
     {
-        path_fit = (FileSpec::Resolve (pathname, resolved_path, sizeof(resolved_path)) < sizeof(resolved_path) - 1);
+        path_fit = (FileSpec::Resolve (normalized.c_str(), &resolved_path[0], resolved_path.size()) < resolved_path.size() - 1);
         m_is_resolved = path_fit;
     }
     else
     {
         // Copy the path because "basename" and "dirname" want to muck with the
         // path buffer
-        if (::strlen (pathname) > sizeof(resolved_path) - 1)
+        if (normalized.size() > resolved_path.size() - 1)
             path_fit = false;
         else
-            ::strcpy (resolved_path, pathname);
+            ::strcpy (&resolved_path[0], normalized.c_str());
     }
     
-    
     if (path_fit)
     {
-        llvm::StringRef resolve_path_ref(resolved_path);
+        llvm::StringRef resolve_path_ref(&resolved_path[0]);
         llvm::StringRef filename_ref = llvm::sys::path::filename(resolve_path_ref);
         if (!filename_ref.empty())
         {
             m_filename.SetString (filename_ref);
             llvm::StringRef directory_ref = llvm::sys::path::parent_path(resolve_path_ref);
             if (!directory_ref.empty())
                 m_directory.SetString(directory_ref);
         }
         else
-            m_directory.SetCString(resolved_path);
+            m_directory.SetCString(&resolved_path[0]);
     }
 }
 
 //----------------------------------------------------------------------
 // Convert to pointer operator. This allows code to check any FileSpec
 // objects to see if they contain anything valid using code such as:
 //
 //  if (file_spec)

 
 bool
 FileSpec::ResolvePath ()
 {
     if (m_is_resolved)
         return true;    // We have already resolved this path
 
     char path_buf[PATH_MAX];    
-    if (!GetPath (path_buf, PATH_MAX))
+    if (!GetPath (path_buf, PATH_MAX, false))
         return false;
     // SetFile(...) will set m_is_resolved correctly if it can resolve the path
     SetFile (path_buf, true);
     return m_is_resolved; 
 }
 
 uint64_t
 FileSpec::GetByteSize() const
 {
     struct stat file_stats;
     if (GetFileStats (this, &file_stats))
         return file_stats.st_size;
     return 0;
 }
 
+FileSpec::PathSyntax
+FileSpec::GetPathSyntax() const
+{
+    return m_syntax;
+}
+
 FileSpec::FileType
 FileSpec::GetFileType () const
 {
     struct stat file_stats;
     if (GetFileStats (this, &file_stats))
     {
         mode_t file_type = file_stats.st_mode & S_IFMT;
         switch (file_type)

 }
 
 //------------------------------------------------------------------
 // Extract the directory and path into a fixed buffer. This is
 // needed as the directory and path are stored in separate string
 // values.
 //------------------------------------------------------------------
 size_t
-FileSpec::GetPath(char *path, size_t path_max_len) const
+FileSpec::GetPath(char *path, size_t path_max_len, bool denormalize) const
 {
-    if (path_max_len)
-    {
-        const char *dirname = m_directory.GetCString();
-        const char *filename = m_filename.GetCString();
-        if (dirname)
-        {
-            if (filename)
-                return ::snprintf (path, path_max_len, "%s/%s", dirname, filename);
-            else
-                return ::snprintf (path, path_max_len, "%s", dirname);
-        }
-        else if (filename)
-        {
-            return ::snprintf (path, path_max_len, "%s", filename);
-        }
-    }
-    if (path)
-        path[0] = '\0';
-    return 0;
+    std::string result = GetPath(denormalize);
+
+    size_t result_length = std::min(path_max_len-1, result.length());
+    ::strncpy(path, result.c_str(), result_length + 1);
+    return result_length;
 }
 
 std::string
-FileSpec::GetPath (void) const
+FileSpec::GetPath (bool denormalize) const
 {
-    static ConstString g_slash_only ("/");
-    std::string path;
-    const char *dirname = m_directory.GetCString();
-    const char *filename = m_filename.GetCString();
-    if (dirname)
+    llvm::SmallString<64> result;
+    if (m_directory)
+        result.append(m_directory.GetCString());
+    if (m_filename)
+        llvm::sys::path::append(result, m_filename.GetCString());
+    if (denormalize && !result.empty())
     {
-        path.append (dirname);
-        if (filename && m_directory != g_slash_only)
-            path.append ("/");
-    }
-    if (filename)
-        path.append (filename);
-    return path;
+        llvm::SmallString<64> denormalized;
+        DeNormalize(result.c_str(), denormalized, m_syntax);
+        result = denormalized;
+    }
+
+    return std::string(result.begin(), result.end());
 }
 
 ConstString
 FileSpec::GetFileNameExtension () const
 {
     if (m_filename)
     {
         const char *filename = m_filename.GetCString();

source/Host/common/Host.cpp

if (namebuflen > 0)
         }
         return Host::SetThreadName (pid, tid, namebuf.get());
     }
     return false;
 }
 
 #endif
 
+FileSpec::PathSyntax
+Host::GetHostPathSyntax()
+{
+#if defined(_WIN32)
+    return FileSpec::ePathSyntaxWindows;
+#else
+    return FileSpec::ePathSyntaxPosix;
+#endif
+}
+
 FileSpec
 Host::GetUserProfileFileSpec ()
 {
     static FileSpec g_profile_filespec;
     if (!g_profile_filespec)
     {
         llvm::SmallString<MAX_PATH> path;
         llvm::sys::path::home_directory(path);
#elif defined (__FreeBSD__) || defined (__FreeBSD_kernel__)
             char *exe_path = new char[exe_path_size];
             if (sysctl(exe_path_mib, 4, exe_path, &exe_path_size, NULL, 0) == 0)
                 g_program_filespec.SetFile(exe_path, false);
             delete[] exe_path;
         }
 #elif defined(_WIN32)
         std::vector<char> buffer(PATH_MAX);
         ::GetModuleFileName(NULL, &buffer[0], buffer.size());
-        g_program_filespec.SetFile(&buffer[0], false, FileSpec::ePathSyntaxWindows);
+        g_program_filespec.SetFile(&buffer[0], false);
 #endif
     }
     return g_program_filespec;
 }
 
 #if !defined (__APPLE__) // see Host.mm
 
 bool