diff options
26 files changed, 278 insertions, 208 deletions
diff --git a/lldb/include/lldb/Core/StreamFile.h b/lldb/include/lldb/Core/StreamFile.h index 54aeab16b3f..77e1557cb91 100644 --- a/lldb/include/lldb/Core/StreamFile.h +++ b/lldb/include/lldb/Core/StreamFile.h @@ -35,18 +35,22 @@ public: StreamFile(FILE *fh, bool transfer_ownership); + StreamFile(std::shared_ptr<File> file) : m_file_sp(file) { assert(file); }; + ~StreamFile() override; - File &GetFile() { return m_file; } + File &GetFile() { return *m_file_sp; } + + const File &GetFile() const { return *m_file_sp; } - const File &GetFile() const { return m_file; } + std::shared_ptr<File> GetFileSP() { return m_file_sp; } void Flush() override; protected: // Classes that inherit from StreamFile can see and modify these - File m_file; + std::shared_ptr<File> m_file_sp; // never NULL size_t WriteImpl(const void *s, size_t length) override; private: diff --git a/lldb/include/lldb/Host/FileCache.h b/lldb/include/lldb/Host/FileCache.h index 0c1ef196164..49f966058f8 100644 --- a/lldb/include/lldb/Host/FileCache.h +++ b/lldb/include/lldb/Host/FileCache.h @@ -22,7 +22,7 @@ class FileCache { private: FileCache() {} - typedef std::map<lldb::user_id_t, lldb::FileSP> FDToFileMap; + typedef std::map<lldb::user_id_t, lldb::FileUP> FDToFileMap; public: static FileCache &GetInstance(); diff --git a/lldb/include/lldb/Host/FileSystem.h b/lldb/include/lldb/Host/FileSystem.h index 733a4a4fe39..6900f3f7359 100644 --- a/lldb/include/lldb/Host/FileSystem.h +++ b/lldb/include/lldb/Host/FileSystem.h @@ -63,9 +63,10 @@ public: /// 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, - bool should_close_fd = true); + llvm::Expected<std::unique_ptr<File>> + Open(const FileSpec &file_spec, uint32_t options, + uint32_t permissions = lldb::eFilePermissionsFileDefault, + bool should_close_fd = true); /// Get a directory iterator. /// \{ diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h index 0cdaf1cf9bf..35f8928debe 100644 --- a/lldb/include/lldb/lldb-forward.h +++ b/lldb/include/lldb/lldb-forward.h @@ -331,6 +331,7 @@ typedef std::shared_ptr<lldb_private::EventDataStructuredData> typedef std::shared_ptr<lldb_private::ExecutionContextRef> ExecutionContextRefSP; typedef std::shared_ptr<lldb_private::ExpressionVariable> ExpressionVariableSP; +typedef std::unique_ptr<lldb_private::File> FileUP; typedef std::shared_ptr<lldb_private::File> FileSP; typedef std::shared_ptr<lldb_private::Function> FunctionSP; typedef std::shared_ptr<lldb_private::FunctionCaller> FunctionCallerSP; diff --git a/lldb/scripts/Python/python-typemaps.swig b/lldb/scripts/Python/python-typemaps.swig index e3956faf20c..0dcd050356e 100644 --- a/lldb/scripts/Python/python-typemaps.swig +++ b/lldb/scripts/Python/python-typemaps.swig @@ -395,13 +395,12 @@ bool SetNumberFromPyObject<double>(double &number, PyObject *obj) { else { PythonFile py_file(PyRefType::Borrowed, $input); - File file; - if (!py_file.GetUnderlyingFile(file)) + lldb::FileUP file = py_file.GetUnderlyingFile(); + if (!file) return nullptr; - - $1 = file.GetStream(); + $1 = file->GetStream(); if ($1) - file.Clear(); + file->Clear(); } } diff --git a/lldb/source/API/SBStream.cpp b/lldb/source/API/SBStream.cpp index ae652338e1e..910563824a8 100644 --- a/lldb/source/API/SBStream.cpp +++ b/lldb/source/API/SBStream.cpp @@ -82,26 +82,27 @@ void SBStream::RedirectToFile(const char *path, bool append) { if (!m_is_file) local_data = static_cast<StreamString *>(m_opaque_up.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; - FileSystem::Instance().Open(stream_file->GetFile(), FileSpec(path), - open_options); - m_opaque_up.reset(stream_file); + llvm::Expected<FileUP> file = + FileSystem::Instance().Open(FileSpec(path), open_options); + if (!file) { + LLDB_LOG_ERROR(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), file.takeError(), + "Cannot open {1}: {0}", path); + return; + } - if (m_opaque_up) { - 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()) - m_opaque_up->Write(&local_data[0], local_data.size()); - } else - m_is_file = false; + m_opaque_up = std::make_unique<StreamFile>(std::move(file.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()) + m_opaque_up->Write(&local_data[0], local_data.size()); } void SBStream::RedirectToFileHandle(FILE *fh, bool transfer_fh_ownership) { @@ -118,17 +119,14 @@ void SBStream::RedirectToFileHandle(FILE *fh, bool transfer_fh_ownership) { if (!m_is_file) local_data = static_cast<StreamString *>(m_opaque_up.get())->GetString(); } - m_opaque_up.reset(new StreamFile(fh, transfer_fh_ownership)); - if (m_opaque_up) { - 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()) - m_opaque_up->Write(&local_data[0], local_data.size()); - } else - m_is_file = false; + m_opaque_up = std::make_unique<StreamFile>(fh, transfer_fh_ownership); + 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()) + m_opaque_up->Write(&local_data[0], local_data.size()); } void SBStream::RedirectToFileDescriptor(int fd, bool transfer_fh_ownership) { @@ -143,16 +141,13 @@ void SBStream::RedirectToFileDescriptor(int fd, bool transfer_fh_ownership) { local_data = static_cast<StreamString *>(m_opaque_up.get())->GetString(); } - m_opaque_up.reset(new StreamFile(::fdopen(fd, "w"), transfer_fh_ownership)); - if (m_opaque_up) { - 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()) - m_opaque_up->Write(&local_data[0], local_data.size()); - } else - m_is_file = false; + m_opaque_up = std::make_unique<StreamFile>(fd, transfer_fh_ownership); + 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()) + m_opaque_up->Write(&local_data[0], local_data.size()); } lldb_private::Stream *SBStream::operator->() { return m_opaque_up.get(); } diff --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp index 1e9c9909f54..ca2b3f52141 100644 --- a/lldb/source/Commands/CommandObjectMemory.cpp +++ b/lldb/source/Commands/CommandObjectMemory.cpp @@ -765,8 +765,8 @@ protected: m_prev_varobj_options = m_varobj_options; m_prev_compiler_type = compiler_type; - StreamFile outfile_stream; - Stream *output_stream = nullptr; + std::unique_ptr<Stream> output_stream_storage; + Stream *output_stream_p = nullptr; const FileSpec &outfile_spec = m_outfile_options.GetFile().GetCurrentValue(); @@ -779,12 +779,14 @@ protected: if (append) open_options |= File::eOpenOptionAppend; - Status error = FileSystem::Instance().Open(outfile_stream.GetFile(), - outfile_spec, open_options); - if (error.Success()) { + auto outfile = FileSystem::Instance().Open(outfile_spec, open_options); + + if (outfile) { + auto outfile_stream_up = + std::make_unique<StreamFile>(std::move(outfile.get())); if (m_memory_options.m_output_as_binary) { const size_t bytes_written = - outfile_stream.Write(data_sp->GetBytes(), bytes_read); + outfile_stream_up->Write(data_sp->GetBytes(), bytes_read); if (bytes_written > 0) { result.GetOutputStream().Printf( "%zi bytes %s to '%s'\n", bytes_written, @@ -800,16 +802,19 @@ protected: } else { // We are going to write ASCII to the file just point the // output_stream to our outfile_stream... - output_stream = &outfile_stream; + output_stream_storage = std::move(outfile_stream_up); + output_stream_p = output_stream_storage.get(); } } else { - result.AppendErrorWithFormat("Failed to open file '%s' for %s.\n", + result.AppendErrorWithFormat("Failed to open file '%s' for %s:\n", path.c_str(), append ? "append" : "write"); + + result.AppendError(llvm::toString(outfile.takeError())); result.SetStatus(eReturnStatusFailed); return false; } } else { - output_stream = &result.GetOutputStream(); + output_stream_p = &result.GetOutputStream(); } ExecutionContextScope *exe_scope = m_exe_ctx.GetBestExecutionContextScope(); @@ -829,7 +834,7 @@ protected: DumpValueObjectOptions options(m_varobj_options.GetAsDumpOptions( eLanguageRuntimeDescriptionDisplayVerbosityFull, format)); - valobj_sp->Dump(*output_stream, options); + valobj_sp->Dump(*output_stream_p, options); } else { result.AppendErrorWithFormat( "failed to create a value object for: (%s) %s\n", @@ -869,13 +874,13 @@ protected: } } - assert(output_stream); + assert(output_stream_p); size_t bytes_dumped = DumpDataExtractor( - data, output_stream, 0, format, item_byte_size, item_count, + data, output_stream_p, 0, format, item_byte_size, item_count, num_per_line / target->GetArchitecture().GetDataByteSize(), addr, 0, 0, exe_scope); m_next_addr = addr + bytes_dumped; - output_stream->EOL(); + output_stream_p->EOL(); return true; } diff --git a/lldb/source/Core/StreamFile.cpp b/lldb/source/Core/StreamFile.cpp index 17662a43604..fe5d7413114 100644 --- a/lldb/source/Core/StreamFile.cpp +++ b/lldb/source/Core/StreamFile.cpp @@ -8,6 +8,7 @@ #include "lldb/Core/StreamFile.h" #include "lldb/Host/FileSystem.h" +#include "lldb/Utility/Log.h" #include <stdio.h> @@ -15,35 +16,54 @@ using namespace lldb; using namespace lldb_private; // StreamFile constructor -StreamFile::StreamFile() : Stream(), m_file() {} +StreamFile::StreamFile() : Stream() { m_file_sp = std::make_shared<File>(); } StreamFile::StreamFile(uint32_t flags, uint32_t addr_size, ByteOrder byte_order) - : Stream(flags, addr_size, byte_order), m_file() {} + : Stream(flags, addr_size, byte_order) { + m_file_sp = std::make_shared<File>(); +} -StreamFile::StreamFile(int fd, bool transfer_ownership) - : Stream(), m_file(fd, File::eOpenOptionWrite, transfer_ownership) {} +StreamFile::StreamFile(int fd, bool transfer_ownership) : Stream() { + m_file_sp = + std::make_shared<File>(fd, File::eOpenOptionWrite, transfer_ownership); +} -StreamFile::StreamFile(FILE *fh, bool transfer_ownership) - : Stream(), m_file(fh, transfer_ownership) {} +StreamFile::StreamFile(FILE *fh, bool transfer_ownership) : Stream() { + m_file_sp = std::make_shared<File>(fh, transfer_ownership); +} -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) : Stream() { + auto file = FileSystem::Instance().Open( + FileSpec(path), File::eOpenOptionWrite | File::eOpenOptionCanCreate | + File::eOpenOptionCloseOnExec); + 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); + m_file_sp = std::make_shared<File>(); + } } StreamFile::StreamFile(const char *path, uint32_t options, uint32_t permissions) - : Stream(), m_file() { - - FileSystem::Instance().Open(m_file, FileSpec(path), options, 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); + m_file_sp = std::make_shared<File>(); + } } StreamFile::~StreamFile() {} -void StreamFile::Flush() { m_file.Flush(); } +void StreamFile::Flush() { m_file_sp->Flush(); } size_t StreamFile::WriteImpl(const void *s, size_t length) { - m_file.Write(s, length); + m_file_sp->Write(s, length); return length; } diff --git a/lldb/source/Expression/REPL.cpp b/lldb/source/Expression/REPL.cpp index f7bd86391f5..cfa520981be 100644 --- a/lldb/source/Expression/REPL.cpp +++ b/lldb/source/Expression/REPL.cpp @@ -398,17 +398,22 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) { // Update our code on disk if (!m_repl_source_path.empty()) { - lldb_private::File file; - FileSystem::Instance().Open(file, FileSpec(m_repl_source_path), + auto file = FileSystem::Instance().Open(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(); + if (file) { + std::string code(m_code.CopyList()); + code.append(1, '\n'); + size_t bytes_written = code.size(); + file.get()->Write(code.c_str(), bytes_written); + file.get()->Close(); + } else { + std::string message = llvm::toString(file.takeError()); + error_sp->Printf("error: couldn't open %s: %s\n", + m_repl_source_path.c_str(), message.c_str()); + } // Now set the default file and line to the REPL source file m_target.GetSourceManager().SetDefaultFileAndLine( diff --git a/lldb/source/Host/common/FileCache.cpp b/lldb/source/Host/common/FileCache.cpp index 4bd3efda7fb..0a7197e55b0 100644 --- a/lldb/source/Host/common/FileCache.cpp +++ b/lldb/source/Host/common/FileCache.cpp @@ -29,12 +29,13 @@ lldb::user_id_t FileCache::OpenFile(const FileSpec &file_spec, uint32_t flags, error.SetErrorString("empty path"); return UINT64_MAX; } - FileSP file_sp(new File()); - error = FileSystem::Instance().Open(*file_sp, file_spec, flags, mode); - if (!file_sp->IsValid()) + auto file = FileSystem::Instance().Open(file_spec, flags, mode); + if (!file) { + error = file.takeError(); return UINT64_MAX; - lldb::user_id_t fd = file_sp->GetDescriptor(); - m_cache[fd] = file_sp; + } + lldb::user_id_t fd = file.get()->GetDescriptor(); + m_cache[fd] = std::move(file.get()); return fd; } @@ -48,12 +49,12 @@ bool FileCache::CloseFile(lldb::user_id_t fd, Status &error) { error.SetErrorStringWithFormat("invalid host file descriptor %" PRIu64, fd); return false; } - FileSP file_sp = pos->second; - if (!file_sp) { + FileUP &file_up = pos->second; + if (!file_up) { error.SetErrorString("invalid host backing file"); return false; } - error = file_sp->Close(); + error = file_up->Close(); m_cache.erase(pos); return error.Success(); } @@ -70,16 +71,16 @@ uint64_t FileCache::WriteFile(lldb::user_id_t fd, uint64_t offset, error.SetErrorStringWithFormat("invalid host file descriptor %" PRIu64, fd); return false; } - FileSP file_sp = pos->second; - if (!file_sp) { + FileUP &file_up = pos->second; + if (!file_up) { error.SetErrorString("invalid host backing file"); return UINT64_MAX; } - if (static_cast<uint64_t>(file_sp->SeekFromStart(offset, &error)) != offset || + if (static_cast<uint64_t>(file_up->SeekFromStart(offset, &error)) != offset || error.Fail()) return UINT64_MAX; size_t bytes_written = src_len; - error = file_sp->Write(src, bytes_written); + error = file_up->Write(src, bytes_written); if (error.Fail()) return UINT64_MAX; return bytes_written; @@ -96,16 +97,16 @@ uint64_t FileCache::ReadFile(lldb::user_id_t fd, uint64_t offset, void *dst, error.SetErrorStringWithFormat("invalid host file descriptor %" PRIu64, fd); return false; } - FileSP file_sp = pos->second; - if (!file_sp) { + FileUP &file_up = pos->second; + if (!file_up) { error.SetErrorString("invalid host backing file"); return UINT64_MAX; } - if (static_cast<uint64_t>(file_sp->SeekFromStart(offset, &error)) != offset || + if (static_cast<uint64_t>(file_up->SeekFromStart(offset, &error)) != offset || error.Fail()) return UINT64_MAX; size_t bytes_read = dst_len; - error = file_sp->Read(dst, bytes_read); + error = file_up->Read(dst, bytes_read); if (error.Fail()) return UINT64_MAX; return bytes_read; diff --git a/lldb/source/Host/common/FileSystem.cpp b/lldb/source/Host/common/FileSystem.cpp index 23b3ebb0b4f..b3442770f01 100644 --- a/lldb/source/Host/common/FileSystem.cpp +++ b/lldb/source/Host/common/FileSystem.cpp @@ -415,33 +415,29 @@ static mode_t GetOpenMode(uint32_t permissions) { return mode; } -Status FileSystem::Open(File &File, const FileSpec &file_spec, uint32_t options, - uint32_t permissions, bool should_close_fd) { +Expected<FileUP> FileSystem::Open(const FileSpec &file_spec, uint32_t options, + uint32_t permissions, bool should_close_fd) { if (m_collector) m_collector->addFile(file_spec.GetPath()); - if (File.IsValid()) - File.Close(); - const int open_flags = GetOpenFlags(options); const mode_t open_mode = (open_flags & O_CREAT) ? GetOpenMode(permissions) : 0; auto path = GetExternalPath(file_spec); if (!path) - return Status(path.getError()); + return errorCodeToError(path.getError()); int descriptor = llvm::sys::RetryAfterSignal( -1, OpenWithFS, *this, path->c_str(), open_flags, open_mode); - Status error; - if (!File::DescriptorIsValid(descriptor)) { - File.SetDescriptor(-1, options, false); - error.SetErrorToErrno(); - } else { - File.SetDescriptor(descriptor, options, should_close_fd); - } - return error; + if (!File::DescriptorIsValid(descriptor)) + return llvm::errorCodeToError( + std::error_code(errno, std::system_category())); + + auto file = std::make_unique<File>(descriptor, options, should_close_fd); + assert(file->IsValid()); + return std::move(file); } ErrorOr<std::string> FileSystem::GetExternalPath(const llvm::Twine &path) { diff --git a/lldb/source/Host/windows/Host.cpp b/lldb/source/Host/windows/Host.cpp index 2ed28066a73..19be271e831 100644 --- a/lldb/source/Host/windows/Host.cpp +++ b/lldb/source/Host/windows/Host.cpp @@ -34,9 +34,11 @@ namespace { bool GetTripleForProcess(const FileSpec &executable, llvm::Triple &triple) { // Open the PE File as a binary file, and parse just enough information to // determine the machine type. - File imageBinary; - FileSystem::Instance().Open(imageBinary, executable, File::eOpenOptionRead, - lldb::eFilePermissionsUserRead); + auto imageBinaryP = FileSystem::Instance().Open( + executable, File::eOpenOptionRead, lldb::eFilePermissionsUserRead); + if (!imageBinaryP) + return llvm::errorToBool(imageBinaryP.takeError()); + File &imageBinary = *imageBinaryP.get(); imageBinary.SeekFromStart(0x3c); int32_t peOffset = 0; uint32_t peHead = 0; diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index a5b2d24aeba..6c97391a68b 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -70,6 +70,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" +#include "llvm/Support/FormatAdapters.h" #include "llvm/Support/Path.h" #include "llvm/Support/PrettyStackTrace.h" @@ -2337,18 +2338,17 @@ void CommandInterpreter::HandleCommandsFromFile( return; } - StreamFileSP input_file_sp(new StreamFile()); std::string cmd_file_path = cmd_file.GetPath(); - Status error = FileSystem::Instance().Open(input_file_sp->GetFile(), cmd_file, - File::eOpenOptionRead); - if (error.Fail()) { - result.AppendErrorWithFormat( - "error: an error occurred read file '%s': %s\n", cmd_file_path.c_str(), - error.AsCString()); + auto file = FileSystem::Instance().Open(cmd_file, File::eOpenOptionRead); + if (!file) { + result.AppendErrorWithFormatv( + "error: an error occurred read file '{0}': {1}\n", cmd_file_path, + llvm::fmt_consume(file.takeError())); result.SetStatus(eReturnStatusFailed); return; } + auto input_file_sp = std::make_shared<StreamFile>(std::move(file.get())); Debugger &debugger = GetDebugger(); diff --git a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp index 38bd79d68cf..ad1b3411c67 100644 --- a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp +++ b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp @@ -2651,14 +2651,14 @@ bool RenderScriptRuntime::SaveAllocation(Stream &strm, const uint32_t alloc_id, // Check we can create writable file FileSpec file_spec(path); FileSystem::Instance().Resolve(file_spec); - File file; - FileSystem::Instance().Open(file, file_spec, - File::eOpenOptionWrite | - File::eOpenOptionCanCreate | - File::eOpenOptionTruncate); + auto file = FileSystem::Instance().Open( + file_spec, File::eOpenOptionWrite | File::eOpenOptionCanCreate | + File::eOpenOptionTruncate); if (!file) { - strm.Printf("Error: Failed to open '%s' for writing", path); + std::string error = llvm::toString(file.takeError()); + strm.Printf("Error: Failed to open '%s' for writing: %s", path, + error.c_str()); strm.EOL(); return false; } @@ -2690,7 +2690,7 @@ bool RenderScriptRuntime::SaveAllocation(Stream &strm, const uint32_t alloc_id, LLDB_LOGF(log, "%s - writing File Header, 0x%" PRIx64 " bytes", __FUNCTION__, (uint64_t)num_bytes); - Status err = file.Write(&head, num_bytes); + Status err = file.get()->Write(&head, num_bytes); if (!err.Success()) { strm.Printf("Error: '%s' when writing to file '%s'", err.AsCString(), path); strm.EOL(); @@ -2715,7 +2715,7 @@ bool RenderScriptRuntime::SaveAllocation(Stream &strm, const uint32_t alloc_id, LLDB_LOGF(log, "%s - writing element headers, 0x%" PRIx64 " bytes.", __FUNCTION__, (uint64_t)num_bytes); - err = file.Write(element_header_buffer.get(), num_bytes); + err = file.get()->Write(element_header_buffer.get(), num_bytes); if (!err.Success()) { strm.Printf("Error: '%s' when writing to file '%s'", err.AsCString(), path); strm.EOL(); @@ -2727,7 +2727,7 @@ bool RenderScriptRuntime::SaveAllocation(Stream &strm, const uint32_t alloc_id, LLDB_LOGF(log, "%s - writing 0x%" PRIx64 " bytes", __FUNCTION__, (uint64_t)num_bytes); - err = file.Write(buffer.get(), num_bytes); + err = file.get()->Write(buffer.get(), num_bytes); if (!err.Success()) { strm.Printf("Error: '%s' when writing to file '%s'", err.AsCString(), path); strm.EOL(); @@ -4578,32 +4578,36 @@ public: return false; } - Stream *output_strm = nullptr; - StreamFile outfile_stream; + Stream *output_stream_p = nullptr; + std::unique_ptr<Stream> output_stream_storage; + const FileSpec &outfile_spec = m_options.m_outfile; // Dump allocation to file instead if (outfile_spec) { // Open output file 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; + auto file = FileSystem::Instance().Open( + outfile_spec, File::eOpenOptionWrite | File::eOpenOptionCanCreate); + if (file) { + output_stream_storage = + std::make_unique<StreamFile>(std::move(file.get())); + output_stream_p = output_stream_storage.get(); result.GetOutputStream().Printf("Results written to '%s'", path.c_str()); result.GetOutputStream().EOL(); } else { - result.AppendErrorWithFormat("Couldn't open file '%s'", path.c_str()); + std::string error = llvm::toString(file.takeError()); + result.AppendErrorWithFormat("Couldn't open file '%s': %s", + path.c_str(), error.c_str()); result.SetStatus(eReturnStatusFailed); return false; } } else - output_strm = &result.GetOutputStream(); + output_stream_p = &result.GetOutputStream(); - assert(output_strm != nullptr); + assert(output_stream_p != nullptr); bool dumped = - runtime->DumpAllocation(*output_strm, m_exe_ctx.GetFramePtr(), id); + runtime->DumpAllocation(*output_stream_p, m_exe_ctx.GetFramePtr(), id); if (dumped) result.SetStatus(eReturnStatusSuccessFinishResult); diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index 577d80f03cd..2fad73c702a 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -6271,22 +6271,23 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, buffer.PutHex32(segment.flags); } - File core_file; std::string core_file_path(outfile.GetPath()); - error = FileSystem::Instance().Open(core_file, outfile, - File::eOpenOptionWrite | - File::eOpenOptionTruncate | - File::eOpenOptionCanCreate); - if (error.Success()) { + auto core_file = FileSystem::Instance().Open( + outfile, File::eOpenOptionWrite | File::eOpenOptionTruncate | + File::eOpenOptionCanCreate); + if (!core_file) { + error = core_file.takeError(); + } else { // 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); + error = core_file.get()->Write(buffer.GetString().data(), + bytes_written); if (error.Success()) { // Now write the file data for all memory segments in the process for (const auto &segment : segment_load_commands) { - if (core_file.SeekFromStart(segment.fileoff) == -1) { + if (core_file.get()->SeekFromStart(segment.fileoff) == -1) { error.SetErrorStringWithFormat( "unable to seek to offset 0x%" PRIx64 " in '%s'", segment.fileoff, core_file_path.c_str()); @@ -6311,7 +6312,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, if (bytes_read == bytes_to_read) { size_t bytes_written = bytes_read; - error = core_file.Write(bytes, bytes_written); + error = core_file.get()->Write(bytes, bytes_written); bytes_left -= bytes_read; addr += bytes_read; } else { @@ -6319,7 +6320,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, // be zero filled memset(bytes, 0, bytes_to_read); size_t bytes_written = bytes_to_read; - error = core_file.Write(bytes, bytes_written); + error = core_file.get()->Write(bytes, bytes_written); bytes_left -= bytes_to_read; addr += bytes_to_read; } diff --git a/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm b/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm index 03bb70f5163..1d259554fbc 100644 --- a/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm +++ b/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm @@ -382,7 +382,7 @@ bool CoreSimulatorSupport::Device::Shutdown(Status &err) { static Status HandleFileAction(ProcessLaunchInfo &launch_info, NSMutableDictionary *options, NSString *key, - const int fd, File &file) { + const int fd, lldb::FileSP &file) { Status error; const FileAction *file_action = launch_info.GetFileActionForFD(fd); if (file_action) { @@ -434,7 +434,7 @@ static Status HandleFileAction(ProcessLaunchInfo &launch_info, file_options |= File::eOpenOptionRead; if ((oflag & O_RDWR) || (oflag & O_RDONLY)) file_options |= File::eOpenOptionWrite; - file.SetDescriptor(created_fd, file_options, true); + file = std::make_shared<File>(created_fd, file_options, true); [options setValue:[NSNumber numberWithInteger:created_fd] forKey:key]; return error; // Success } else { @@ -494,9 +494,9 @@ CoreSimulatorSupport::Device::Spawn(ProcessLaunchInfo &launch_info) { [options setObject:env_dict forKey:kSimDeviceSpawnEnvironment]; Status error; - File stdin_file; - File stdout_file; - File stderr_file; + lldb::FileSP stdin_file; + lldb::FileSP stdout_file; + lldb::FileSP stderr_file; error = HandleFileAction(launch_info, options, kSimDeviceSpawnStdin, STDIN_FILENO, stdin_file); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp index 1fdd089dd5d..f60991034aa 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp @@ -512,14 +512,23 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_Open( mode_t mode = packet.GetHexMaxU32(false, 0600); FileSpec path_spec(path); FileSystem::Instance().Resolve(path_spec); - File file; // Do not close fd. - Status error = - FileSystem::Instance().Open(file, path_spec, flags, mode, false); - const int save_errno = error.GetError(); + auto file = FileSystem::Instance().Open(path_spec, flags, mode, false); + + int save_errno = 0; + int descriptor = File::kInvalidDescriptor; + if (file) { + descriptor = file.get()->GetDescriptor(); + } else { + std::error_code code = errorToErrorCode(file.takeError()); + if (code.category() == std::system_category()) { + save_errno = code.value(); + } + } + StreamString response; response.PutChar('F'); - response.Printf("%i", file.GetDescriptor()); + response.Printf("%i", descriptor); if (save_errno) response.Printf(",%i", save_errno); return SendPacketNoLock(response.GetString()); diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index d54363cae26..ab1e572316a 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -99,12 +99,14 @@ namespace 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 = FileSystem::Instance().Open(strm.GetFile(), FileSpec(path), - File::eOpenOptionWrite | - File::eOpenOptionCanCreate); - if (error.Success()) - ((ProcessGDBRemote *)p)->GetGDBRemote().DumpHistory(strm); + auto file = FileSystem::Instance().Open( + FileSpec(path), File::eOpenOptionWrite | File::eOpenOptionCanCreate); + if (!file) { + llvm::consumeError(file.takeError()); + return; + } + StreamFile stream(std::move(file.get())); + ((ProcessGDBRemote *)p)->GetGDBRemote().DumpHistory(stream); } } // namespace lldb diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp index 97f8388d771..bb7f77fa514 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp @@ -949,11 +949,6 @@ 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; - FileSystem::Instance().Open(file, FileSpec(path), GetOptionsFromMode(mode)); - Reset(file, mode); -} PythonFile::PythonFile(PyRefType type, PyObject *o) { Reset(type, o); } @@ -1036,17 +1031,19 @@ uint32_t PythonFile::GetOptionsFromMode(llvm::StringRef mode) { .Default(0); } -bool PythonFile::GetUnderlyingFile(File &file) const { +FileUP PythonFile::GetUnderlyingFile() const { if (!IsValid()) - return false; + return nullptr; - file.Close(); // 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 = PythonFile::GetOptionsFromMode(py_mode.GetString()); - file.SetDescriptor(PyObject_AsFileDescriptor(m_py_obj), options, false); - return file.IsValid(); + auto file = std::make_unique<File>(PyObject_AsFileDescriptor(m_py_obj), + options, false); + if (!file->IsValid()) + return nullptr; + return file; } #endif diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h index 049ce90bb1b..3e1a3541bc7 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h @@ -455,7 +455,6 @@ class PythonFile : public PythonObject { public: PythonFile(); PythonFile(File &file, const char *mode); - PythonFile(const char *path, const char *mode); PythonFile(PyRefType type, PyObject *o); ~PythonFile() override; @@ -469,7 +468,7 @@ public: static uint32_t GetOptionsFromMode(llvm::StringRef mode); - bool GetUnderlyingFile(File &file) const; + lldb::FileUP GetUnderlyingFile() const; }; } // namespace lldb_private diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp index e8af9db2ce4..1d130672825 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -45,6 +45,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/FileSystem.h" +#include "llvm/Support/FormatAdapters.h" #include <memory> #include <mutex> @@ -901,17 +902,24 @@ bool ScriptInterpreterPythonImpl::ExecuteOneLine( debugger.AdoptTopIOHandlerFilesIfInvalid(input_file_sp, output_file_sp, error_file_sp); } else { - input_file_sp = std::make_shared<StreamFile>(); - FileSystem::Instance().Open(input_file_sp->GetFile(), + auto nullin = FileSystem::Instance().Open( FileSpec(FileSystem::DEV_NULL), File::eOpenOptionRead); - - output_file_sp = std::make_shared<StreamFile>(); - FileSystem::Instance().Open(output_file_sp->GetFile(), + auto nullout = FileSystem::Instance().Open( FileSpec(FileSystem::DEV_NULL), File::eOpenOptionWrite); - - error_file_sp = output_file_sp; + if (!nullin) { + result->AppendErrorWithFormatv("failed to open /dev/null: {0}\n", + llvm::fmt_consume(nullin.takeError())); + return false; + } + if (!nullout) { + result->AppendErrorWithFormatv("failed to open /dev/null: {0}\n", + llvm::fmt_consume(nullout.takeError())); + return false; + } + input_file_sp = std::make_shared<StreamFile>(std::move(nullin.get())); + error_file_sp = output_file_sp = std::make_shared<StreamFile>(std::move(nullout.get())); } FILE *in_file = input_file_sp->GetFile().GetStream(); diff --git a/lldb/source/Target/ModuleCache.cpp b/lldb/source/Target/ModuleCache.cpp index cd3f211b81c..124cdacfb4d 100644 --- a/lldb/source/Target/ModuleCache.cpp +++ b/lldb/source/Target/ModuleCache.cpp @@ -48,7 +48,7 @@ std::string GetEscapedHostname(const char *hostname) { class ModuleLock { private: - File m_file; + FileUP m_file_up; std::unique_ptr<lldb_private::LockFile> m_lock; FileSpec m_file_spec; @@ -157,16 +157,19 @@ ModuleLock::ModuleLock(const FileSpec &root_dir_spec, const UUID &uuid, return; m_file_spec = JoinPath(lock_dir_spec, uuid.GetAsString().c_str()); - FileSystem::Instance().Open(m_file, m_file_spec, - File::eOpenOptionWrite | - File::eOpenOptionCanCreate | - File::eOpenOptionCloseOnExec); - if (!m_file) { - error.SetErrorToErrno(); + + auto file = FileSystem::Instance().Open( + m_file_spec, File::eOpenOptionWrite | File::eOpenOptionCanCreate | + File::eOpenOptionCloseOnExec); + if (file) + m_file_up = std::move(file.get()); + else { + m_file_up.reset(); + error = Status(file.takeError()); return; } - m_lock.reset(new lldb_private::LockFile(m_file.GetDescriptor())); + m_lock.reset(new lldb_private::LockFile(m_file_up->GetDescriptor())); error = m_lock->WriteLock(0, 1); if (error.Fail()) error.SetErrorStringWithFormat("Failed to lock file: %s", @@ -174,10 +177,11 @@ ModuleLock::ModuleLock(const FileSpec &root_dir_spec, const UUID &uuid, } void ModuleLock::Delete() { - if (!m_file) + if (!m_file_up) return; - m_file.Close(); + m_file_up->Close(); + m_file_up.reset(); llvm::sys::fs::remove(m_file_spec.GetPath()); } diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp index fac9b56026b..482c8b503c6 100644 --- a/lldb/source/Target/Platform.cpp +++ b/lldb/source/Target/Platform.cpp @@ -1226,15 +1226,15 @@ Status Platform::PutFile(const FileSpec &source, const FileSpec &destination, if (fs::is_symlink_file(source.GetPath())) source_open_options |= File::eOpenOptionDontFollowSymlinks; - File source_file; - Status error = FileSystem::Instance().Open( - source_file, source, source_open_options, lldb::eFilePermissionsUserRW); - uint32_t permissions = source_file.GetPermissions(error); + auto source_file = FileSystem::Instance().Open( + source, source_open_options, lldb::eFilePermissionsUserRW); + if (!source_file) + return Status(source_file.takeError()); + Status error; + uint32_t permissions = source_file.get()->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 | File::eOpenOptionTruncate | File::eOpenOptionCloseOnExec, @@ -1249,7 +1249,7 @@ Status Platform::PutFile(const FileSpec &source, const FileSpec &destination, uint64_t offset = 0; for (;;) { size_t bytes_read = buffer_sp->GetByteSize(); - error = source_file.Read(buffer_sp->GetBytes(), bytes_read); + error = source_file.get()->Read(buffer_sp->GetBytes(), bytes_read); if (error.Fail() || bytes_read == 0) break; @@ -1262,7 +1262,7 @@ Status Platform::PutFile(const FileSpec &source, const FileSpec &destination, if (bytes_written != bytes_read) { // We didn't write the correct number of bytes, so adjust the file // position in the source file we are reading from... - source_file.SeekFromStart(offset); + source_file.get()->SeekFromStart(offset); } } CloseFile(dest_file, error); diff --git a/lldb/unittests/Host/FileSystemTest.cpp b/lldb/unittests/Host/FileSystemTest.cpp index 7fea3b3a529..31184d9ad53 100644 --- a/lldb/unittests/Host/FileSystemTest.cpp +++ b/lldb/unittests/Host/FileSystemTest.cpp @@ -288,3 +288,18 @@ TEST(FileSystemTest, EnumerateDirectory) { EXPECT_THAT(visited, testing::UnorderedElementsAre("/foo", "/bar", "/baz", "/qux")); } + +TEST(FileSystemTest, OpenErrno) { +#ifdef _WIN32 + FileSpec spec("C:\\FILE\\THAT\\DOES\\NOT\\EXIST.TXT"); +#else + FileSpec spec("/file/that/does/not/exist.txt"); +#endif + FileSystem fs; + auto file = fs.Open(spec, File::eOpenOptionRead, 0, true); + ASSERT_FALSE(file); + std::error_code code = errorToErrorCode(file.takeError()); + EXPECT_EQ(code.category(), std::system_category()); + EXPECT_EQ(code.value(), ENOENT); +} + diff --git a/lldb/unittests/ScriptInterpreter/Python/CMakeLists.txt b/lldb/unittests/ScriptInterpreter/Python/CMakeLists.txt index fdb86db7cf3..618767de91e 100644 --- a/lldb/unittests/ScriptInterpreter/Python/CMakeLists.txt +++ b/lldb/unittests/ScriptInterpreter/Python/CMakeLists.txt @@ -6,6 +6,7 @@ add_lldb_unittest(ScriptInterpreterPythonTests LINK_LIBS lldbHost lldbPluginScriptInterpreterPython + LLVMTestingSupport LINK_COMPONENTS Support )
\ No newline at end of file diff --git a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp index d43da010b1d..e6d1c4d14b0 100644 --- a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp +++ b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp @@ -15,6 +15,7 @@ #include "lldb/Host/FileSystem.h" #include "lldb/Host/HostInfo.h" #include "lldb/lldb-enumerations.h" +#include "llvm/Testing/Support/Error.h" #include "PythonTestSuite.h" @@ -581,10 +582,10 @@ TEST_F(PythonDataObjectsTest, TestPythonCallableInvoke) { } TEST_F(PythonDataObjectsTest, TestPythonFile) { - File file; - FileSystem::Instance().Open(file, FileSpec(FileSystem::DEV_NULL), - File::eOpenOptionRead); - PythonFile py_file(file, "r"); + auto file = FileSystem::Instance().Open(FileSpec(FileSystem::DEV_NULL), + File::eOpenOptionRead); + ASSERT_THAT_EXPECTED(file, llvm::Succeeded()); + PythonFile py_file(*file.get(), "r"); EXPECT_TRUE(PythonFile::Check(py_file.get())); } |
