[FileSystem] Open File instances through the FileSystem.

[JDevlieghere]

Summary

This patch modifies how we open File instances in LLDB. Rather than passing a path or FileSpec to the constructor, we now go through the virtual file system. This is needed in order to make things work with the VFS.

The current implementation doesn't use the VFS yet. The reason is that in LLDB we mostly use FILE pointers or file descriptors to manipulate files. This is currently not supported by the VFS and I haven't decided to address this yet. The two alternative I see are:

• Having the VFS translate paths. This will only work for virtual file systems where the files actually reside on disk. This would be sufficient for the reproducer use case but lacks generality.
• Having the VFS provide FILE pointers. Still we wouldn't be able to completely support in-memory vfses. I believe on POSIX there's a way to have a file point to memory, but I don't think you can do that for a file descriptor. Also not sure if that would work on Windows?

Both options are incomplete and I don't have the bandwidth to change how LLDB deals with files. I'm currently leaning towards the first alternative because LLVM doesn't provide FILE* APIs and I don't think we should start adding them now. Also I don't want to extend the VFS for just LLDB as that's going to be a pain to hook up and maintain. Anyway, I'm open to suggestions!

[labath]

I am not sure what do you mean by "translating paths" in the VFS. If you mean something like "return a FILE * for /whatever/my_reproducer/vfs/bin/ls when I ask for /bin/ls", then I think that's a good idea, as it's most likely to work with all of our existing code (e.g. mmapping). Although, in this case, i am not sure how much benefit will the llvm VFS bring to the game, as most of the operations could then be implemented by plainly prepending some prefix to a given path.

Getting rid of FILE * is not going to be easy, as it is used for some of our external dependencies (libedit). It's possible to create a fake FILE* on posix systems (fopencookie, fmemopen), but I don't think it's possible to do something like that on windows (which is why I stopped when I was looking at this some time ago).

Also, be aware that there are some places where we open raw_ostreams directly (Debugger::EnableLog comes to mind). I guess those will need to go through the FileSystem too...

source/Host/common/FileSystem.cpp

253 ↗

(On Diff #172295)

File::eOpenOptionRead should be sufficient, no?

source/Host/posix/FileSystem.cpp

74–79 ↗

(On Diff #172295)

Why are these two const? It seems that at least with eOpenOptionCanCreate, one can create a new file, which is definitely not a "const" operation.

source/Utility/FileSpec.cpp

321–322 ↗

(On Diff #172295)

We already have operator bool for this.

[JDevlieghere]

In D54020#1285201, @labath wrote:

I am not sure what do you mean by "translating paths" in the VFS. If you mean something like "return a FILE * for /whatever/my_reproducer/vfs/bin/ls when I ask for /bin/ls", then I think that's a good idea, as it's most likely to work with all of our existing code (e.g. mmapping). Although, in this case, i am not sure how much benefit will the llvm VFS bring to the game, as most of the operations could then be implemented by plainly prepending some prefix to a given path.

Let me try to explain this better. This is mostly a question about what kind of API the VFS (which lives in LLVM) provides to deal with files in lldb (i.e. through FILE* and file descriptors).

The first possibility is providing a method in the VFS that takes a "virtual" path and returns the "underlying" path. Something like Optional<path> vfs::getUnderlyingPath(path). This doesn't just have to be a prefix thing but you are right that it's mostly going to be just that. The problem is that this method wouldn't work for virtual file systems that don't have files on disk. Hence the lack of generality.

The second possibility is providing a method in the VFS that returns FILE*. When I was writing this I was still hoping that there was something like the fake FILE* you mentioned below. If that were the case it could support virtual file systems that have files that strictly live in memory. But if this doesn't work for Windows it's not really any better than the first alternative.

Getting rid of FILE * is not going to be easy, as it is used for some of our external dependencies (libedit). It's possible to create a fake FILE* on posix systems (fopencookie, fmemopen), but I don't think it's possible to do something like that on windows (which is why I stopped when I was looking at this some time ago).

Yup, this is not something I'm proposing.

Also, be aware that there are some places where we open raw_ostreams directly (Debugger::EnableLog comes to mind). I guess those will need to go through the FileSystem too...

Yup, we need to audit all file access. This particular example actually goes through file so it should be covered by this change.

[JDevlieghere]

source/Host/common/FileSystem.cpp

253 ↗

(On Diff #172295)

Sorry, I don't know what you mean?

source/Host/posix/FileSystem.cpp

74–79 ↗

(On Diff #172295)

Depends on what you consider const, the real filesystem or the class. But yeah, the const doesn't add much so I can un-const this.

[JDevlieghere]

Address Pavel's feedback:

• Un-const open functions.
• Remove ::Empty() again as it was redundant with operator bool().
• Rename OpenFile to just Open. We might need more Open methods in the future.

[labath]

In D54020#1285539, @JDevlieghere wrote:

In D54020#1285201, @labath wrote:

I am not sure what do you mean by "translating paths" in the VFS. If you mean something like "return a FILE * for /whatever/my_reproducer/vfs/bin/ls when I ask for /bin/ls", then I think that's a good idea, as it's most likely to work with all of our existing code (e.g. mmapping). Although, in this case, i am not sure how much benefit will the llvm VFS bring to the game, as most of the operations could then be implemented by plainly prepending some prefix to a given path.

Let me try to explain this better. This is mostly a question about what kind of API the VFS (which lives in LLVM) provides to deal with files in lldb (i.e. through FILE* and file descriptors).

The first possibility is providing a method in the VFS that takes a "virtual" path and returns the "underlying" path. Something like Optional<path> vfs::getUnderlyingPath(path). This doesn't just have to be a prefix thing but you are right that it's mostly going to be just that. The problem is that this method wouldn't work for virtual file systems that don't have files on disk. Hence the lack of generality.

Ok, I think we're on the same page here. What I was wondering is that given this lack of generality (this operation would only be supported on DiskBackedFilesystem, and not all other kinds of file systems), whether it is not better to just do a custom solution instead of going through the VFS layer (thereby removing one layer, since now we have lldb_private::FileSystem sitting on top of llvm::VirtualFileSystem, sitting on top of the real thing). E.g., if we know we can restrict ourselves to the case where the on disk layout matches the real system, then all filesystem operations can be implemented very trivially via something like:

auto virtual_op(const Twine &Path, ...) { return real_op(Prefix+Path, ...); }

I am not sure whether this is a good idea (there's definitely a case to be made for reusing existing infrastructure), but it is something to think about.

Also, be aware that there are some places where we open raw_ostreams directly (Debugger::EnableLog comes to mind). I guess those will need to go through the FileSystem too...

Yup, we need to audit all file access. This particular example actually goes through file so it should be covered by this change.

You're right, I should have looked at this more closely. It should be fine (assuming we can have real fds for the virtual files).

source/Host/common/FileSystem.cpp

253 ↗

(On Diff #172295)

I meant to remove the superfluous OpenOptions qualification, As this is a non-class enum, it is not necessary, and it doesn't help clarity, since the enum value already contains the type name. None of the existing code uses this kind of qualification.

[JDevlieghere]

In D54020#1285799, @labath wrote:

Ok, I think we're on the same page here. What I was wondering is that given this lack of generality (this operation would only be supported on DiskBackedFilesystem, and not all other kinds of file systems), whether it is not better to just do a custom solution instead of going through the VFS layer (thereby removing one layer, since now we have lldb_private::FileSystem sitting on top of llvm::VirtualFileSystem, sitting on top of the real thing). E.g., if we know we can restrict ourselves to the case where the on disk layout matches the real system, then all filesystem operations can be implemented very trivially via something like:

auto virtual_op(const Twine &Path, ...) { return real_op(Prefix+Path, ...); }

I am not sure whether this is a good idea (there's definitely a case to be made for reusing existing infrastructure), but it is something to think about.

Thanks Pavel, I really appreciate you thinking this through with me! I understand what you're saying but I still think the VFS is worthwhile:

• The lack of generality I brought up only applies to in-memory file systems. Every VFS implementation that's backed by disk would still work.
• The only reason hard-coding the vfs implementation (because that's essentially what the prefix thing is) is the coincidence that no prefix equals the real file system.
• We could have a prefix VFS implementation that's really simple and self-contained without cluttering the LLDB FileSystem class with details.

In the end I believe the indirection is more of a feature than a bug. I'm worried that, if we do our own thing, we'll end up in this situation where we have something in LLDB that looks really similar to something in LLVM but isn't quite the same. As soon as we want to do more complex than just prepending a prefix we'll need an abstraction anyway, so it might as well be the thing from LLVM.

source/Host/common/FileSystem.cpp

253 ↗

(On Diff #172295)

Got it, thanks!

lldb/trunk/include/lldb/Host/File.h

public:
 
   File(FILE *fh, bool transfer_ownership)
       : IOObject(eFDTypeFile, false), m_descriptor(kInvalidDescriptor),
         m_stream(fh), m_options(0), m_own_stream(transfer_ownership),
         m_is_interactive(eLazyBoolCalculate),
         m_is_real_terminal(eLazyBoolCalculate),
         m_supports_colors(eLazyBoolCalculate) {}
 
-  //------------------------------------------------------------------
-  /// Constructor with path.
-  ///
-  /// Takes a path to a file which can be just a filename, or a full path. If
-  /// \a path is not nullptr or empty, this function will call File::Open
-  /// (const char *path, uint32_t options, uint32_t permissions).
-  ///
-  /// @param[in] path
-  ///     The full or partial path to a file.
-  ///
-  /// @param[in] options
-  ///     Options to use when opening (see File::OpenOptions)
-  ///
-  /// @param[in] permissions
-  ///     Options to use when opening (see File::Permissions)
-  ///
-  /// @see File::Open (const char *path, uint32_t options, uint32_t
-  /// permissions)
-  //------------------------------------------------------------------
-  File(const char *path, uint32_t options,
-       uint32_t permissions = lldb::eFilePermissionsFileDefault);
-
-  //------------------------------------------------------------------
-  /// Constructor with FileSpec.
-  ///
-  /// Takes a FileSpec pointing to a file which can be just a filename, or a
-  /// full path. If \a path is not nullptr or empty, this function will call
-  /// File::Open (const char *path, uint32_t options, uint32_t permissions).
-  ///
-  /// @param[in] filespec
-  ///     The FileSpec for this file.
-  ///
-  /// @param[in] options
-  ///     Options to use when opening (see File::OpenOptions)
-  ///
-  /// @param[in] permissions
-  ///     Options to use when opening (see File::Permissions)
-  ///
-  /// @see File::Open (const char *path, uint32_t options, uint32_t
-  /// permissions)
-  //------------------------------------------------------------------
-  File(const FileSpec &filespec, uint32_t options,
-       uint32_t permissions = lldb::eFilePermissionsFileDefault);
-
   File(int fd, bool transfer_ownership)
       : IOObject(eFDTypeFile, transfer_ownership), m_descriptor(fd),
         m_stream(kInvalidStream), m_options(0), m_own_stream(false),
         m_is_interactive(eLazyBoolCalculate),
         m_is_real_terminal(eLazyBoolCalculate) {}
 
   //------------------------------------------------------------------
   /// Destructor.
public:
   //------------------------------------------------------------------
   /// Get the file spec for this file.
   ///
   /// @return
   ///     A reference to the file specification object.
   //------------------------------------------------------------------
   Status GetFileSpec(FileSpec &file_spec) const;
 
-  //------------------------------------------------------------------
-  /// Open a file for read/writing with the specified options.
-  ///
-  /// Takes a path to a file which can be just a filename, or a full path.
-  ///
-  /// @param[in] path
-  ///     The full or partial path to a file.
-  ///
-  /// @param[in] options
-  ///     Options to use when opening (see File::OpenOptions)
-  ///
-  /// @param[in] permissions
-  ///     Options to use when opening (see File::Permissions)
-  //------------------------------------------------------------------
-  Status Open(const char *path, uint32_t options,
-              uint32_t permissions = lldb::eFilePermissionsFileDefault);
-
   Status Close() override;
 
   void Clear();
 
   int GetDescriptor() const;
 
   WaitableHandle GetWaitableHandle() override;
 
public:
   ///     format string \a format.
   //------------------------------------------------------------------
   size_t Printf(const char *format, ...) __attribute__((format(printf, 2, 3)));
 
   size_t PrintfVarArg(const char *format, va_list args);
 
   void SetOptions(uint32_t options) { m_options = options; }
 
+  static bool DescriptorIsValid(int descriptor) { return descriptor >= 0; };
+
 protected:
-  bool DescriptorIsValid() const { return m_descriptor >= 0; }
+  bool DescriptorIsValid() const { return DescriptorIsValid(m_descriptor); }
 
   bool StreamIsValid() const { return m_stream != kInvalidStream; }
 
   void CalculateInteractiveAndTerminal();
 
   //------------------------------------------------------------------
   // Member variables
   //------------------------------------------------------------------

lldb/trunk/include/lldb/Host/FileSystem.h

 //===-- FileSystem.h --------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_Host_FileSystem_h
 #define liblldb_Host_FileSystem_h
 
+#include "lldb/Host/File.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Status.h"
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/VirtualFileSystem.h"
 
 #include "lldb/lldb-types.h"
public:
   Status ResolveSymbolicLink(const FileSpec &src, FileSpec &dst);
 
   /// Wraps ::fopen in a platform-independent way.
   FILE *Fopen(const char *path, const char *mode);
 
   /// Wraps ::open in a platform-independent way.
   int Open(const char *path, int flags, int mode);
 
+  Status Open(File &File, const FileSpec &file_spec, uint32_t options,
+              uint32_t permissions = lldb::eFilePermissionsFileDefault);
+
   /// Returns the modification time of the given file.
   /// @{
   llvm::sys::TimePoint<> GetModificationTime(const FileSpec &file_spec) const;
   llvm::sys::TimePoint<> GetModificationTime(const llvm::Twine &path) const;
   /// @}
 
   /// Returns the on-disk size of the given file in bytes.
   /// @{

lldb/trunk/source/API/SBStream.cpp

 //===-- SBStream.cpp ----------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/API/SBStream.h"
 
 #include "lldb/Core/StreamFile.h"
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StreamString.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 SBStream::SBStream() : m_opaque_ap(new StreamString()), m_is_file(false) {}
if (!m_is_file)
       local_data = static_cast<StreamString *>(m_opaque_ap.get())->GetString();
   }
   StreamFile *stream_file = new StreamFile;
   uint32_t open_options = File::eOpenOptionWrite | File::eOpenOptionCanCreate;
   if (append)
     open_options |= File::eOpenOptionAppend;
   else
     open_options |= File::eOpenOptionTruncate;
-  stream_file->GetFile().Open(path, open_options,
-                              lldb::eFilePermissionsFileDefault);
 
+  FileSystem::Instance().Open(stream_file->GetFile(), FileSpec(path),
+                              open_options);
   m_opaque_ap.reset(stream_file);
 
   if (m_opaque_ap.get()) {
     m_is_file = true;
 
     // If we had any data locally in our StreamString, then pass that along to
     // the to new file we are redirecting to.
     if (!local_data.empty())

lldb/trunk/source/Commands/CommandObjectBugreport.cpp

if (thread) {
                  "image show-unwind --address 0x%" PRIx64, pc);
         commands.AppendString(command_buffer);
       }
     }
 
     const FileSpec &outfile_spec =
         m_outfile_options.GetFile().GetCurrentValue();
     if (outfile_spec) {
-      char path[PATH_MAX];
-      outfile_spec.GetPath(path, sizeof(path));
 
       uint32_t open_options =
           File::eOpenOptionWrite | File::eOpenOptionCanCreate |
           File::eOpenOptionAppend | File::eOpenOptionCloseOnExec;
 
       const bool append = m_outfile_options.GetAppend().GetCurrentValue();
       if (!append)
         open_options |= File::eOpenOptionTruncate;
 
       StreamFileSP outfile_stream = std::make_shared<StreamFile>();
-      Status error = outfile_stream->GetFile().Open(path, open_options);
+      File &file = outfile_stream->GetFile();
+      Status error =
+          FileSystem::Instance().Open(file, outfile_spec, open_options);
       if (error.Fail()) {
+        auto path = outfile_spec.GetPath();
         result.AppendErrorWithFormat("Failed to open file '%s' for %s: %s\n",
-                                     path, append ? "append" : "write",
+                                     path.c_str(), append ? "append" : "write",
                                      error.AsCString());
         result.SetStatus(eReturnStatusFailed);
         return false;
       }
 
       result.SetImmediateOutputStream(outfile_stream);
     }
 

lldb/trunk/source/Commands/CommandObjectMemory.cpp

#undef ALL_KEYWORDS
     m_prev_outfile_options = m_outfile_options;
     m_prev_varobj_options = m_varobj_options;
     m_prev_clang_ast_type = clang_ast_type;
 
     StreamFile outfile_stream;
     Stream *output_stream = nullptr;
     const FileSpec &outfile_spec =
         m_outfile_options.GetFile().GetCurrentValue();
+
+    std::string path = outfile_spec.GetPath();
     if (outfile_spec) {
-      char path[PATH_MAX];
-      outfile_spec.GetPath(path, sizeof(path));
 
       uint32_t open_options =
           File::eOpenOptionWrite | File::eOpenOptionCanCreate;
       const bool append = m_outfile_options.GetAppend().GetCurrentValue();
       if (append)
         open_options |= File::eOpenOptionAppend;
 
-      if (outfile_stream.GetFile().Open(path, open_options).Success()) {
+      Status error = FileSystem::Instance().Open(outfile_stream.GetFile(),
+                                                 outfile_spec, open_options);
+      if (error.Success()) {
         if (m_memory_options.m_output_as_binary) {
           const size_t bytes_written =
               outfile_stream.Write(data_sp->GetBytes(), bytes_read);
           if (bytes_written > 0) {
             result.GetOutputStream().Printf(
                 "%zi bytes %s to '%s'\n", bytes_written,
-                append ? "appended" : "written", path);
+                append ? "appended" : "written", path.c_str());
             return true;
           } else {
             result.AppendErrorWithFormat("Failed to write %" PRIu64
                                          " bytes to '%s'.\n",
-                                         (uint64_t)bytes_read, path);
+                                         (uint64_t)bytes_read, path.c_str());
             result.SetStatus(eReturnStatusFailed);
             return false;
           }
         } else {
           // We are going to write ASCII to the file just point the
           // output_stream to our outfile_stream...
           output_stream = &outfile_stream;
         }
       } else {
-        result.AppendErrorWithFormat("Failed to open file '%s' for %s.\n", path,
-                                     append ? "append" : "write");
+        result.AppendErrorWithFormat("Failed to open file '%s' for %s.\n",
+                                     path.c_str(), append ? "append" : "write");
         result.SetStatus(eReturnStatusFailed);
         return false;
       }
     } else {
       output_stream = &result.GetOutputStream();
     }
 
     ExecutionContextScope *exe_scope = m_exe_ctx.GetBestExecutionContextScope();

lldb/trunk/source/Core/StreamFile.cpp

 //===-- StreamFile.cpp ------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Core/StreamFile.h"
+#include "lldb/Host/FileSystem.h"
 
 #include <stdio.h>
 
 using namespace lldb;
 using namespace lldb_private;
 
 //----------------------------------------------------------------------
 // StreamFile constructor
 //----------------------------------------------------------------------
 StreamFile::StreamFile() : Stream(), m_file() {}
 
 StreamFile::StreamFile(uint32_t flags, uint32_t addr_size, ByteOrder byte_order)
     : Stream(flags, addr_size, byte_order), m_file() {}
 
 StreamFile::StreamFile(int fd, bool transfer_ownership)
     : Stream(), m_file(fd, transfer_ownership) {}
 
 StreamFile::StreamFile(FILE *fh, bool transfer_ownership)
     : Stream(), m_file(fh, transfer_ownership) {}
 
-StreamFile::StreamFile(const char *path)
-    : Stream(),
-      m_file(path, File::eOpenOptionWrite | File::eOpenOptionCanCreate |
-                       File::eOpenOptionCloseOnExec,
-             lldb::eFilePermissionsFileDefault) {}
+StreamFile::StreamFile(const char *path) : Stream(), m_file() {
+  FileSystem::Instance().Open(m_file, FileSpec(path),
+                              File::eOpenOptionWrite |
+                                  File::eOpenOptionCanCreate |
+                                  File::eOpenOptionCloseOnExec);
+}
 
 StreamFile::StreamFile(const char *path, uint32_t options, uint32_t permissions)
-    : Stream(), m_file(path, options, permissions) {}
+    : Stream(), m_file() {
+
+  FileSystem::Instance().Open(m_file, FileSpec(path), options, permissions);
+}
 
 StreamFile::~StreamFile() {}
 
 void StreamFile::Flush() { m_file.Flush(); }
 
 size_t StreamFile::WriteImpl(const void *s, size_t length) {
   m_file.Write(s, length);
   return length;
 }

lldb/trunk/source/Expression/REPL.cpp

if (code[0] == ':') {
 
         if (add_to_code) {
           const uint32_t new_default_line = m_code.GetSize() + 1;
 
           m_code.SplitIntoLines(code);
 
           // Update our code on disk
           if (!m_repl_source_path.empty()) {
-            lldb_private::File file(m_repl_source_path.c_str(),
+            lldb_private::File file;
+            FileSystem::Instance().Open(file, FileSpec(m_repl_source_path),
                                         File::eOpenOptionWrite |
                                             File::eOpenOptionTruncate |
                                             File::eOpenOptionCanCreate,
                                         lldb::eFilePermissionsFileDefault);
             std::string code(m_code.CopyList());
             code.append(1, '\n');
             size_t bytes_written = code.size();
             file.Write(code.c_str(), bytes_written);
             file.Close();
 
             // Now set the default file and line to the REPL source file
             m_target.GetSourceManager().SetDefaultFileAndLine(

lldb/trunk/source/Host/common/File.cpp

if (options & File::eOpenOptionAppend) {
     return "w";
   }
   return NULL;
 }
 
 int File::kInvalidDescriptor = -1;
 FILE *File::kInvalidStream = NULL;
 
-File::File(const char *path, uint32_t options, uint32_t permissions)
-    : IOObject(eFDTypeFile, false), m_descriptor(kInvalidDescriptor),
-      m_stream(kInvalidStream), m_options(), m_own_stream(false),
-      m_is_interactive(eLazyBoolCalculate),
-      m_is_real_terminal(eLazyBoolCalculate) {
-  Open(path, options, permissions);
-}
-
-File::File(const FileSpec &filespec, uint32_t options, uint32_t permissions)
-    : IOObject(eFDTypeFile, false), m_descriptor(kInvalidDescriptor),
-      m_stream(kInvalidStream), m_options(0), m_own_stream(false),
-      m_is_interactive(eLazyBoolCalculate),
-      m_is_real_terminal(eLazyBoolCalculate)
-
-{
-  if (filespec) {
-    Open(filespec.GetPath().c_str(), options, permissions);
-  }
-}
-
 File::~File() { Close(); }
 
 int File::GetDescriptor() const {
   if (DescriptorIsValid())
     return m_descriptor;
 
   // Don't open the file descriptor if we don't need to, just get it from the
   // stream if we have one.

 
 void File::SetStream(FILE *fh, bool transfer_ownership) {
   if (IsValid())
     Close();
   m_stream = fh;
   m_own_stream = transfer_ownership;
 }
 
-static int DoOpen(const char *path, int flags, int mode) {
-  return FileSystem::Instance().Open(path, flags, mode);
-}
-
-Status File::Open(const char *path, uint32_t options, uint32_t permissions) {
-  Status error;
-  if (IsValid())
-    Close();
-
-  int oflag = 0;
-  const bool read = options & eOpenOptionRead;
-  const bool write = options & eOpenOptionWrite;
-  if (write) {
-    if (read)
-      oflag |= O_RDWR;
-    else
-      oflag |= O_WRONLY;
-
-    if (options & eOpenOptionAppend)
-      oflag |= O_APPEND;
-
-    if (options & eOpenOptionTruncate)
-      oflag |= O_TRUNC;
-
-    if (options & eOpenOptionCanCreate)
-      oflag |= O_CREAT;
-
-    if (options & eOpenOptionCanCreateNewOnly)
-      oflag |= O_CREAT | O_EXCL;
-  } else if (read) {
-    oflag |= O_RDONLY;
-
-#ifndef _WIN32
-    if (options & eOpenOptionDontFollowSymlinks)
-      oflag |= O_NOFOLLOW;
-#endif
-  }
-
-#ifndef _WIN32
-  if (options & eOpenOptionNonBlocking)
-    oflag |= O_NONBLOCK;
-  if (options & eOpenOptionCloseOnExec)
-    oflag |= O_CLOEXEC;
-#else
-  oflag |= O_BINARY;
-#endif
-
-  mode_t mode = 0;
-  if (oflag & O_CREAT) {
-    if (permissions & lldb::eFilePermissionsUserRead)
-      mode |= S_IRUSR;
-    if (permissions & lldb::eFilePermissionsUserWrite)
-      mode |= S_IWUSR;
-    if (permissions & lldb::eFilePermissionsUserExecute)
-      mode |= S_IXUSR;
-    if (permissions & lldb::eFilePermissionsGroupRead)
-      mode |= S_IRGRP;
-    if (permissions & lldb::eFilePermissionsGroupWrite)
-      mode |= S_IWGRP;
-    if (permissions & lldb::eFilePermissionsGroupExecute)
-      mode |= S_IXGRP;
-    if (permissions & lldb::eFilePermissionsWorldRead)
-      mode |= S_IROTH;
-    if (permissions & lldb::eFilePermissionsWorldWrite)
-      mode |= S_IWOTH;
-    if (permissions & lldb::eFilePermissionsWorldExecute)
-      mode |= S_IXOTH;
-  }
-
-  m_descriptor = llvm::sys::RetryAfterSignal(-1, DoOpen, path, oflag, mode);
-  if (!DescriptorIsValid())
-    error.SetErrorToErrno();
-  else {
-    m_should_close_fd = true;
-    m_options = options;
-  }
-
-  return error;
-}
-
 uint32_t File::GetPermissions(Status &error) const {
   int fd = GetDescriptor();
   if (fd != kInvalidDescriptor) {
     struct stat file_stats;
     if (::fstat(fd, &file_stats) == -1)
       error.SetErrorToErrno();
     else {
       error.Clear();

lldb/trunk/source/Host/common/FileCache.cpp

 //===-- FileCache.cpp -------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Host/FileCache.h"
 
 #include "lldb/Host/File.h"
+#include "lldb/Host/FileSystem.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 FileCache *FileCache::m_instance = nullptr;
 
 FileCache &FileCache::GetInstance() {
   if (m_instance == nullptr)
     m_instance = new FileCache();
 
   return *m_instance;
 }
 
 lldb::user_id_t FileCache::OpenFile(const FileSpec &file_spec, uint32_t flags,
                                     uint32_t mode, Status &error) {
-  std::string path(file_spec.GetPath());
-  if (path.empty()) {
+  if (!file_spec) {
     error.SetErrorString("empty path");
     return UINT64_MAX;
   }
   FileSP file_sp(new File());
-  error = file_sp->Open(path.c_str(), flags, mode);
+  error = FileSystem::Instance().Open(*file_sp, file_spec, flags, mode);
   if (file_sp->IsValid() == false)
     return UINT64_MAX;
   lldb::user_id_t fd = file_sp->GetDescriptor();
   m_cache[fd] = file_sp;
   return fd;
 }
 
 bool FileCache::CloseFile(lldb::user_id_t fd, Status &error) {

lldb/trunk/source/Host/common/FileSystem.cpp

 //===-- FileSystem.cpp ------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Host/FileSystem.h"
 
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/TildeExpressionResolver.h"
 
+#include "llvm/Support/Errno.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Threading.h"
 
+#include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <stdarg.h>
+#include <stdio.h>
+
+#ifdef _WIN32
+#include "lldb/Host/windows/windows.h"
+#else
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <termios.h>
+#include <unistd.h>
+#endif
+
 #include <algorithm>
 #include <fstream>
 #include <vector>
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace llvm;
 
bool FileSystem::ResolveExecutableLocation(FileSpec &file_spec) {
   // Make sure that the result exists.
   FileSpec result(*error_or_path);
   if (!Exists(result))
     return false;
 
   file_spec = result;
   return true;
 }
+
+static int OpenWithFS(const FileSystem &fs, const char *path, int flags,
+                      int mode) {
+  return const_cast<FileSystem &>(fs).Open(path, flags, mode);
+}
+
+static int GetOpenFlags(uint32_t options) {
+  const bool read = options & File::eOpenOptionRead;
+  const bool write = options & File::eOpenOptionWrite;
+
+  int open_flags = 0;
+  if (write) {
+    if (read)
+      open_flags |= O_RDWR;
+    else
+      open_flags |= O_WRONLY;
+
+    if (options & File::eOpenOptionAppend)
+      open_flags |= O_APPEND;
+
+    if (options & File::eOpenOptionTruncate)
+      open_flags |= O_TRUNC;
+
+    if (options & File::eOpenOptionCanCreate)
+      open_flags |= O_CREAT;
+
+    if (options & File::eOpenOptionCanCreateNewOnly)
+      open_flags |= O_CREAT | O_EXCL;
+  } else if (read) {
+    open_flags |= O_RDONLY;
+
+#ifndef _WIN32
+    if (options & File::eOpenOptionDontFollowSymlinks)
+      open_flags |= O_NOFOLLOW;
+#endif
+  }
+
+#ifndef _WIN32
+  if (options & File::eOpenOptionNonBlocking)
+    open_flags |= O_NONBLOCK;
+  if (options & File::eOpenOptionCloseOnExec)
+    open_flags |= O_CLOEXEC;
+#else
+  open_flags |= O_BINARY;
+#endif
+
+  return open_flags;
+}
+
+static mode_t GetOpenMode(uint32_t permissions) {
+  mode_t mode = 0;
+  if (permissions & lldb::eFilePermissionsUserRead)
+    mode |= S_IRUSR;
+  if (permissions & lldb::eFilePermissionsUserWrite)
+    mode |= S_IWUSR;
+  if (permissions & lldb::eFilePermissionsUserExecute)
+    mode |= S_IXUSR;
+  if (permissions & lldb::eFilePermissionsGroupRead)
+    mode |= S_IRGRP;
+  if (permissions & lldb::eFilePermissionsGroupWrite)
+    mode |= S_IWGRP;
+  if (permissions & lldb::eFilePermissionsGroupExecute)
+    mode |= S_IXGRP;
+  if (permissions & lldb::eFilePermissionsWorldRead)
+    mode |= S_IROTH;
+  if (permissions & lldb::eFilePermissionsWorldWrite)
+    mode |= S_IWOTH;
+  if (permissions & lldb::eFilePermissionsWorldExecute)
+    mode |= S_IXOTH;
+  return mode;
+}
+
+Status FileSystem::Open(File &File, const FileSpec &file_spec, uint32_t options,
+                        uint32_t permissions) {
+  if (File.IsValid())
+    File.Close();
+
+  const int open_flags = GetOpenFlags(options);
+  const mode_t open_mode =
+      (open_flags & O_CREAT) ? GetOpenMode(permissions) : 0;
+  const std::string path = file_spec.GetPath();
+
+  int descriptor = llvm::sys::RetryAfterSignal(
+      -1, OpenWithFS, *this, path.c_str(), open_flags, open_mode);
+
+  Status error;
+  if (!File::DescriptorIsValid(descriptor)) {
+    File.SetDescriptor(descriptor, false);
+    error.SetErrorToErrno();
+  } else {
+    File.SetDescriptor(descriptor, true);
+    File.SetOptions(options);
+  }
+  return error;
+}

lldb/trunk/source/Interpreter/CommandInterpreter.cpp

#endif
   }
 
   cmd_obj_sp = GetCommandSPExact("frame variable", false);
   if (cmd_obj_sp) {
     AddAlias("v", cmd_obj_sp);
     AddAlias("var", cmd_obj_sp);
     AddAlias("vo", cmd_obj_sp, "--object-description");
   }
-  
 }
 
 void CommandInterpreter::Clear() {
   m_command_io_handler_sp.reset();
 
   if (m_script_interpreter_sp)
     m_script_interpreter_sp->Clear();
 }

 
 void CommandInterpreter::HandleCommandsFromFile(
     FileSpec &cmd_file, ExecutionContext *context,
     CommandInterpreterRunOptions &options, CommandReturnObject &result) {
   if (FileSystem::Instance().Exists(cmd_file)) {
     StreamFileSP input_file_sp(new StreamFile());
 
     std::string cmd_file_path = cmd_file.GetPath();
-    Status error = input_file_sp->GetFile().Open(cmd_file_path.c_str(),
-                                                 File::eOpenOptionRead);
-
+    Status error = FileSystem::Instance().Open(input_file_sp->GetFile(),
+                                               cmd_file, File::eOpenOptionRead);
     if (error.Success()) {
       Debugger &debugger = GetDebugger();
 
       uint32_t flags = 0;
 
       if (options.m_stop_on_continue == eLazyBoolCalculate) {
         if (m_command_source_flags.empty()) {
           // Stop on continue by default

lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp

assert(alloc->data_ptr.isValid() && alloc->element.type.isValid() &&
          alloc->element.type_vec_size.isValid() &&
          alloc->element.datum_size.get() &&
          alloc->element.type_kind.isValid() && alloc->dimension.isValid() &&
          "Allocation information not available");
 
   // Check we can create writable file
   FileSpec file_spec(path);
   FileSystem::Instance().Resolve(file_spec);
-  File file(file_spec, File::eOpenOptionWrite | File::eOpenOptionCanCreate |
+  File file;
+  FileSystem::Instance().Open(file, file_spec,
+                              File::eOpenOptionWrite |
+                                  File::eOpenOptionCanCreate |
                                   File::eOpenOptionTruncate);
+
   if (!file) {
     strm.Printf("Error: Failed to open '%s' for writing", path);
     strm.EOL();
     return false;
   }
 
   // Read allocation into buffer of heap memory
   const std::shared_ptr<uint8_t> buffer = GetAllocationData(alloc, frame_ptr);
bool DoExecute(Args &command, CommandReturnObject &result) override {
     }
 
     Stream *output_strm = nullptr;
     StreamFile outfile_stream;
     const FileSpec &outfile_spec =
         m_options.m_outfile; // Dump allocation to file instead
     if (outfile_spec) {
       // Open output file
-      char path[256];
-      outfile_spec.GetPath(path, sizeof(path));
-      if (outfile_stream.GetFile()
-              .Open(path, File::eOpenOptionWrite | File::eOpenOptionCanCreate)
-              .Success()) {
+      std::string path = outfile_spec.GetPath();
+      auto error = FileSystem::Instance().Open(
+          outfile_stream.GetFile(), outfile_spec,
+          File::eOpenOptionWrite | File::eOpenOptionCanCreate);
+      if (error.Success()) {
         output_strm = &outfile_stream;
-        result.GetOutputStream().Printf("Results written to '%s'", path);
+        result.GetOutputStream().Printf("Results written to '%s'",
+                                        path.c_str());
         result.GetOutputStream().EOL();
       } else {
-        result.AppendErrorWithFormat("Couldn't open file '%s'", path);
+        result.AppendErrorWithFormat("Couldn't open file '%s'", path.c_str());
         result.SetStatus(eReturnStatusFailed);
         return false;
       }
     } else
       output_strm = &result.GetOutputStream();
 
     assert(output_strm != nullptr);
     bool dumped =

lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

if (target_triple.getVendor() == llvm::Triple::Apple &&
             buffer.PutHex32(segment.maxprot);
             buffer.PutHex32(segment.initprot);
             buffer.PutHex32(segment.nsects);
             buffer.PutHex32(segment.flags);
           }
 
           File core_file;
           std::string core_file_path(outfile.GetPath());
-          error = core_file.Open(core_file_path.c_str(),
+          error = FileSystem::Instance().Open(core_file, outfile,
                                               File::eOpenOptionWrite |
                                                   File::eOpenOptionTruncate |
                                                   File::eOpenOptionCanCreate);
           if (error.Success()) {
             // Read 1 page at a time
             uint8_t bytes[0x1000];
             // Write the mach header and load commands out to the core file
             size_t bytes_written = buffer.GetString().size();
             error = core_file.Write(buffer.GetString().data(), bytes_written);
             if (error.Success()) {
               // Now write the file data for all memory segments in the process

lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

 // Provide a function that can easily dump the packet history if we know a
 // ProcessGDBRemote * value (which we can get from logs or from debugging). We
 // need the function in the lldb namespace so it makes it into the final
 // executable since the LLDB shared library only exports stuff in the lldb
 // namespace. This allows you to attach with a debugger and call this function
 // and get the packet history dumped to a file.
 void DumpProcessGDBRemotePacketHistory(void *p, const char *path) {
   StreamFile strm;
-  Status error(strm.GetFile().Open(path, File::eOpenOptionWrite |
-                                             File::eOpenOptionCanCreate));
+  Status error = FileSystem::Instance().Open(strm.GetFile(), FileSpec(path),
+                                             File::eOpenOptionWrite |
+                                                 File::eOpenOptionCanCreate);
   if (error.Success())
     ((ProcessGDBRemote *)p)->GetGDBRemote().DumpHistory(strm);
 }
 }
 
 namespace {
 
 static constexpr PropertyDefinition g_properties[] = {

lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

return PythonObject(PyRefType::Owned,
                       PyObject_CallObject(m_py_obj, arg_tuple.get()));
 }
 
 PythonFile::PythonFile() : PythonObject() {}
 
 PythonFile::PythonFile(File &file, const char *mode) { Reset(file, mode); }
 
 PythonFile::PythonFile(const char *path, const char *mode) {
-  lldb_private::File file(path, GetOptionsFromMode(mode));
+  lldb_private::File file;
+  FileSystem::Instance().Open(file, FileSpec(path), GetOptionsFromMode(mode));
   Reset(file, mode);
 }
 
 PythonFile::PythonFile(PyRefType type, PyObject *o) { Reset(type, o); }
 
 PythonFile::~PythonFile() {}
 
 bool PythonFile::Check(PyObject *py_obj) {

lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

#endif
           }
         }
       }
       if (!input_file_sp || !output_file_sp || !error_file_sp)
         debugger.AdoptTopIOHandlerFilesIfInvalid(input_file_sp, output_file_sp,
                                                  error_file_sp);
     } else {
       input_file_sp.reset(new StreamFile());
-      input_file_sp->GetFile().Open(FileSystem::DEV_NULL,
-                                    File::eOpenOptionRead);
+      FileSystem::Instance().Open(input_file_sp->GetFile(),
+                                  FileSpec(FileSystem::DEV_NULL),
+
       output_file_sp.reset(new StreamFile());
-      output_file_sp->GetFile().Open(FileSystem::DEV_NULL,
-                                     File::eOpenOptionWrite);
+      FileSystem::Instance().Open(output_file_sp->GetFile(),
+                                  FileSpec(FileSystem::DEV_NULL),
+
       error_file_sp = output_file_sp;
     }
 
     FILE *in_file = input_file_sp->GetFile().GetStream();
     FILE *out_file = output_file_sp->GetFile().GetStream();
     FILE *err_file = error_file_sp->GetFile().GetStream();
     bool success = false;
     {

lldb/trunk/source/Target/ModuleCache.cpp

 ModuleLock::ModuleLock(const FileSpec &root_dir_spec, const UUID &uuid,
                        Status &error) {
   const auto lock_dir_spec = JoinPath(root_dir_spec, kLockDirName);
   error = MakeDirectory(lock_dir_spec);
   if (error.Fail())
     return;
 
   m_file_spec = JoinPath(lock_dir_spec, uuid.GetAsString().c_str());
-  m_file.Open(m_file_spec.GetCString(), File::eOpenOptionWrite |
-                                            File::eOpenOptionCanCreate |
-                                            File::eOpenOptionCloseOnExec);
+  FileSystem::Instance().Open(m_file, m_file_spec,
+                              File::eOpenOptionWrite |
   if (!m_file) {
     error.SetErrorToErrno();
     return;
   }
 
   m_lock.reset(new lldb_private::LockFile(m_file.GetDescriptor()));
   error = m_lock->WriteLock(0, 1);
   if (error.Fail())

lldb/trunk/source/Target/Platform.cpp

if (log)
     log->Printf("[PutFile] Using block by block transfer....\n");
 
   uint32_t source_open_options =
       File::eOpenOptionRead | File::eOpenOptionCloseOnExec;
   namespace fs = llvm::sys::fs;
   if (fs::is_symlink_file(source.GetPath()))
     source_open_options |= File::eOpenOptionDontFollowSymlinks;
 
-  File source_file(source, source_open_options, lldb::eFilePermissionsUserRW);
-  Status error;
+  File source_file;
+  Status error = FileSystem::Instance().Open(
+      source_file, source, source_open_options, lldb::eFilePermissionsUserRW);
   uint32_t permissions = source_file.GetPermissions(error);
   if (permissions == 0)
     permissions = lldb::eFilePermissionsFileDefault;
 
   if (!source_file.IsValid())
     return Status("PutFile: unable to open source file");
   lldb::user_id_t dest_file = OpenFile(
       destination, File::eOpenOptionCanCreate | File::eOpenOptionWrite |

lldb/trunk/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

TEST_F(PythonDataObjectsTest, TestDictionaryResolutionWithDot) {
   PythonInteger version_minor =
       PythonObject::ResolveNameWithDictionary("sys.version_info.minor", dict)
           .AsType<PythonInteger>();
   EXPECT_EQ(PY_MAJOR_VERSION, version_major.GetInteger());
   EXPECT_EQ(PY_MINOR_VERSION, version_minor.GetInteger());
 }
 
 TEST_F(PythonDataObjectsTest, TestPythonInteger) {
   // Test that integers behave correctly when wrapped by a PythonInteger.
 
 #if PY_MAJOR_VERSION < 3
   // Verify that `PythonInt` works correctly when given a PyInt object.
   // Note that PyInt doesn't exist in Python 3.x, so this is only for 2.x
   PyObject *py_int = PyInt_FromLong(12);
   EXPECT_TRUE(PythonInteger::Check(py_int));
   PythonInteger python_int(PyRefType::Owned, py_int);
 
TEST_F(PythonDataObjectsTest, TestPythonCallableInvoke) {
   auto list_result = result.AsType<PythonList>();
   EXPECT_EQ(3U, list_result.GetSize());
   EXPECT_EQ(one.get(), list_result.GetItemAtIndex(0).get());
   EXPECT_EQ(two.get(), list_result.GetItemAtIndex(1).get());
   EXPECT_EQ(three.get(), list_result.GetItemAtIndex(2).get());
 }
 
 TEST_F(PythonDataObjectsTest, TestPythonFile) {
-  File file(FileSystem::DEV_NULL, File::eOpenOptionRead);
+  File file;
+  FileSystem::Instance().Open(file, FileSpec(FileSystem::DEV_NULL),
+                              File::eOpenOptionRead);
   PythonFile py_file(file, "r");
   EXPECT_TRUE(PythonFile::Check(py_file.get()));
 }
 
 TEST_F(PythonDataObjectsTest, TestObjectAttributes) {
   PythonInteger py_int(42);
   EXPECT_TRUE(py_int.HasAttribute("numerator"));
   EXPECT_FALSE(py_int.HasAttribute("this_should_not_exist"));