Author: zturner Date: Thu Oct 15 14:35:48 2015 New Revision: 250444 URL: http://llvm.org/viewvc/llvm-project?rev=250444&view=rev Log: Introduce a `PythonFile` object, and use it everywhere.
Python file handling got an overhaul in Python 3, and it affects the way we have to interact with files. Notably: 1) `PyFile_FromFile` no longer exists, and instead we have to use `PyFile_FromFd`. This means having a way to get an fd from a FILE*. For this we reuse the lldb_private::File class to convert between FILE*s and fds, since there are some subtleties regarding ownership rules when FILE*s and fds refer to the same file. 2) PyFile is no longer a builtin type, so there is no such thing as `PyFile_Check`. Instead, files in Python 3 are just instances of `io.IOBase`. So the logic for checking if something is a file in Python 3 is to check if it is a subclass of that module. Additionally, some unit tests are added to verify that `PythonFile` works as expected on Python 2 and Python 3, and `ScriptInterpreterPython` is updated to use `PythonFile` instead of manual calls to the various `PyFile_XXX` methods. Modified: lldb/trunk/source/Host/common/File.cpp lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h lldb/trunk/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp Modified: lldb/trunk/source/Host/common/File.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/File.cpp?rev=250444&r1=250443&r2=250444&view=diff ============================================================================== --- lldb/trunk/source/Host/common/File.cpp (original) +++ lldb/trunk/source/Host/common/File.cpp Thu Oct 15 14:35:48 2015 @@ -143,7 +143,13 @@ File::GetDescriptor() const // Don't open the file descriptor if we don't need to, just get it from the // stream if we have one. if (StreamIsValid()) - return fileno (m_stream); + { +#if defined(LLVM_ON_WIN32) + return _fileno(m_stream); +#else + return fileno(m_stream); +#endif + } // Invalid descriptor and invalid stream, return invalid descriptor. return kInvalidDescriptor; 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=250444&r1=250443&r2=250444&view=diff ============================================================================== --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp (original) +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp Thu Oct 15 14:35:48 2015 @@ -75,6 +75,8 @@ PythonObject::GetObjectType() const return PyObjectType::String; if (PythonInteger::Check(m_py_obj)) return PyObjectType::Integer; + if (PythonFile::Check(m_py_obj)) + return PyObjectType::File; return PyObjectType::Unknown; } @@ -101,6 +103,15 @@ PythonObject::Str() } bool +PythonObject::HasAttribute(llvm::StringRef attr) const +{ + if (!IsValid()) + return false; + PythonString py_attr(attr); + return !!PyObject_HasAttr(m_py_obj, py_attr.get()); +} + +bool PythonObject::IsNone() const { return m_py_obj == Py_None; @@ -566,4 +577,78 @@ PythonDictionary::CreateStructuredDictio return result; } +PythonFile::PythonFile(File &file, const char *mode) +{ + Reset(file, mode); +} + +PythonFile::PythonFile(PyRefType type, PyObject *o) +{ + Reset(type, o); +} + +PythonFile::~PythonFile() +{ +} + +bool +PythonFile::Check(PyObject *py_obj) +{ +#if PY_MAJOR_VERSION < 3 + return PyFile_Check(py_obj); +#else + // In Python 3, there is no `PyFile_Check`, and in fact PyFile is not even a + // first-class object type anymore. `PyFile_FromFd` is just a thin wrapper + // over `io.open()`, which returns some object derived from `io.IOBase`. + // As a result, the only way to detect a file in Python 3 is to check whether + // it inherits from `io.IOBase`. Since it is possible for non-files to also + // inherit from `io.IOBase`, we additionally verify that it has the `fileno` + // attribute, which should guarantee that it is backed by the file system. + PythonObject io_module(PyRefType::Owned, PyImport_ImportModule("io")); + PythonDictionary io_dict(PyRefType::Borrowed, PyModule_GetDict(io_module.get())); + PythonObject io_base_class = io_dict.GetItemForKey(PythonString("IOBase")); + + PythonObject object_type(PyRefType::Owned, PyObject_Type(py_obj)); + + if (1 != PyObject_IsSubclass(object_type.get(), io_base_class.get())) + return false; + if (!object_type.HasAttribute("fileno")) + return false; + + return true; +#endif +} + +void +PythonFile::Reset(PyRefType type, PyObject *py_obj) +{ + // Grab the desired reference type so that if we end up rejecting + // `py_obj` it still gets decremented if necessary. + PythonObject result(type, py_obj); + + if (!PythonFile::Check(py_obj)) + { + PythonObject::Reset(); + return; + } + + // Calling PythonObject::Reset(const PythonObject&) will lead to stack + // overflow since it calls back into the virtual implementation. + PythonObject::Reset(PyRefType::Borrowed, result.get()); +} + +void +PythonFile::Reset(File &file, const char *mode) +{ + char *cmode = const_cast<char *>(mode); +#if PY_MAJOR_VERSION >= 3 + Reset(PyRefType::Owned, + PyFile_FromFd(file.GetDescriptor(), nullptr, cmode, -1, nullptr, "ignore", nullptr, 0)); +#else + // Read through the Python source, doesn't seem to modify these strings + Reset(PyRefType::Owned, + PyFile_FromFile(file.GetStream(), const_cast<char *>(""), cmode, nullptr)); +#endif +} + #endif Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h?rev=250444&r1=250443&r2=250444&view=diff ============================================================================== --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h (original) +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h Thu Oct 15 14:35:48 2015 @@ -19,6 +19,7 @@ #include "lldb/Core/ConstString.h" #include "lldb/Core/StructuredData.h" #include "lldb/Core/Flags.h" +#include "lldb/Host/File.h" #include "lldb/Interpreter/OptionValue.h" namespace lldb_private { @@ -67,7 +68,8 @@ enum class PyObjectType Integer, Dictionary, List, - String + String, + File }; enum class PyRefType @@ -197,6 +199,9 @@ public: } bool + HasAttribute(llvm::StringRef attribute) const; + + bool IsValid() const; bool @@ -315,6 +320,21 @@ public: StructuredData::DictionarySP CreateStructuredDictionary() const; }; +class PythonFile : public PythonObject +{ + public: + explicit PythonFile(File &file, const char *mode); + PythonFile(PyRefType type, PyObject *o); + ~PythonFile() override; + + static bool Check(PyObject *py_obj); + + using PythonObject::Reset; + + void Reset(PyRefType type, PyObject *py_obj) override; + void Reset(File &file, const char *mode); +}; + } // namespace lldb_private #endif // LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_PYTHONDATAOBJECTS_H Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp?rev=250444&r1=250443&r2=250444&view=diff ============================================================================== --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp (original) +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp Thu Oct 15 14:35:48 2015 @@ -542,26 +542,6 @@ ScriptInterpreterPython::LeaveSession () m_session_is_active = false; } -static PythonObject -PyFile_FromFile_Const(FILE *fp, const char *mode) -{ - char *cmode = const_cast<char*>(mode); -#if PY_MAJOR_VERSION >= 3 -#if defined(LLVM_ON_WIN32) - int fd = _fileno(fp); -#else - int fd = fileno(fp); -#endif - - return PythonObject(PyRefType::Owned, - PyFile_FromFd(fd, nullptr, cmode, -1, nullptr, "ignore", nullptr, 0)); -#else - // Read through the Python source, doesn't seem to modify these strings - return PythonObject(PyRefType::Owned, - PyFile_FromFile(fp, const_cast<char*>(""), cmode, nullptr)); -#endif -} - bool ScriptInterpreterPython::EnterSession (uint16_t on_entry_flags, FILE *in, @@ -610,10 +590,14 @@ ScriptInterpreterPython::EnterSession (u PythonDictionary &sys_module_dict = GetSysModuleDictionary (); if (sys_module_dict.IsValid()) { + File in_file(in, false); + File out_file(out, false); + File err_file(err, false); + lldb::StreamFileSP in_sp; lldb::StreamFileSP out_sp; lldb::StreamFileSP err_sp; - if (in == nullptr || out == nullptr || err == nullptr) + if (!in_file.IsValid() || !out_file.IsValid() || !err_file.IsValid()) m_interpreter.GetDebugger().AdoptTopIOHandlerFilesIfInvalid (in_sp, out_sp, err_sp); m_saved_stdin.Reset(); @@ -621,36 +605,45 @@ ScriptInterpreterPython::EnterSession (u if ((on_entry_flags & Locker::NoSTDIN) == 0) { // STDIN is enabled - if (in == nullptr && in_sp) - in = in_sp->GetFile().GetStream(); - if (in) + if (!in_file.IsValid() && in_sp) + in_file = in_sp->GetFile(); + if (in_file.IsValid()) { + // Flush the file before giving it to python to avoid interleaved output. + in_file.Flush(); + m_saved_stdin = sys_module_dict.GetItemForKey(PythonString("stdin")); // This call can deadlock your process if the file is locked - PythonObject new_file = PyFile_FromFile_Const(in, "r"); + PythonFile new_file(in_file, "r"); sys_module_dict.SetItemForKey (PythonString("stdin"), new_file); } } - if (out == nullptr && out_sp) - out = out_sp->GetFile().GetStream(); - if (out) + if (!out_file.IsValid() && out_sp) + out_file = out_sp->GetFile(); + if (out_file.IsValid()) { + // Flush the file before giving it to python to avoid interleaved output. + out_file.Flush(); + m_saved_stdout = sys_module_dict.GetItemForKey(PythonString("stdout")); - PythonObject new_file = PyFile_FromFile_Const(out, "w"); + PythonFile new_file(out_file, "w"); sys_module_dict.SetItemForKey (PythonString("stdout"), new_file); } else m_saved_stdout.Reset(); - if (err == nullptr && err_sp) - err = err_sp->GetFile().GetStream(); - if (err) + if (!err_file.IsValid() && err_sp) + err_file = err_sp->GetFile(); + if (err_file.IsValid()) { + // Flush the file before giving it to python to avoid interleaved output. + err_file.Flush(); + m_saved_stderr = sys_module_dict.GetItemForKey(PythonString("stderr")); - PythonObject new_file = PyFile_FromFile_Const(err, "w"); + PythonFile new_file(err_file, "w"); sys_module_dict.SetItemForKey (PythonString("stderr"), new_file); } else Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h?rev=250444&r1=250443&r2=250444&view=diff ============================================================================== --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h (original) +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h Thu Oct 15 14:35:48 2015 @@ -520,6 +520,7 @@ public: PyGILState_STATE m_GILState; }; protected: + enum class AddLocation { Beginning, Modified: lldb/trunk/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp?rev=250444&r1=250443&r2=250444&view=diff ============================================================================== --- lldb/trunk/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp (original) +++ lldb/trunk/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp Thu Oct 15 14:35:48 2015 @@ -9,6 +9,8 @@ #include "gtest/gtest.h" +#include "lldb/Host/File.h" +#include "lldb/Host/FileSystem.h" #include "lldb/Host/HostInfo.h" #include "Plugins/ScriptInterpreter/Python/lldb-python.h" #include "Plugins/ScriptInterpreter/Python/PythonDataObjects.h" @@ -373,3 +375,10 @@ TEST_F(PythonDataObjectsTest, TestPython EXPECT_STREQ(string_value0, string_sp->GetValue().c_str()); EXPECT_EQ(int_value1, int_sp->GetValue()); } + +TEST_F(PythonDataObjectsTest, TestPythonFile) +{ + File file(FileSystem::DEV_NULL, File::eOpenOptionRead); + PythonFile py_file(file, "r"); + EXPECT_TRUE(PythonFile::Check(py_file.get())); +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits