uint32_t options -> File::OpenOptions options

[lawrence_danna]

Summary

This patch re-types everywhere that passes a File::OpenOptions
as a uint32_t so it actually uses File::OpenOptions.

It also converts some OpenOptions related functions that fail
by returning 0 or NULL into llvm::Expected

split off from https://reviews.llvm.org/D68737

[labath]

I originally thought you would do something local, but changing this across the board is definitely better, and it doesn't seem the patch is that big. LLVM already has a utility to work out the kinks in the bitmask enum use case. Please take a look at llvm/ADT/BitmaskEnum.h on how to make use of that.

lldb/include/lldb/Host/File.h

59

A better way to handle that is to make this a typed enum (enum OpenOptions : uint16_t). Then you can add something like LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/0xffffu) to make sure the bitmask operators also work well.

lldb/source/Host/common/File.cpp

42

I'd consider leaving the static function as const char * -- the function doesn't do anything complex, and there's only one way it can fail (which can be signalled by a nullptr), so having the Expected wrapper around that does not seem all that useful -- I'll leave it up to you though.

74

For similar reasons, I'd make this an Optional<OpenOptions>.

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

1093–1098

If GetOptionsFromMode stays as Expected then this is not correct, as it does not handle the error case. Overall, I'd say we shouldn't use auto here..

[lawrence_danna]

review fixes

[lawrence_danna]

lldb/source/Host/common/File.cpp

42

If I do that I'd still just have to convert it to Error in the callers, so I think I'll leave it this way.

[lawrence_danna]

fix

[labath]

lldb/source/Host/common/File.cpp

42

Yes, but OTOH, if you did that, there would also be callers where you wouldn't need the consumeError(.takeError()) stuff. Anyway, I think this is fine too...

lldb/include/lldb/Core/StreamFile.h

public:
   StreamFile();
 
   StreamFile(uint32_t flags, uint32_t addr_size, lldb::ByteOrder byte_order);
 
   StreamFile(int fd, bool transfer_ownership);
 
   StreamFile(const char *path);
 
-  StreamFile(const char *path, uint32_t options,
+  StreamFile(const char *path, File::OpenOptions options,
              uint32_t permissions = lldb::eFilePermissionsFileDefault);
 
   StreamFile(FILE *fh, bool transfer_ownership);
 
   StreamFile(std::shared_ptr<File> file) : m_file_sp(file) { assert(file); };
 
   ~StreamFile() override;
 

lldb/include/lldb/Host/File.h

 //===-- File.h --------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_File_h_
 #define liblldb_File_h_
 
 #include "lldb/Host/PosixApi.h"
 #include "lldb/Utility/IOObject.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-private.h"
+#include "llvm/ADT/BitmaskEnum.h"
 
 #include <mutex>
 #include <stdarg.h>
 #include <stdio.h>
 #include <sys/types.h>
 
 namespace lldb_private {
 
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+
 /// \class File File.h "lldb/Host/File.h"
 /// An abstract base class for files.
 ///
 /// Files will often be NativeFiles, which provides a wrapper
 /// around host OS file functionality.   But it
 /// is also possible to subclass file to provide objects that have file
 /// or stream functionality but are not backed by any host OS file.
 class File : public IOObject {
 public:
   static int kInvalidDescriptor;
   static FILE *kInvalidStream;
 
   // NB this enum is used in the lldb platform gdb-remote packet
   // vFile:open: and existing values cannot be modified.
-  enum OpenOptions {
+  //
+  // FIXME
+  // These values do not match the values used by GDB
+  // * https://sourceware.org/gdb/onlinedocs/gdb/Open-Flags.html#Open-Flags
+  // * rdar://problem/46788934
+  enum OpenOptions : uint32_t {
     eOpenOptionRead = (1u << 0),  // Open file for reading
     eOpenOptionWrite = (1u << 1), // Open file for writing
     eOpenOptionAppend =
         (1u << 2), // Don't truncate file when opening, append to end of file
     eOpenOptionTruncate = (1u << 3),    // Truncate file when opening
     eOpenOptionNonBlocking = (1u << 4), // File reads
     eOpenOptionCanCreate = (1u << 5),   // Create file if doesn't already exist
     eOpenOptionCanCreateNewOnly =
         (1u << 6), // Can create file only if it doesn't already exist
     eOpenOptionDontFollowSymlinks = (1u << 7),
     eOpenOptionCloseOnExec =
-        (1u << 8) // Close the file when executing a new process
+        (1u << 8), // Close the file when executing a new process
+    LLVM_MARK_AS_BITMASK_ENUM(/* largest_value= */ eOpenOptionCloseOnExec)
   };
 
-  static mode_t ConvertOpenOptionsForPOSIXOpen(uint32_t open_options);
-  static uint32_t GetOptionsFromMode(llvm::StringRef mode);
+  static mode_t ConvertOpenOptionsForPOSIXOpen(OpenOptions open_options);
+  static llvm::Expected<OpenOptions> GetOptionsFromMode(llvm::StringRef mode);
   static bool DescriptorIsValid(int descriptor) { return descriptor >= 0; };
 
   File()
       : IOObject(eFDTypeFile), m_is_interactive(eLazyBoolCalculate),
         m_is_real_terminal(eLazyBoolCalculate),
         m_supports_colors(eLazyBoolCalculate){};
 
   /// Read bytes from a file from the current file position into buf.

 private:
   DISALLOW_COPY_AND_ASSIGN(File);
 };
 
 class NativeFile : public File {
 public:
   NativeFile()
       : m_descriptor(kInvalidDescriptor), m_own_descriptor(false),
-        m_stream(kInvalidStream), m_options(0), m_own_stream(false) {}
+        m_stream(kInvalidStream), m_options(), m_own_stream(false) {}
 
   NativeFile(FILE *fh, bool transfer_ownership)
       : m_descriptor(kInvalidDescriptor), m_own_descriptor(false), m_stream(fh),
-        m_options(0), m_own_stream(transfer_ownership) {}
+        m_options(), m_own_stream(transfer_ownership) {}
 
-  NativeFile(int fd, uint32_t options, bool transfer_ownership)
+  NativeFile(int fd, OpenOptions options, bool transfer_ownership)
       : m_descriptor(fd), m_own_descriptor(transfer_ownership),
         m_stream(kInvalidStream), m_options(options), m_own_stream(false) {}
 
   ~NativeFile() override { Close(); }
 
   bool IsValid() const override {
     return DescriptorIsValid() || StreamIsValid();
   }
bool DescriptorIsValid() const {
     return File::DescriptorIsValid(m_descriptor);
   }
   bool StreamIsValid() const { return m_stream != kInvalidStream; }
 
   // Member variables
   int m_descriptor;
   bool m_own_descriptor;
   FILE *m_stream;
-  uint32_t m_options;
+  OpenOptions m_options;
   bool m_own_stream;
   std::mutex offset_access_mutex;
 
 private:
   DISALLOW_COPY_AND_ASSIGN(NativeFile);
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_File_h_

lldb/include/lldb/Host/FileCache.h

 //===-- FileCache.h ---------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #ifndef liblldb_Host_FileCache_h
 #define liblldb_Host_FileCache_h
 
 #include <map>
 #include <stdint.h>
 
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 
+#include "lldb/Host/File.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Status.h"
 
 namespace lldb_private {
 class FileCache {
 private:
   FileCache() {}
 
   typedef std::map<lldb::user_id_t, lldb::FileUP> FDToFileMap;
 
 public:
   static FileCache &GetInstance();
 
-  lldb::user_id_t OpenFile(const FileSpec &file_spec, uint32_t flags,
+  lldb::user_id_t OpenFile(const FileSpec &file_spec, File::OpenOptions flags,
                            uint32_t mode, Status &error);
   bool CloseFile(lldb::user_id_t fd, Status &error);
 
   uint64_t WriteFile(lldb::user_id_t fd, uint64_t offset, const void *src,
                      uint64_t src_len, Status &error);
   uint64_t ReadFile(lldb::user_id_t fd, uint64_t offset, void *dst,
                     uint64_t dst_len, Status &error);
 
 private:
   static FileCache *m_instance;
 
   FDToFileMap m_cache;
 };
 }
 
 #endif

lldb/include/lldb/Host/FileSystem.h

public:
 
   /// 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);
 
   llvm::Expected<std::unique_ptr<File>>
-  Open(const FileSpec &file_spec, uint32_t options,
+  Open(const FileSpec &file_spec, File::OpenOptions options,
        uint32_t permissions = lldb::eFilePermissionsFileDefault,
        bool should_close_fd = true);
 
   /// Get a directory iterator.
   /// \{
   llvm::vfs::directory_iterator DirBegin(const FileSpec &file_spec,
                                          std::error_code &ec);
   llvm::vfs::directory_iterator DirBegin(const llvm::Twine &dir,

lldb/include/lldb/Target/Platform.h

 #include <map>
 #include <memory>
 #include <mutex>
 #include <string>
 #include <vector>
 
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/Core/UserSettingsController.h"
+#include "lldb/Host/File.h"
 #include "lldb/Interpreter/Options.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Timeout.h"
 #include "lldb/Utility/UserIDResolver.h"
 #include "lldb/lldb-private-forward.h"
 #include "lldb/lldb-public.h"
public:
   virtual Status MakeDirectory(const FileSpec &file_spec, uint32_t permissions);
 
   virtual Status GetFilePermissions(const FileSpec &file_spec,
                                     uint32_t &file_permissions);
 
   virtual Status SetFilePermissions(const FileSpec &file_spec,
                                     uint32_t file_permissions);
 
-  virtual lldb::user_id_t OpenFile(const FileSpec &file_spec, uint32_t flags,
-                                   uint32_t mode, Status &error) {
+  virtual lldb::user_id_t OpenFile(const FileSpec &file_spec,
+                                   File::OpenOptions flags, uint32_t mode,
+                                   Status &error) {
     return UINT64_MAX;
   }
 
   virtual bool CloseFile(lldb::user_id_t fd, Status &error) { return false; }
 
   virtual lldb::user_id_t GetFileSize(const FileSpec &file_spec) {
     return UINT64_MAX;
   }

lldb/include/lldb/Target/RemoteAwarePlatform.h

 /// operations to a remote platform instance (such as PlatformRemoteGDBServer).
 class RemoteAwarePlatform : public Platform {
 public:
   using Platform::Platform;
 
   bool GetModuleSpec(const FileSpec &module_file_spec, const ArchSpec &arch,
                      ModuleSpec &module_spec) override;
 
-  lldb::user_id_t OpenFile(const FileSpec &file_spec, uint32_t flags,
+  lldb::user_id_t OpenFile(const FileSpec &file_spec, File::OpenOptions flags,
                            uint32_t mode, Status &error) override;
 
   bool CloseFile(lldb::user_id_t fd, Status &error) override;
 
   uint64_t ReadFile(lldb::user_id_t fd, uint64_t offset, void *dst,
                     uint64_t dst_len, Status &error) override;
 
   uint64_t WriteFile(lldb::user_id_t fd, uint64_t offset, const void *src,

lldb/source/API/SBFile.cpp

SBFile::SBFile(FILE *file, bool transfer_ownership) {
   LLDB_RECORD_DUMMY(void, SBFile, (FILE *, bool), file, transfer_ownership);
   m_opaque_sp = std::make_shared<NativeFile>(file, transfer_ownership);
 }
 
 SBFile::SBFile(int fd, const char *mode, bool transfer_owndership) {
   LLDB_RECORD_DUMMY(void, SBFile, (int, const char *, bool), fd, mode,
                     transfer_owndership);
   auto options = File::GetOptionsFromMode(mode);
-  m_opaque_sp = std::make_shared<NativeFile>(fd, options, transfer_owndership);
+  if (!options) {
+    llvm::consumeError(options.takeError());
+    return;
+  }
+  m_opaque_sp =
+      std::make_shared<NativeFile>(fd, options.get(), transfer_owndership);
 }
 
 SBError SBFile::Read(uint8_t *buf, size_t num_bytes, size_t *bytes_read) {
   LLDB_RECORD_DUMMY(lldb::SBError, SBFile, Read, (uint8_t *, size_t, size_t *),
                     buf, num_bytes, bytes_read);
   SBError error;
   if (!m_opaque_sp) {
     error.SetErrorString("invalid SBFile");

lldb/source/API/SBStream.cpp

void SBStream::RedirectToFile(const char *path, bool append) {
 
   std::string local_data;
   if (m_opaque_up) {
     // See if we have any locally backed data. If so, copy it so we can then
     // redirect it to the file so we don't lose the data
     if (!m_is_file)
       local_data = static_cast<StreamString *>(m_opaque_up.get())->GetString();
   }
-  uint32_t open_options = File::eOpenOptionWrite | File::eOpenOptionCanCreate;
+  auto open_options = File::eOpenOptionWrite | File::eOpenOptionCanCreate;
   if (append)
     open_options |= File::eOpenOptionAppend;
   else
     open_options |= File::eOpenOptionTruncate;
 
   llvm::Expected<FileUP> file =
       FileSystem::Instance().Open(FileSpec(path), open_options);
   if (!file) {

lldb/source/Commands/CommandObjectMemory.cpp

#undef ALL_KEYWORDS
     std::unique_ptr<Stream> output_stream_storage;
     Stream *output_stream_p = nullptr;
     const FileSpec &outfile_spec =
         m_outfile_options.GetFile().GetCurrentValue();
 
     std::string path = outfile_spec.GetPath();
     if (outfile_spec) {
 
-      uint32_t open_options =
-          File::eOpenOptionWrite | File::eOpenOptionCanCreate;
+      auto open_options = File::eOpenOptionWrite | File::eOpenOptionCanCreate;
       const bool append = m_outfile_options.GetAppend().GetCurrentValue();
       if (append)
         open_options |= File::eOpenOptionAppend;
 
       auto outfile = FileSystem::Instance().Open(outfile_spec, open_options);
 
       if (outfile) {
         auto outfile_stream_up =

lldb/source/Commands/CommandObjectSettings.cpp

public:
     bool m_append = false;
   };
 
 protected:
   bool DoExecute(Args &args, CommandReturnObject &result) override {
     FileSpec file_spec(m_options.m_filename);
     FileSystem::Instance().Resolve(file_spec);
     std::string path(file_spec.GetPath());
-    uint32_t options = File::OpenOptions::eOpenOptionWrite |
-                       File::OpenOptions::eOpenOptionCanCreate;
+    auto options = File::eOpenOptionWrite | File::eOpenOptionCanCreate;
     if (m_options.m_append)
-      options |= File::OpenOptions::eOpenOptionAppend;
+      options |= File::eOpenOptionAppend;
     else
-      options |= File::OpenOptions::eOpenOptionTruncate;
+      options |= File::eOpenOptionTruncate;
 
     StreamFile out_file(path.c_str(), options,
                         lldb::eFilePermissionsFileDefault);
 
     if (!out_file.GetFile().IsValid()) {
       result.AppendErrorWithFormat("%s: unable to write to file", path.c_str());
       result.SetStatus(eReturnStatusFailed);
       return false;

lldb/source/Core/StreamFile.cpp

StreamFile::StreamFile(const char *path) : Stream() {
   else {
     // TODO refactor this so the error gets popagated up instead of logged here.
     LLDB_LOG_ERROR(GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST), file.takeError(),
                    "Cannot open {1}: {0}", path);
     m_file_sp = std::make_shared<File>();
   }
 }
 
-StreamFile::StreamFile(const char *path, uint32_t options, uint32_t permissions)
+StreamFile::StreamFile(const char *path, File::OpenOptions options,
+                       uint32_t permissions)
     : Stream() {
   auto file = FileSystem::Instance().Open(FileSpec(path), options, permissions);
   if (file)
     m_file_sp = std::move(file.get());
   else {
     // TODO refactor this so the error gets popagated up instead of logged here.
     LLDB_LOG_ERROR(GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST), file.takeError(),
                    "Cannot open {1}: {0}", path);

lldb/source/Host/common/File.cpp

 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 
 using namespace lldb;
 using namespace lldb_private;
+using llvm::Expected;
 
-static const char *GetStreamOpenModeFromOptions(uint32_t options) {
+static Expected<const char *> GetStreamOpenModeFromOptions(uint32_t options) {
   if (options & File::eOpenOptionAppend) {
     if (options & File::eOpenOptionRead) {
       if (options & File::eOpenOptionCanCreateNewOnly)
         return "a+x";
       else
         return "a+";
     } else if (options & File::eOpenOptionWrite) {
       if (options & File::eOpenOptionCanCreateNewOnly)
if (options & File::eOpenOptionCanCreate) {
         return "w+";
     } else
       return "r+";
   } else if (options & File::eOpenOptionRead) {
     return "r";
   } else if (options & File::eOpenOptionWrite) {
     return "w";
   }
-  return nullptr;
+  return llvm::createStringError(
+      llvm::inconvertibleErrorCode(),
+      "invalid options, cannot convert to mode string");
 }
 
-uint32_t File::GetOptionsFromMode(llvm::StringRef mode) {
-  return llvm::StringSwitch<uint32_t>(mode)
+Expected<File::OpenOptions> File::GetOptionsFromMode(llvm::StringRef mode) {
+  OpenOptions opts =
+      llvm::StringSwitch<OpenOptions>(mode)
           .Cases("r", "rb", eOpenOptionRead)
           .Cases("w", "wb", eOpenOptionWrite)
           .Cases("a", "ab",
                  eOpenOptionWrite | eOpenOptionAppend | eOpenOptionCanCreate)
           .Cases("r+", "rb+", "r+b", eOpenOptionRead | eOpenOptionWrite)
           .Cases("w+", "wb+", "w+b",
                  eOpenOptionRead | eOpenOptionWrite | eOpenOptionCanCreate |
                      eOpenOptionTruncate)
           .Cases("a+", "ab+", "a+b",
                  eOpenOptionRead | eOpenOptionWrite | eOpenOptionAppend |
                      eOpenOptionCanCreate)
-      .Default(0);
+          .Default(OpenOptions());
+  if (opts)
+    return opts;
+  return llvm::createStringError(
+      llvm::inconvertibleErrorCode(),
+      "invalid mode, cannot convert to File::OpenOptions");
 }
 
 int File::kInvalidDescriptor = -1;
 FILE *File::kInvalidStream = nullptr;
 
 Status File::Read(void *buf, size_t &num_bytes) {
   return std::error_code(ENOTSUP, std::system_category());
 }

 
 IOObject::WaitableHandle NativeFile::GetWaitableHandle() {
   return GetDescriptor();
 }
 
 FILE *NativeFile::GetStream() {
   if (!StreamIsValid()) {
     if (DescriptorIsValid()) {
-      const char *mode = GetStreamOpenModeFromOptions(m_options);
-      if (mode) {
+      auto mode = GetStreamOpenModeFromOptions(m_options);
+      if (!mode)
+        llvm::consumeError(mode.takeError());
+      else {
         if (!m_own_descriptor) {
 // We must duplicate the file descriptor if we don't own it because when you
 // call fdopen, the stream will own the fd
 #ifdef _WIN32
           m_descriptor = ::_dup(GetDescriptor());
 #else
           m_descriptor = dup(GetDescriptor());
 #endif
           m_own_descriptor = true;
         }
 
-        m_stream =
-            llvm::sys::RetryAfterSignal(nullptr, ::fdopen, m_descriptor, mode);
+        m_stream = llvm::sys::RetryAfterSignal(nullptr, ::fdopen, m_descriptor,
+                                               mode.get());
 
         // If we got a stream, then we own the stream and should no longer own
         // the descriptor because fclose() will close it for us
 
         if (m_stream) {
           m_own_stream = true;
           m_own_descriptor = false;
         }
if (StreamIsValid()) {
     }
   }
   if (DescriptorIsValid() && m_own_descriptor) {
     if (::close(m_descriptor) != 0)
       error.SetErrorToErrno();
   }
   m_descriptor = kInvalidDescriptor;
   m_stream = kInvalidStream;
-  m_options = 0;
+  m_options = OpenOptions(0);
   m_own_stream = false;
   m_own_descriptor = false;
   m_is_interactive = eLazyBoolCalculate;
   m_is_real_terminal = eLazyBoolCalculate;
   return error;
 }
 
 FILE *NativeFile::TakeStreamAndClear() {
   FILE *stream = GetStream();
   m_stream = NULL;
   m_descriptor = kInvalidDescriptor;
-  m_options = 0;
+  m_options = OpenOptions();
   m_own_stream = false;
   m_own_descriptor = false;
   m_is_interactive = m_supports_colors = m_is_real_terminal =
       eLazyBoolCalculate;
   return stream;
 }
 
 Status NativeFile::GetFileSpec(FileSpec &file_spec) const {

 size_t NativeFile::PrintfVarArg(const char *format, va_list args) {
   if (StreamIsValid()) {
     return ::vfprintf(m_stream, format, args);
   } else {
     return File::PrintfVarArg(format, args);
   }
 }
 
-mode_t File::ConvertOpenOptionsForPOSIXOpen(uint32_t open_options) {
+mode_t File::ConvertOpenOptionsForPOSIXOpen(OpenOptions open_options) {
   mode_t mode = 0;
   if (open_options & eOpenOptionRead && open_options & eOpenOptionWrite)
     mode |= O_RDWR;
   else if (open_options & eOpenOptionWrite)
     mode |= O_WRONLY;
 
   if (open_options & eOpenOptionAppend)
     mode |= O_APPEND;
 
   if (open_options & eOpenOptionTruncate)
     mode |= O_TRUNC;
 
   if (open_options & eOpenOptionNonBlocking)
     mode |= O_NONBLOCK;
 
   if (open_options & eOpenOptionCanCreateNewOnly)
     mode |= O_CREAT | O_EXCL;
   else if (open_options & eOpenOptionCanCreate)
     mode |= O_CREAT;
 
   return mode;
 }
-

lldb/source/Host/common/FileCache.cpp

 
 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) {
+lldb::user_id_t FileCache::OpenFile(const FileSpec &file_spec,
+                                    File::OpenOptions flags, uint32_t mode,
+                                    Status &error) {
   if (!file_spec) {
     error.SetErrorString("empty path");
     return UINT64_MAX;
   }
   auto file = FileSystem::Instance().Open(file_spec, flags, mode);
   if (!file) {
     error = file.takeError();
     return UINT64_MAX;

lldb/source/Host/common/FileSystem.cpp

if (permissions & lldb::eFilePermissionsWorldRead)
     mode |= S_IROTH;
   if (permissions & lldb::eFilePermissionsWorldWrite)
     mode |= S_IWOTH;
   if (permissions & lldb::eFilePermissionsWorldExecute)
     mode |= S_IXOTH;
   return mode;
 }
 
-Expected<FileUP> FileSystem::Open(const FileSpec &file_spec, uint32_t options,
+Expected<FileUP> FileSystem::Open(const FileSpec &file_spec,
+                                  File::OpenOptions options,
                                   uint32_t permissions, bool should_close_fd) {
   if (m_collector)
     m_collector->addFile(file_spec.GetPath());
 
   const int open_flags = GetOpenFlags(options);
   const mode_t open_mode =
       (open_flags & O_CREAT) ? GetOpenMode(permissions) : 0;
 

lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm

case FileAction::eFileActionOpen: {
             }
           }
         }
         Status posix_error;
         int oflag = file_action->GetActionArgument();
         int created_fd =
             open(file_spec.GetPath().c_str(), oflag, S_IRUSR | S_IWUSR);
         if (created_fd >= 0) {
-          uint32_t file_options = 0;
+          auto file_options = File::OpenOptions(0);
           if ((oflag & O_RDWR) || (oflag & O_RDONLY))
             file_options |= File::eOpenOptionRead;
           if ((oflag & O_RDWR) || (oflag & O_RDONLY))
             file_options |= File::eOpenOptionWrite;
           file = std::make_shared<NativeFile>(created_fd, file_options, true);
           [options setValue:[NSNumber numberWithInteger:created_fd] forKey:key];
           return error; // Success
         } else {

lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h

Status MakeDirectory(const FileSpec &file_spec,
                        uint32_t file_permissions) override;
 
   Status GetFilePermissions(const FileSpec &file_spec,
                             uint32_t &file_permissions) override;
 
   Status SetFilePermissions(const FileSpec &file_spec,
                             uint32_t file_permissions) override;
 
-  lldb::user_id_t OpenFile(const FileSpec &file_spec, uint32_t flags,
+  lldb::user_id_t OpenFile(const FileSpec &file_spec, File::OpenOptions flags,
                            uint32_t mode, Status &error) override;
 
   bool CloseFile(lldb::user_id_t fd, Status &error) override;
 
   uint64_t ReadFile(lldb::user_id_t fd, uint64_t offset, void *data_ptr,
                     uint64_t len, Status &error) override;
 
   uint64_t WriteFile(lldb::user_id_t fd, uint64_t offset, const void *data,

lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp

LLDB_LOGF(log,
             "PlatformRemoteGDBServer::SetFilePermissions(path='%s', "
             "file_permissions=%o) error = %u (%s)",
             file_spec.GetCString(), file_permissions, error.GetError(),
             error.AsCString());
   return error;
 }
 
 lldb::user_id_t PlatformRemoteGDBServer::OpenFile(const FileSpec &file_spec,
-                                                  uint32_t flags, uint32_t mode,
+                                                  File::OpenOptions flags,
+                                                  uint32_t mode,
                                                   Status &error) {
   return m_gdb_client.OpenFile(file_spec, flags, mode, error);
 }
 
 bool PlatformRemoteGDBServer::CloseFile(lldb::user_id_t fd, Status &error) {
   return m_gdb_client.CloseFile(fd, error);
 }
 

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

 #include "GDBRemoteClientBase.h"
 
 #include <chrono>
 #include <map>
 #include <mutex>
 #include <string>
 #include <vector>
 
+#include "lldb/Host/File.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/GDBRemote.h"
 #include "lldb/Utility/StructuredData.h"
 #if defined(_WIN32)
 #include "lldb/Host/windows/PosixApi.h"
 #endif
 
 #include "llvm/ADT/Optional.h"
LazyBool SupportsAllocDeallocMemory() // const
     // alloc/dealloc memory packets.
     // m_supports_alloc_dealloc_memory = lldb_private::eLazyBoolNo;
     return m_supports_alloc_dealloc_memory;
   }
 
   size_t GetCurrentThreadIDs(std::vector<lldb::tid_t> &thread_ids,
                              bool &sequence_mutex_unavailable);
 
-  lldb::user_id_t OpenFile(const FileSpec &file_spec, uint32_t flags,
+  lldb::user_id_t OpenFile(const FileSpec &file_spec, File::OpenOptions flags,
                            mode_t mode, Status &error);
 
   bool CloseFile(lldb::user_id_t fd, Status &error);
 
   lldb::user_id_t GetFileSize(const FileSpec &file_spec);
 
   Status GetFilePermissions(const FileSpec &file_spec,
                             uint32_t &file_permissions);

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

if (response.GetChar() == ',') {
     else
       error.SetError(-1, eErrorTypeGeneric);
   } else
     error.Clear();
   return result;
 }
 lldb::user_id_t
 GDBRemoteCommunicationClient::OpenFile(const lldb_private::FileSpec &file_spec,
-                                       uint32_t flags, mode_t mode,
+                                       File::OpenOptions flags, mode_t mode,
                                        Status &error) {
   std::string path(file_spec.GetPath(false));
   lldb_private::StreamString stream;
   stream.PutCString("vFile:open:");
   if (path.empty())
     return UINT64_MAX;
   stream.PutStringAsRawHex8(path);
   stream.PutChar(',');

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_vFile_Open(
     StringExtractorGDBRemote &packet) {
   packet.SetFilePos(::strlen("vFile:open:"));
   std::string path;
   packet.GetHexByteStringTerminatedBy(path, ',');
   if (!path.empty()) {
     if (packet.GetChar() == ',') {
-      uint32_t flags = packet.GetHexMaxU32(false, 0);
+      // FIXME
+      // The flag values for OpenOptions do not match the values used by GDB
+      // * https://sourceware.org/gdb/onlinedocs/gdb/Open-Flags.html#Open-Flags
+      // * rdar://problem/46788934
+      auto flags = File::OpenOptions(packet.GetHexMaxU32(false, 0));
       if (packet.GetChar() == ',') {
         mode_t mode = packet.GetHexMaxU32(false, 0600);
         FileSpec path_spec(path);
         FileSystem::Instance().Resolve(path_spec);
         // Do not close fd.
         auto file = FileSystem::Instance().Open(path_spec, flags, mode, false);
 
         int save_errno = 0;

 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_vFile_Close(
     StringExtractorGDBRemote &packet) {
   packet.SetFilePos(::strlen("vFile:close:"));
   int fd = packet.GetS32(-1);
   int err = -1;
   int save_errno = 0;
   if (fd >= 0) {
-    NativeFile file(fd, 0, true);
+    NativeFile file(fd, File::OpenOptions(0), true);
     Status error = file.Close();
     err = 0;
     save_errno = error.GetError();
   } else {
     save_errno = EINVAL;
   }
   StreamString response;
   response.PutChar('F');

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

 FileUP PythonFile::GetUnderlyingFile() const {
   if (!IsValid())
     return nullptr;
 
   // We don't own the file descriptor returned by this function, make sure the
   // File object knows about that.
   PythonString py_mode = GetAttributeValue("mode").AsType<PythonString>();
   auto options = File::GetOptionsFromMode(py_mode.GetString());
-  auto file = std::unique_ptr<File>(
-      new NativeFile(PyObject_AsFileDescriptor(m_py_obj), options, false));
+  if (!options) {
+    llvm::consumeError(options.takeError());
+    return nullptr;
+  }
+  auto file = std::unique_ptr<File>(new NativeFile(
+      PyObject_AsFileDescriptor(m_py_obj), options.get(), false));
   if (!file->IsValid())
     return nullptr;
   return file;
 }
 
 namespace {
 class GIL {
 public:

 void PythonException::log(llvm::raw_ostream &OS) const { OS << toCString(); }
 
 std::error_code PythonException::convertToErrorCode() const {
   return llvm::inconvertibleErrorCode();
 }
 
 char PythonException::ID = 0;
 
-llvm::Expected<uint32_t> GetOptionsForPyObject(const PythonObject &obj) {
-  uint32_t options = 0;
+llvm::Expected<File::OpenOptions>
+GetOptionsForPyObject(const PythonObject &obj) {
 #if PY_MAJOR_VERSION >= 3
+  auto options = File::OpenOptions(0);
   auto readable = As<bool>(obj.CallMethod("readable"));
   if (!readable)
     return readable.takeError();
   auto writable = As<bool>(obj.CallMethod("writable"));
   if (!writable)
     return writable.takeError();
   if (readable.get())
     options |= File::eOpenOptionRead;
   if (writable.get())
     options |= File::eOpenOptionWrite;
+  return options;
 #else
   PythonString py_mode = obj.GetAttributeValue("mode").AsType<PythonString>();
-  options = File::GetOptionsFromMode(py_mode.GetString());
+  return File::GetOptionsFromMode(py_mode.GetString());
 #endif
-  return options;
 }
 
 // Base class template for python files.   All it knows how to do
 // is hold a reference to the python object and close or flush it
 // when the File is closed.
 namespace {
 template <typename Base> class OwnedPythonFile : public Base {
 public:

 } // namespace
 
 // A SimplePythonFile is a OwnedPythonFile that just does all I/O as
 // a NativeFile
 namespace {
 class SimplePythonFile : public OwnedPythonFile<NativeFile> {
 public:
   SimplePythonFile(const PythonFile &file, bool borrowed, int fd,
-                   uint32_t options)
+                   File::OpenOptions options)
       : OwnedPythonFile(file, borrowed, fd, options, false) {}
 };
 } // namespace
 
 #if PY_MAJOR_VERSION >= 3
 
 namespace {
 class PythonBuffer {

lldb/source/Target/Platform.cpp

bool Platform::IsCompatibleArchitecture(const ArchSpec &arch,
   return false;
 }
 
 Status Platform::PutFile(const FileSpec &source, const FileSpec &destination,
                          uint32_t uid, uint32_t gid) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM));
   LLDB_LOGF(log, "[PutFile] Using block by block transfer....\n");
 
-  uint32_t source_open_options =
+  auto source_open_options =
       File::eOpenOptionRead | File::eOpenOptionCloseOnExec;
   namespace fs = llvm::sys::fs;
   if (fs::is_symlink_file(source.GetPath()))
     source_open_options |= File::eOpenOptionDontFollowSymlinks;
 
   auto source_file = FileSystem::Instance().Open(source, source_open_options,
                                                  lldb::eFilePermissionsUserRW);
   if (!source_file)

lldb/source/Target/RemoteAwarePlatform.cpp

Status RemoteAwarePlatform::SetFilePermissions(const FileSpec &file_spec,
                                                uint32_t file_permissions) {
   if (m_remote_platform_sp)
     return m_remote_platform_sp->SetFilePermissions(file_spec,
                                                     file_permissions);
   return Platform::SetFilePermissions(file_spec, file_permissions);
 }
 
 lldb::user_id_t RemoteAwarePlatform::OpenFile(const FileSpec &file_spec,
-                                              uint32_t flags, uint32_t mode,
-                                              Status &error) {
+                                              File::OpenOptions flags,
+                                              uint32_t mode, Status &error) {
   if (IsHost())
     return FileCache::GetInstance().OpenFile(file_spec, flags, mode, error);
   if (m_remote_platform_sp)
     return m_remote_platform_sp->OpenFile(file_spec, flags, mode, error);
   return Platform::OpenFile(file_spec, flags, mode, error);
 }
 
 bool RemoteAwarePlatform::CloseFile(lldb::user_id_t fd, Status &error) {

lldb/source/Target/Target.cpp

Status Target::SerializeBreakpointsToFile(const FileSpec &file,
   }
 
   if (!break_store_ptr) {
     break_store_sp = std::make_shared<StructuredData::Array>();
     break_store_ptr = break_store_sp.get();
   }
 
   StreamFile out_file(path.c_str(),
-                      File::OpenOptions::eOpenOptionTruncate |
-                          File::OpenOptions::eOpenOptionWrite |
-                          File::OpenOptions::eOpenOptionCanCreate |
-                          File::OpenOptions::eOpenOptionCloseOnExec,
+                      File::eOpenOptionTruncate | File::eOpenOptionWrite |
+                          File::eOpenOptionCanCreate |
+                          File::eOpenOptionCloseOnExec,
                       lldb::eFilePermissionsFileDefault);
   if (!out_file.GetFile().IsValid()) {
     error.SetErrorStringWithFormat("Unable to open output file: %s.",
                                    path.c_str());
     return error;
   }
 
   std::unique_lock<std::recursive_mutex> lock;