[Lldb-commits] [lldb] r250444 - Introduce a `PythonFile` object, and use it everywhere.
Zachary Turner via lldb-commits
lldb-commits at lists.llvm.org
Thu Oct 15 12:35:48 PDT 2015
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()));
+}
More information about the lldb-commits
mailing list