Author: jdevlieghere Date: Mon Sep 23 13:36:46 2019 New Revision: 372652 URL: http://llvm.org/viewvc/llvm-project?rev=372652&view=rev Log: File::SetDescriptor() should require options
lvm_private::File::GetStream() can fail if m_options == 0 It's not clear from the header a File created with a descriptor will be not be usable by many parts of LLDB unless SetOptions is also called, but it is. This is because those parts of LLDB rely on GetStream() to use the file, and that in turn relies on calling fdopen on the descriptor. When calling fdopen, GetStream relies on m_options to determine the access mode. If m_options has never been set, GetStream() will fail. This patch adds options as a required argument to File::SetDescriptor and the corresponding constructor. Patch by: Lawrence D'Anna Differential revision: https://reviews.llvm.org/D67792 Modified: lldb/trunk/include/lldb/Host/File.h lldb/trunk/source/Core/StreamFile.cpp lldb/trunk/source/Host/common/File.cpp lldb/trunk/source/Host/common/FileSystem.cpp lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp lldb/trunk/source/Target/Process.cpp lldb/trunk/unittests/Host/FileTest.cpp Modified: lldb/trunk/include/lldb/Host/File.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/File.h?rev=372652&r1=372651&r2=372652&view=diff ============================================================================== --- lldb/trunk/include/lldb/Host/File.h (original) +++ lldb/trunk/include/lldb/Host/File.h Mon Sep 23 13:36:46 2019 @@ -64,9 +64,9 @@ public: m_is_real_terminal(eLazyBoolCalculate), m_supports_colors(eLazyBoolCalculate) {} - File(int fd, bool transfer_ownership) + File(int fd, uint32_t options, bool transfer_ownership) : IOObject(eFDTypeFile, transfer_ownership), m_descriptor(fd), - m_stream(kInvalidStream), m_options(0), m_own_stream(false), + m_stream(kInvalidStream), m_options(options), m_own_stream(false), m_is_interactive(eLazyBoolCalculate), m_is_real_terminal(eLazyBoolCalculate) {} @@ -125,7 +125,7 @@ public: WaitableHandle GetWaitableHandle() override; - void SetDescriptor(int fd, bool transfer_ownership); + void SetDescriptor(int fd, uint32_t options, bool transfer_ownership); FILE *GetStream(); @@ -332,8 +332,6 @@ public: 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: Modified: lldb/trunk/source/Core/StreamFile.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/StreamFile.cpp?rev=372652&r1=372651&r2=372652&view=diff ============================================================================== --- lldb/trunk/source/Core/StreamFile.cpp (original) +++ lldb/trunk/source/Core/StreamFile.cpp Mon Sep 23 13:36:46 2019 @@ -21,7 +21,7 @@ StreamFile::StreamFile(uint32_t flags, u : Stream(flags, addr_size, byte_order), m_file() {} StreamFile::StreamFile(int fd, bool transfer_ownership) - : Stream(), m_file(fd, transfer_ownership) {} + : Stream(), m_file(fd, File::eOpenOptionWrite, transfer_ownership) {} StreamFile::StreamFile(FILE *fh, bool transfer_ownership) : Stream(), m_file(fh, transfer_ownership) {} Modified: lldb/trunk/source/Host/common/File.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/File.cpp?rev=372652&r1=372651&r2=372652&view=diff ============================================================================== --- lldb/trunk/source/Host/common/File.cpp (original) +++ lldb/trunk/source/Host/common/File.cpp Mon Sep 23 13:36:46 2019 @@ -93,11 +93,12 @@ int File::GetDescriptor() const { IOObject::WaitableHandle File::GetWaitableHandle() { return GetDescriptor(); } -void File::SetDescriptor(int fd, bool transfer_ownership) { +void File::SetDescriptor(int fd, uint32_t options, bool transfer_ownership) { if (IsValid()) Close(); m_descriptor = fd; m_should_close_fd = transfer_ownership; + m_options = options; } FILE *File::GetStream() { Modified: lldb/trunk/source/Host/common/FileSystem.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/FileSystem.cpp?rev=372652&r1=372651&r2=372652&view=diff ============================================================================== --- lldb/trunk/source/Host/common/FileSystem.cpp (original) +++ lldb/trunk/source/Host/common/FileSystem.cpp Mon Sep 23 13:36:46 2019 @@ -436,11 +436,10 @@ Status FileSystem::Open(File &File, cons Status error; if (!File::DescriptorIsValid(descriptor)) { - File.SetDescriptor(descriptor, false); + File.SetDescriptor(-1, options, false); error.SetErrorToErrno(); } else { - File.SetDescriptor(descriptor, should_close_fd); - File.SetOptions(options); + File.SetDescriptor(descriptor, options, should_close_fd); } return error; } Modified: lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp?rev=372652&r1=372651&r2=372652&view=diff ============================================================================== --- lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp (original) +++ lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp Mon Sep 23 13:36:46 2019 @@ -86,8 +86,8 @@ ConnectionFileDescriptor::ConnectionFile ConnectionFileDescriptor::ConnectionFileDescriptor(int fd, bool owns_fd) : Connection(), m_pipe(), m_mutex(), m_shutting_down(false), m_waiting_for_accept(false), m_child_processes_inherit(false) { - m_write_sp = std::make_shared<File>(fd, owns_fd); - m_read_sp = std::make_shared<File>(fd, false); + m_write_sp = std::make_shared<File>(fd, File::eOpenOptionWrite, owns_fd); + m_read_sp = std::make_shared<File>(fd, File::eOpenOptionRead, false); Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION | LIBLLDB_LOG_OBJECT)); @@ -218,8 +218,10 @@ ConnectionStatus ConnectionFileDescripto m_read_sp = std::move(tcp_socket); m_write_sp = m_read_sp; } else { - m_read_sp = std::make_shared<File>(fd, false); - m_write_sp = std::make_shared<File>(fd, false); + m_read_sp = + std::make_shared<File>(fd, File::eOpenOptionRead, false); + m_write_sp = + std::make_shared<File>(fd, File::eOpenOptionWrite, false); } m_uri = *addr; return eConnectionStatusSuccess; @@ -268,8 +270,8 @@ ConnectionStatus ConnectionFileDescripto ::fcntl(fd, F_SETFL, flags); } } - m_read_sp = std::make_shared<File>(fd, true); - m_write_sp = std::make_shared<File>(fd, false); + m_read_sp = std::make_shared<File>(fd, File::eOpenOptionRead, true); + m_write_sp = std::make_shared<File>(fd, File::eOpenOptionWrite, false); return eConnectionStatusSuccess; } #endif Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp?rev=372652&r1=372651&r2=372652&view=diff ============================================================================== --- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (original) +++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp Mon Sep 23 13:36:46 2019 @@ -901,7 +901,7 @@ ClangExpressionParser::ParseInternal(Dia } if (temp_fd != -1) { - lldb_private::File file(temp_fd, true); + lldb_private::File file(temp_fd, File::eOpenOptionWrite, true); const size_t expr_text_len = strlen(expr_text); size_t bytes_written = expr_text_len; if (file.Write(expr_text, bytes_written).Success()) { Modified: lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm?rev=372652&r1=372651&r2=372652&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm (original) +++ lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm Mon Sep 23 13:36:46 2019 @@ -425,11 +425,16 @@ static Status HandleFileAction(ProcessLa } } Status posix_error; + int oflag = file_action->GetActionArgument(); int created_fd = - open(file_spec.GetPath().c_str(), file_action->GetActionArgument(), - S_IRUSR | S_IWUSR); + open(file_spec.GetPath().c_str(), oflag, S_IRUSR | S_IWUSR); if (created_fd >= 0) { - file.SetDescriptor(created_fd, true); + uint32_t file_options = 0; + if ((oflag & O_RDWR) || (oflag & O_RDONLY)) + file_options |= File::eOpenOptionRead; + if ((oflag & O_RDWR) || (oflag & O_RDONLY)) + file_options |= File::eOpenOptionWrite; + file.SetDescriptor(created_fd, file_options, true); [options setValue:[NSNumber numberWithInteger:created_fd] forKey:key]; return error; // Success } else { Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp?rev=372652&r1=372651&r2=372652&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp Mon Sep 23 13:36:46 2019 @@ -537,7 +537,7 @@ GDBRemoteCommunicationServerCommon::Hand int err = -1; int save_errno = 0; if (fd >= 0) { - File file(fd, true); + File file(fd, 0, true); Status error = file.Close(); err = 0; save_errno = error.GetError(); @@ -568,7 +568,7 @@ GDBRemoteCommunicationServerCommon::Hand } std::string buffer(count, 0); - File file(fd, false); + File file(fd, File::eOpenOptionRead, false); Status error = file.Read(static_cast<void *>(&buffer[0]), count, offset); const ssize_t bytes_read = error.Success() ? count : -1; const int save_errno = error.GetError(); @@ -600,7 +600,7 @@ GDBRemoteCommunicationServerCommon::Hand if (packet.GetChar() == ',') { std::string buffer; if (packet.GetEscapedBinaryData(buffer)) { - File file(fd, false); + File file(fd, File::eOpenOptionWrite, false); size_t count = buffer.size(); Status error = file.Write(static_cast<const void *>(&buffer[0]), count, offset); Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp?rev=372652&r1=372651&r2=372652&view=diff ============================================================================== --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp (original) +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp Mon Sep 23 13:36:46 2019 @@ -1043,9 +1043,9 @@ bool PythonFile::GetUnderlyingFile(File file.Close(); // We don't own the file descriptor returned by this function, make sure the // File object knows about that. - file.SetDescriptor(PyObject_AsFileDescriptor(m_py_obj), false); PythonString py_mode = GetAttributeValue("mode").AsType<PythonString>(); - file.SetOptions(PythonFile::GetOptionsFromMode(py_mode.GetString())); + auto options = PythonFile::GetOptionsFromMode(py_mode.GetString()); + file.SetDescriptor(PyObject_AsFileDescriptor(m_py_obj), options, false); return file.IsValid(); } Modified: lldb/trunk/source/Target/Process.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=372652&r1=372651&r2=372652&view=diff ============================================================================== --- lldb/trunk/source/Target/Process.cpp (original) +++ lldb/trunk/source/Target/Process.cpp Mon Sep 23 13:36:46 2019 @@ -4299,9 +4299,10 @@ public: IOHandlerProcessSTDIO(Process *process, int write_fd) : IOHandler(process->GetTarget().GetDebugger(), IOHandler::Type::ProcessIO), - m_process(process), m_write_file(write_fd, false) { + m_process(process), + m_write_file(write_fd, File::eOpenOptionWrite, false) { m_pipe.CreateNew(false); - m_read_file.SetDescriptor(GetInputFD(), false); + m_read_file.SetDescriptor(GetInputFD(), File::eOpenOptionRead, false); } ~IOHandlerProcessSTDIO() override = default; Modified: lldb/trunk/unittests/Host/FileTest.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/FileTest.cpp?rev=372652&r1=372651&r2=372652&view=diff ============================================================================== --- lldb/trunk/unittests/Host/FileTest.cpp (original) +++ lldb/trunk/unittests/Host/FileTest.cpp Mon Sep 23 13:36:46 2019 @@ -34,3 +34,24 @@ TEST(File, GetWaitableHandleFileno) { File file(stream, true); EXPECT_EQ(file.GetWaitableHandle(), fd); } + +TEST(File, GetStreamFromDescriptor) { + const auto *Info = testing::UnitTest::GetInstance()->current_test_info(); + llvm::SmallString<128> name; + int fd; + llvm::sys::fs::createTemporaryFile(llvm::Twine(Info->test_case_name()) + "-" + + Info->name(), + "test", fd, name); + + llvm::FileRemover remover(name); + ASSERT_GE(fd, 0); + + File file(fd, File::eOpenOptionWrite, true); + ASSERT_TRUE(file.IsValid()); + + FILE *stream = file.GetStream(); + ASSERT_TRUE(stream != NULL); + + EXPECT_EQ(file.GetDescriptor(), fd); + EXPECT_EQ(file.GetWaitableHandle(), fd); +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
