Index: source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h =================================================================== --- source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h +++ source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h @@ -25,7 +25,6 @@ class PythonString; class PythonList; class PythonDictionary; -class PythonObject; class PythonInteger; class StructuredPythonObject : public StructuredData::Generic @@ -71,50 +70,88 @@ String }; - class PythonObject +enum class PyRefType +{ + Borrowed, // We are not given ownership of the incoming PyObject. + // We cannot safely hold it without calling Py_INCREF. + Owned // We have ownership of the incoming PyObject. We should + // not call Py_INCREF. +}; + +enum class PyInitialValue +{ + Invalid, + Empty +}; + +class PyRef { public: - PythonObject () : - m_py_obj(NULL) + PyRef() + : m_py_obj(nullptr) { } - - explicit PythonObject (PyObject* py_obj) : - m_py_obj(NULL) + + PyRef(PyRefType type, PyObject *py_obj) + : m_py_obj(nullptr) { - Reset (py_obj); + Reset(type, py_obj); } - - PythonObject (const PythonObject &rhs) : - m_py_obj(NULL) + + PyRef(const PyRef &rhs) + : m_py_obj(nullptr) { - Reset (rhs.m_py_obj); + Reset(rhs); } - virtual - ~PythonObject () + virtual ~PyRef() { Reset(); } + + void + Reset() { - Reset (NULL); + // Avoid calling the virtual method since it's not necessary + // to actually validate the type of the PyObject if we're + // just setting to null. + if (Py_IsInitialized()) + Py_XDECREF(m_py_obj); + m_py_obj = nullptr; } - bool - Reset (const PythonObject &object) + void + Reset(const PyRef &rhs) { - return Reset(object.get()); + // Avoid calling the virtual method if it's not necessary + // to actually validate the type of the PyObject. + if (!rhs.get()) + Reset(); + else + Reset(PyRefType::Borrowed, rhs.m_py_obj); } - virtual bool - Reset (PyObject* py_obj = NULL) + // PyRef is implicitly convertible to PyObject *, which will call the + // wrong overload. We want to explicitly disallow this, since a PyObject + // *always* owns its reference. Therefore the overload which takes a + // PyRefType doesn't make sense, and the copy constructor should be used. + void + Reset(PyRefType type, const PyRef &ref) = delete; + + virtual void + Reset(PyRefType type, PyObject *py_obj) { - if (py_obj != m_py_obj) - { - if (Py_IsInitialized()) - Py_XDECREF(m_py_obj); - m_py_obj = py_obj; - if (Py_IsInitialized()) - Py_XINCREF(m_py_obj); - } - return true; + if (py_obj == m_py_obj) + return; + + if (Py_IsInitialized()) + Py_XDECREF(m_py_obj); + + m_py_obj = py_obj; + + // If this is a borrowed reference, we need to convert it to + // an owned reference by incrementing it. If it is an owned + // reference (for example the caller allocated it with PyDict_New() + // then we must *not* increment it. + if (Py_IsInitialized() && type == PyRefType::Borrowed) + Py_XINCREF(m_py_obj); } void @@ -142,12 +179,18 @@ PythonString Str (); - - explicit operator bool () const + + PyRef & + operator=(const PyRef &other) { - return m_py_obj != NULL; + Reset(PyRefType::Borrowed, other.get()); + return *this; } - + + operator bool() const { return m_py_obj != NULL; } + + operator PyObject *() const { return m_py_obj; } + bool IsNULLOrNone () const; @@ -157,20 +200,22 @@ PyObject* m_py_obj; }; - class PythonString: public PythonObject + class PythonString : public PyRef { public: - PythonString (); - PythonString (PyObject *o); - PythonString (const PythonObject &object); - PythonString (llvm::StringRef string); - PythonString (const char *string); - virtual ~PythonString (); + PythonString(); + PythonString(PyRefType type, PyObject *o); + PythonString(const PythonString &object); + explicit PythonString(llvm::StringRef string); + explicit PythonString(const char *string); + ~PythonString() override; static bool Check(PyObject *py_obj); - virtual bool - Reset (PyObject* py_obj = NULL); + // Bring in the no-argument base class version + using PyRef::Reset; + + void Reset(PyRefType type, PyObject *py_obj) override; llvm::StringRef GetString() const; @@ -183,20 +228,21 @@ StructuredData::StringSP CreateStructuredString() const; }; - class PythonInteger: public PythonObject + class PythonInteger : public PyRef { public: - - PythonInteger (); - PythonInteger (PyObject* py_obj); - PythonInteger (const PythonObject &object); - PythonInteger (int64_t value); - virtual ~PythonInteger (); + PythonInteger(); + PythonInteger(PyRefType type, PyObject *o); + PythonInteger(const PythonInteger &object); + explicit PythonInteger(int64_t value); + ~PythonInteger() override; static bool Check(PyObject *py_obj); - virtual bool - Reset (PyObject* py_obj = NULL); + // Bring in the no-argument base class version + using PyRef::Reset; + + void Reset(PyRefType type, PyObject *py_obj) override; int64_t GetInteger() const; @@ -205,76 +251,59 @@ StructuredData::IntegerSP CreateStructuredInteger() const; }; - - class PythonList: public PythonObject + + class PythonList : public PyRef { public: - PythonList(); - PythonList (PyObject* py_obj); - PythonList (const PythonObject &object); - virtual ~PythonList (); + PythonList(PyInitialValue value); + PythonList(PyRefType type, PyObject *o); + PythonList(const PythonList &list); + ~PythonList() override; static bool Check(PyObject *py_obj); - virtual bool - Reset (PyObject* py_obj = NULL); + // Bring in the no-argument base class version + using PyRef::Reset; + + void Reset(PyRefType type, PyObject *py_obj) override; uint32_t GetSize() const; - PythonObject GetItemAtIndex(uint32_t index) const; + PyRef GetItemAtIndex(uint32_t index) const; - void - SetItemAtIndex (uint32_t index, const PythonObject &object); - - void - AppendItem (const PythonObject &object); + void SetItemAtIndex(uint32_t index, const PyRef &object); + + void AppendItem(const PyRef &object); StructuredData::ArraySP CreateStructuredArray() const; }; - - class PythonDictionary: public PythonObject + + class PythonDictionary : public PyRef { public: - PythonDictionary(); - PythonDictionary (PyObject* object); - PythonDictionary (const PythonObject &object); - virtual ~PythonDictionary (); + PythonDictionary(PyInitialValue value); + PythonDictionary(PyRefType type, PyObject *o); + PythonDictionary(const PythonDictionary &dict); + ~PythonDictionary() override; static bool Check(PyObject *py_obj); - virtual bool - Reset (PyObject* object = NULL); + // Bring in the no-argument base class version + using PyRef::Reset; + + void Reset(PyRefType type, PyObject *py_obj) override; uint32_t GetSize() const; - PythonObject - GetItemForKey (const PythonString &key) const; - - const char * - GetItemForKeyAsString (const PythonString &key, const char *fail_value = NULL) const; + PyRef GetItemForKey(const PythonString &key) const; - int64_t - GetItemForKeyAsInteger (const PythonString &key, int64_t fail_value = 0) const; + PyRef GetItemForKey(const char *key) const; - PythonObject - GetItemForKey (const char *key) const; + PythonList GetKeys() const; - typedef bool (*DictionaryIteratorCallback)(PythonString* key, PythonDictionary* dict); - - PythonList - GetKeys () const; - - PythonString - GetKeyAtPosition (uint32_t pos) const; - - PythonObject - GetValueAtPosition (uint32_t pos) const; - - void - SetItemForKey (const PythonString &key, PyObject *value); + void SetItemForKey(const PythonString &key, PyObject *value); - void - SetItemForKey (const PythonString &key, const PythonObject& value); + void SetItemForKey(const char *key, PyObject *value); StructuredData::DictionarySP CreateStructuredDictionary() const; }; Index: source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp =================================================================== --- source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp +++ source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp @@ -33,11 +33,11 @@ } //---------------------------------------------------------------------- -// PythonObject +// PyRef //---------------------------------------------------------------------- void -PythonObject::Dump (Stream &strm) const +PyRef::Dump(Stream &strm) const { if (m_py_obj) { @@ -62,7 +62,7 @@ } PyObjectType -PythonObject::GetObjectType() const +PyRef::GetObjectType() const { if (IsNULLOrNone()) return PyObjectType::None; @@ -87,46 +87,46 @@ } PythonString -PythonObject::Repr () +PyRef::Repr() { if (!m_py_obj) - return PythonString (); + return PythonString(); PyObject *repr = PyObject_Repr(m_py_obj); if (!repr) - return PythonString (); - return PythonString(repr); + return PythonString(); + return PythonString(PyRefType::Owned, repr); } PythonString -PythonObject::Str () +PyRef::Str() { if (!m_py_obj) - return PythonString (); + return PythonString(); PyObject *str = PyObject_Str(m_py_obj); if (!str) - return PythonString (); - return PythonString(str); + return PythonString(); + return PythonString(PyRefType::Owned, str); } bool -PythonObject::IsNULLOrNone () const +PyRef::IsNULLOrNone() const { return ((m_py_obj == nullptr) || (m_py_obj == Py_None)); } StructuredData::ObjectSP -PythonObject::CreateStructuredObject() const +PyRef::CreateStructuredObject() const { switch (GetObjectType()) { case PyObjectType::Dictionary: - return PythonDictionary(m_py_obj).CreateStructuredDictionary(); + return PythonDictionary(PyRefType::Borrowed, m_py_obj).CreateStructuredDictionary(); case PyObjectType::Integer: - return PythonInteger(m_py_obj).CreateStructuredInteger(); + return PythonInteger(PyRefType::Borrowed, m_py_obj).CreateStructuredInteger(); case PyObjectType::List: - return PythonList(m_py_obj).CreateStructuredArray(); + return PythonList(PyRefType::Borrowed, m_py_obj).CreateStructuredArray(); case PyObjectType::String: - return PythonString(m_py_obj).CreateStructuredString(); + return PythonString(PyRefType::Borrowed, m_py_obj).CreateStructuredString(); case PyObjectType::None: return StructuredData::ObjectSP(); default: @@ -138,32 +138,31 @@ // PythonString //---------------------------------------------------------------------- -PythonString::PythonString (PyObject *py_obj) : - PythonObject() +PythonString::PythonString(PyRefType type, PyObject *py_obj) + : PyRef() { - Reset(py_obj); // Use "Reset()" to ensure that py_obj is a string + Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a string } -PythonString::PythonString (const PythonObject &object) : - PythonObject() +PythonString::PythonString(const PythonString &object) + : PyRef(object) { - Reset(object.get()); // Use "Reset()" to ensure that py_obj is a string } PythonString::PythonString(llvm::StringRef string) - : PythonObject() + : PyRef() { SetString(string); } PythonString::PythonString(const char *string) - : PythonObject() + : PyRef() { SetString(llvm::StringRef(string)); } -PythonString::PythonString () : - PythonObject() +PythonString::PythonString() + : PyRef() { } @@ -184,29 +183,34 @@ #endif } -bool -PythonString::Reset(PyObject *py_obj) +void +PythonString::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. + PyRef result(type, py_obj); + if (!PythonString::Check(py_obj)) { - PythonObject::Reset(nullptr); - return false; + PyRef::Reset(); + return; } -// Convert this to a PyBytes object, and only store the PyBytes. Note that in -// Python 2.x, PyString and PyUnicode are interchangeable, and PyBytes is an alias -// of PyString. So on 2.x, if we get into this branch, we already have a PyBytes. - //#if PY_MAJOR_VERSION >= 3 + // Convert this to a PyBytes object, and only store the PyBytes. Note that in + // Python 2.x, PyString and PyUnicode are interchangeable, and PyBytes is an alias + // of PyString. So on 2.x, if we get into this branch, we already have a PyBytes. if (PyUnicode_Check(py_obj)) { - PyObject *unicode = py_obj; - py_obj = PyUnicode_AsUTF8String(py_obj); - Py_XDECREF(unicode); + // Since we're converting this to a different object, we assume ownership of the + // new object regardless of the value of `type`. + result.Reset(PyRefType::Owned, PyUnicode_AsUTF8String(py_obj)); } - //#endif - assert(PyBytes_Check(py_obj) && "PythonString::Reset received a non-string"); - return PythonObject::Reset(py_obj); + assert(PyBytes_Check(result) && "PythonString::Reset received a non-string"); + + // Calling PyRef::Reset(const PyRef&) will lead to stack overflow since it calls + // back into the virtual implementation. + PyRef::Reset(PyRefType::Borrowed, result.get()); } llvm::StringRef @@ -236,10 +240,10 @@ #if PY_MAJOR_VERSION >= 3 PyObject *unicode = PyUnicode_FromStringAndSize(string.data(), string.size()); PyObject *bytes = PyUnicode_AsUTF8String(unicode); - PythonObject::Reset(bytes); - Py_XDECREF(unicode); + PyRef::Reset(PyRefType::Owned, bytes); + Py_DECREF(unicode); #else - PythonObject::Reset(PyString_FromStringAndSize(string.data(), string.size())); + PyRef::Reset(PyRefType::Owned, PyString_FromStringAndSize(string.data(), string.size())); #endif } @@ -255,22 +259,21 @@ // PythonInteger //---------------------------------------------------------------------- -PythonInteger::PythonInteger (PyObject *py_obj) : - PythonObject() +PythonInteger::PythonInteger(PyRefType type, PyObject *py_obj) + : PyRef() { - Reset(py_obj); // Use "Reset()" to ensure that py_obj is a integer type + Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a integer type } -PythonInteger::PythonInteger (const PythonObject &object) : - PythonObject() +PythonInteger::PythonInteger(const PythonInteger &object) + : PyRef(object) { - Reset(object.get()); // Use "Reset()" to ensure that py_obj is a integer type } -PythonInteger::PythonInteger (int64_t value) : - PythonObject() +PythonInteger::PythonInteger(int64_t value) + : PyRef() { - SetInteger (value); + SetInteger(value); } @@ -293,13 +296,17 @@ #endif } -bool -PythonInteger::Reset(PyObject *py_obj) +void +PythonInteger::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. + PyRef result(type, py_obj); + if (!PythonInteger::Check(py_obj)) { - PythonObject::Reset(nullptr); - return false; + PyRef::Reset(); + return; } #if PY_MAJOR_VERSION < 3 @@ -308,15 +315,18 @@ // since 3.x doesn't even have a PyInt. if (PyInt_Check(py_obj)) { - PyObject *py_long = PyLong_FromLongLong(PyInt_AsLong(py_obj)); - Py_XDECREF(py_obj); - py_obj = py_long; + // Since we converted the original object to a different type, the new + // object is an owned object regardless of the ownership semantics requested + // by the user. + result.Reset(PyRefType::Owned, PyLong_FromLongLong(PyInt_AsLong(py_obj))); } #endif - assert(PyLong_Check(py_obj) && "Couldn't get a PyLong from this PyObject"); + assert(PyLong_Check(result) && "Couldn't get a PyLong from this PyObject"); - return PythonObject::Reset(py_obj); + // Calling PyRef::Reset(const PyRef&) will lead to stack overflow since it calls + // back into the virtual implementation. + PyRef::Reset(PyRefType::Borrowed, result.get()); } int64_t @@ -332,9 +342,9 @@ } void -PythonInteger::SetInteger (int64_t value) +PythonInteger::SetInteger(int64_t value) { - PythonObject::Reset(PyLong_FromLongLong(value)); + PyRef::Reset(PyRefType::Owned, PyLong_FromLongLong(value)); } StructuredData::IntegerSP @@ -349,22 +359,22 @@ // PythonList //---------------------------------------------------------------------- -PythonList::PythonList() - : PythonObject(PyList_New(0)) +PythonList::PythonList(PyInitialValue value) + : PyRef() { + if (value == PyInitialValue::Empty) + Reset(PyRefType::Owned, PyList_New(0)); } -PythonList::PythonList (PyObject *py_obj) : - PythonObject() +PythonList::PythonList(PyRefType type, PyObject *py_obj) + : PyRef() { - Reset(py_obj); // Use "Reset()" to ensure that py_obj is a list + Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a list } - -PythonList::PythonList (const PythonObject &object) : - PythonObject() +PythonList::PythonList(const PythonList &list) + : PyRef(list) { - Reset(object.get()); // Use "Reset()" to ensure that py_obj is a list } PythonList::~PythonList () @@ -379,16 +389,22 @@ return PyList_Check(py_obj); } -bool -PythonList::Reset(PyObject *py_obj) +void +PythonList::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. + PyRef result(type, py_obj); + if (!PythonList::Check(py_obj)) { - PythonObject::Reset(nullptr); - return false; + PyRef::Reset(); + return; } - return PythonObject::Reset(py_obj); + // Calling PyRef::Reset(const PyRef&) will lead to stack overflow since it calls + // back into the virtual implementation. + PyRef::Reset(PyRefType::Borrowed, result.get()); } uint32_t @@ -399,23 +415,28 @@ return 0; } -PythonObject +PyRef PythonList::GetItemAtIndex(uint32_t index) const { if (m_py_obj) - return PythonObject(PyList_GetItem(m_py_obj, index)); - return PythonObject(); + return PyRef(PyRefType::Borrowed, PyList_GetItem(m_py_obj, index)); + return PyRef(); } void -PythonList::SetItemAtIndex (uint32_t index, const PythonObject & object) +PythonList::SetItemAtIndex(uint32_t index, const PyRef &object) { if (m_py_obj && object) + { + // PyList_SetItem is documented to "steal" a reference, so we need to + // convert it to an owned reference by incrementing it. + Py_INCREF(object.get()); PyList_SetItem(m_py_obj, index, object.get()); + } } void -PythonList::AppendItem (const PythonObject &object) +PythonList::AppendItem(const PyRef &object) { if (m_py_obj && object) PyList_Append(m_py_obj, object.get()); @@ -428,7 +449,7 @@ uint32_t count = GetSize(); for (uint32_t i = 0; i < count; ++i) { - PythonObject obj = GetItemAtIndex(i); + PyRef obj = GetItemAtIndex(i); result->AddItem(obj.CreateStructuredObject()); } return result; @@ -438,22 +459,22 @@ // PythonDictionary //---------------------------------------------------------------------- -PythonDictionary::PythonDictionary() - : PythonObject(PyDict_New()) +PythonDictionary::PythonDictionary(PyInitialValue value) + : PyRef() { + if (value == PyInitialValue::Empty) + Reset(PyRefType::Owned, PyDict_New()); } -PythonDictionary::PythonDictionary (PyObject *py_obj) : - PythonObject(py_obj) +PythonDictionary::PythonDictionary(PyRefType type, PyObject *py_obj) + : PyRef() { - Reset(py_obj); // Use "Reset()" to ensure that py_obj is a dictionary + Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a dictionary } - -PythonDictionary::PythonDictionary (const PythonObject &object) : - PythonObject() +PythonDictionary::PythonDictionary(const PythonDictionary &object) + : PyRef(object) { - Reset(object.get()); // Use "Reset()" to ensure that py_obj is a dictionary } PythonDictionary::~PythonDictionary () @@ -469,16 +490,22 @@ return PyDict_Check(py_obj); } -bool -PythonDictionary::Reset(PyObject *py_obj) +void +PythonDictionary::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. + PyRef result(type, py_obj); + if (!PythonDictionary::Check(py_obj)) { - PythonObject::Reset(nullptr); - return false; + PyRef::Reset(); + return; } - return PythonObject::Reset(py_obj); + // Calling PyRef::Reset(const PyRef&) will lead to stack overflow since it calls + // back into the virtual implementation. + PyRef::Reset(PyRefType::Borrowed, result.get()); } uint32_t @@ -489,110 +516,44 @@ return 0; } -PythonObject -PythonDictionary::GetItemForKey (const char *key) const +PyRef +PythonDictionary::GetItemForKey(const char *key) const { if (key && key[0]) { PythonString python_key(key); return GetItemForKey(python_key); } - return PythonObject(); + return PyRef(); } - -PythonObject -PythonDictionary::GetItemForKey (const PythonString &key) const +PyRef +PythonDictionary::GetItemForKey(const PythonString &key) const { if (m_py_obj && key) - return PythonObject(PyDict_GetItem(m_py_obj, key.get())); - return PythonObject(); -} - - -const char * -PythonDictionary::GetItemForKeyAsString (const PythonString &key, const char *fail_value) const -{ - if (m_py_obj && key) - { - PyObject *py_obj = PyDict_GetItem(m_py_obj, key.get()); - if (py_obj && PythonString::Check(py_obj)) - { - PythonString str(py_obj); - return str.GetString().data(); - } - } - return fail_value; -} - -int64_t -PythonDictionary::GetItemForKeyAsInteger (const PythonString &key, int64_t fail_value) const -{ - if (m_py_obj && key) - { - PyObject *py_obj = PyDict_GetItem(m_py_obj, key.get()); - if (PythonInteger::Check(py_obj)) - { - PythonInteger int_obj(py_obj); - return int_obj.GetInteger(); - } - } - return fail_value; + return PyRef(PyRefType::Borrowed, PyDict_GetItem(m_py_obj, key.get())); + return PyRef(); } PythonList PythonDictionary::GetKeys () const { if (m_py_obj) - return PythonList(PyDict_Keys(m_py_obj)); - return PythonList(); -} - -PythonString -PythonDictionary::GetKeyAtPosition (uint32_t pos) const -{ - PyObject *key, *value; - Py_ssize_t pos_iter = 0; - - if (m_py_obj) - { - while (PyDict_Next(m_py_obj, &pos_iter, &key, &value)) - { - if (pos-- == 0) - return PythonString(key); - } - } - return PythonString(); -} - -PythonObject -PythonDictionary::GetValueAtPosition (uint32_t pos) const -{ - PyObject *key, *value; - Py_ssize_t pos_iter = 0; - - if (!m_py_obj) - return PythonObject(); - - while (PyDict_Next(m_py_obj, &pos_iter, &key, &value)) { - if (pos-- == 0) - return PythonObject(value); - } - return PythonObject(); + return PythonList(PyRefType::Owned, PyDict_Keys(m_py_obj)); + return PythonList(PyInitialValue::Invalid); } void -PythonDictionary::SetItemForKey (const PythonString &key, PyObject *value) +PythonDictionary::SetItemForKey(const PythonString &key, PyObject *value) { if (m_py_obj && key && value) - PyDict_SetItem(m_py_obj, key.get(), value); + PyDict_SetItem(m_py_obj, key, value); } void -PythonDictionary::SetItemForKey (const PythonString &key, const PythonObject &value) +PythonDictionary::SetItemForKey(const char *key, PyObject *value) { - if (m_py_obj && key && value) - PyDict_SetItem(m_py_obj, key.get(), value.get()); + SetItemForKey(PythonString(key), value); } StructuredData::DictionarySP @@ -603,9 +564,9 @@ uint32_t num_keys = keys.GetSize(); for (uint32_t i = 0; i < num_keys; ++i) { - PythonObject key = keys.GetItemAtIndex(i); + PyRef key = keys.GetItemAtIndex(i); PythonString key_str = key.Str(); - PythonObject value = GetItemForKey(key); + PyRef value = GetItemForKey(key_str); StructuredData::ObjectSP structured_value = value.CreateStructuredObject(); result->AddItem(key_str.GetString(), structured_value); } Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h =================================================================== --- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h +++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h @@ -553,9 +553,9 @@ eIOHandlerBreakpoint, eIOHandlerWatchpoint }; - PythonObject & - GetMainModule (); - + + PyRef &GetMainModule(); + PythonDictionary & GetSessionDictionary (); @@ -564,16 +564,16 @@ bool GetEmbeddedInterpreterModuleObjects (); - - PythonObject m_saved_stdin; - PythonObject m_saved_stdout; - PythonObject m_saved_stderr; - PythonObject m_main_module; - PythonObject m_lldb_module; + + PyRef m_saved_stdin; + PyRef m_saved_stdout; + PyRef m_saved_stderr; + PyRef m_main_module; + PyRef m_lldb_module; PythonDictionary m_session_dict; PythonDictionary m_sys_module_dict; - PythonObject m_run_one_line_function; - PythonObject m_run_one_line_str_global; + PyRef m_run_one_line_function; + PyRef m_run_one_line_str_global; std::string m_dictionary_name; TerminalState m_terminal_state; ActiveIOHandler m_active_io_handler; Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp =================================================================== --- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -103,8 +103,7 @@ #endif } -static std::string -ReadPythonBacktrace (PyObject* py_backtrace); +static std::string ReadPythonBacktrace(PyRef py_backtrace); ScriptInterpreterPython::Locker::Locker (ScriptInterpreterPython *py_interpreter, uint16_t on_entry, @@ -180,27 +179,26 @@ DoFreeLock(); } - -ScriptInterpreterPython::ScriptInterpreterPython (CommandInterpreter &interpreter) : - ScriptInterpreter (interpreter, eScriptLanguagePython), - IOHandlerDelegateMultiline("DONE"), - m_saved_stdin (), - m_saved_stdout (), - m_saved_stderr (), - m_main_module (), - m_lldb_module (), - m_session_dict(nullptr), - m_sys_module_dict(nullptr), - m_run_one_line_function (), - m_run_one_line_str_global (), - m_dictionary_name (interpreter.GetDebugger().GetInstanceName().AsCString()), - m_terminal_state (), - m_active_io_handler (eIOHandlerNone), - m_session_is_active (false), - m_pty_slave_is_open (false), - m_valid_session (true), - m_lock_count (0), - m_command_thread_state (nullptr) +ScriptInterpreterPython::ScriptInterpreterPython(CommandInterpreter &interpreter) + : ScriptInterpreter(interpreter, eScriptLanguagePython) + , IOHandlerDelegateMultiline("DONE") + , m_saved_stdin() + , m_saved_stdout() + , m_saved_stderr() + , m_main_module() + , m_lldb_module() + , m_session_dict(PyInitialValue::Invalid) + , m_sys_module_dict(PyInitialValue::Invalid) + , m_run_one_line_function() + , m_run_one_line_str_global() + , m_dictionary_name(interpreter.GetDebugger().GetInstanceName().AsCString()) + , m_terminal_state() + , m_active_io_handler(eIOHandlerNone) + , m_session_is_active(false) + , m_pty_slave_is_open(false) + , m_valid_session(true) + , m_lock_count(0) + , m_command_thread_state(nullptr) { assert(g_initialized && "ScriptInterpreterPython created but InitializePrivate has not been called!"); @@ -469,11 +467,12 @@ m_session_is_active = false; } -static PyObject * +static PyRef PyFile_FromFile_Const(FILE *fp, const char *name, const char *mode, int (*close)(FILE *)) { // Read through the Python source, doesn't seem to modify these strings - return PyFile_FromFile(fp, const_cast(name), const_cast(mode), close); + return PyRef(PyRefType::Owned, + PyFile_FromFile(fp, const_cast(name), const_cast(mode), close)); } bool @@ -539,11 +538,10 @@ in = in_sp->GetFile().GetStream(); if (in) { - m_saved_stdin.Reset(sys_module_dict.GetItemForKey("stdin")); + m_saved_stdin = sys_module_dict.GetItemForKey("stdin"); // This call can deadlock your process if the file is locked - PyObject *new_file = PyFile_FromFile_Const (in, "", "r", nullptr); + PyRef new_file = PyFile_FromFile_Const(in, "", "r", nullptr); sys_module_dict.SetItemForKey ("stdin", new_file); - Py_DECREF (new_file); } } @@ -551,11 +549,10 @@ out = out_sp->GetFile().GetStream(); if (out) { - m_saved_stdout.Reset(sys_module_dict.GetItemForKey("stdout")); + m_saved_stdout = sys_module_dict.GetItemForKey("stdout"); - PyObject *new_file = PyFile_FromFile_Const (out, "", "w", nullptr); + PyRef new_file = PyFile_FromFile_Const(out, "", "w", nullptr); sys_module_dict.SetItemForKey ("stdout", new_file); - Py_DECREF (new_file); } else m_saved_stdout.Reset(); @@ -564,11 +561,10 @@ err = err_sp->GetFile().GetStream(); if (err) { - m_saved_stderr.Reset(sys_module_dict.GetItemForKey("stderr")); + m_saved_stderr = sys_module_dict.GetItemForKey("stderr"); - PyObject *new_file = PyFile_FromFile_Const (err, "", "w", nullptr); + PyRef new_file = PyFile_FromFile_Const(err, "", "w", nullptr); sys_module_dict.SetItemForKey ("stderr", new_file); - Py_DECREF (new_file); } else m_saved_stderr.Reset(); @@ -580,11 +576,11 @@ return true; } -PythonObject & -ScriptInterpreterPython::GetMainModule () +PyRef & +ScriptInterpreterPython::GetMainModule() { if (!m_main_module) - m_main_module.Reset(PyImport_AddModule ("__main__")); + m_main_module.Reset(PyRefType::Borrowed, PyImport_AddModule("__main__")); return m_main_module; } @@ -593,13 +589,14 @@ { if (!m_session_dict) { - PythonObject &main_module = GetMainModule (); + PyRef &main_module = GetMainModule(); if (main_module) { - PythonDictionary main_dict(PyModule_GetDict (main_module.get())); + PythonDictionary main_dict(PyRefType::Borrowed, PyModule_GetDict(main_module.get())); if (main_dict) { - m_session_dict = main_dict.GetItemForKey(m_dictionary_name.c_str()); + PyRef item = main_dict.GetItemForKey(m_dictionary_name.c_str()); + m_session_dict.Reset(PyRefType::Borrowed, item.get()); } } } @@ -611,9 +608,9 @@ { if (!m_sys_module_dict) { - PyObject *sys_module = PyImport_AddModule ("sys"); + PyRef sys_module(PyRefType::Borrowed, PyImport_AddModule("sys")); if (sys_module) - m_sys_module_dict.Reset(PyModule_GetDict (sys_module)); + m_sys_module_dict.Reset(PyRefType::Borrowed, PyModule_GetDict(sys_module)); } return m_sys_module_dict; } @@ -639,20 +636,20 @@ bool ScriptInterpreterPython::GetEmbeddedInterpreterModuleObjects () { - if (!m_run_one_line_function) - { - PyObject *module = PyImport_AddModule ("lldb.embedded_interpreter"); - if (module != nullptr) - { - PythonDictionary module_dict (PyModule_GetDict (module)); - if (module_dict) - { - m_run_one_line_function = module_dict.GetItemForKey("run_one_line"); - m_run_one_line_str_global = module_dict.GetItemForKey("g_run_one_line_str"); - } - } - } - return (bool)m_run_one_line_function; + if (m_run_one_line_function) + return true; + + PyRef module(PyRefType::Borrowed, PyImport_AddModule ("lldb.embedded_interpreter")); + if (!module) + return false; + + PythonDictionary module_dict(PyRefType::Borrowed, PyModule_GetDict(module)); + if (!module_dict) + return false; + + m_run_one_line_function = module_dict.GetItemForKey("run_one_line"); + m_run_one_line_str_global = module_dict.GetItemForKey("g_run_one_line_str"); + return m_run_one_line_function; } static void @@ -754,14 +751,13 @@ { if (GetEmbeddedInterpreterModuleObjects ()) { - PyObject *pfunc = m_run_one_line_function.get(); - - if (pfunc && PyCallable_Check (pfunc)) + if (PyCallable_Check(m_run_one_line_function)) { - PythonObject pargs (Py_BuildValue("(Os)", session_dict.get(), command)); + PyRef pargs(PyRefType::Owned, Py_BuildValue("(Os)", session_dict.get(), command)); if (pargs) { - PythonObject return_value(PyObject_CallObject (pfunc, pargs.get())); + PyRef return_value(PyRefType::Owned, + PyObject_CallObject(m_run_one_line_function, pargs)); if (return_value) success = true; else if (options.GetMaskoutErrors() && PyErr_Occurred ()) @@ -961,10 +957,10 @@ ScriptInterpreterPython::Locker::AcquireLock | ScriptInterpreterPython::Locker::InitSession | (options.GetSetLLDBGlobals() ? ScriptInterpreterPython::Locker::InitGlobals : 0) | Locker::NoSTDIN, ScriptInterpreterPython::Locker::FreeAcquiredLock | ScriptInterpreterPython::Locker::TearDownSession); - PyObject *py_return = nullptr; - PythonObject &main_module = GetMainModule (); - PythonDictionary globals (PyModule_GetDict(main_module.get())); - PyObject *py_error = nullptr; + PyRef py_return; + PyRef &main_module = GetMainModule(); + PythonDictionary globals(PyRefType::Borrowed, PyModule_GetDict(main_module)); + PyRef py_error; bool ret_success = false; int success; @@ -972,32 +968,32 @@ if (!locals) { - locals = PyObject_GetAttrString (globals.get(), m_dictionary_name.c_str()); + locals.Reset(PyRefType::Owned, PyObject_GetAttrString(globals, m_dictionary_name.c_str())); } if (!locals) locals = globals; - py_error = PyErr_Occurred(); - if (py_error != nullptr) + py_error.Reset(PyRefType::Borrowed, PyErr_Occurred()); + if (!py_error) PyErr_Clear(); if (in_string != nullptr) { { // scope for PythonInputReaderManager //PythonInputReaderManager py_input(options.GetEnableIO() ? this : NULL); - py_return = PyRun_String (in_string, Py_eval_input, globals.get(), locals.get()); - if (py_return == nullptr) - { - py_error = PyErr_Occurred (); - if (py_error != nullptr) - PyErr_Clear (); - - py_return = PyRun_String (in_string, Py_single_input, globals.get(), locals.get()); + py_return.Reset(PyRefType::Owned, PyRun_String(in_string, Py_eval_input, globals, locals)); + if (!py_return) + { + py_error.Reset(PyRefType::Borrowed, PyErr_Occurred()); + if (!py_error) + PyErr_Clear(); + + py_return.Reset(PyRefType::Owned, PyRun_String(in_string, Py_single_input, globals, locals)); } } - if (py_return != nullptr) + if (py_return) { switch (return_type) { @@ -1088,12 +1084,13 @@ case eScriptReturnTypeOpaqueObject: { success = true; - Py_XINCREF(py_return); - *((PyObject**)ret_value) = py_return; + PyObject *saved_value = py_return; + Py_XINCREF(saved_value); + *((PyObject **)ret_value) = saved_value; break; } } - Py_XDECREF (py_return); + if (success) ret_success = true; else @@ -1101,8 +1098,8 @@ } } - py_error = PyErr_Occurred(); - if (py_error != nullptr) + py_error.Reset(PyRefType::Borrowed, PyErr_Occurred()); + if (py_error) { ret_success = false; if (options.GetMaskoutErrors()) @@ -1125,72 +1122,74 @@ ScriptInterpreterPython::Locker::AcquireLock | ScriptInterpreterPython::Locker::InitSession | (options.GetSetLLDBGlobals() ? ScriptInterpreterPython::Locker::InitGlobals : 0) | Locker::NoSTDIN, ScriptInterpreterPython::Locker::FreeAcquiredLock | ScriptInterpreterPython::Locker::TearDownSession); - PythonObject return_value; - PythonObject &main_module = GetMainModule (); - PythonDictionary globals (PyModule_GetDict(main_module.get())); - PyObject *py_error = nullptr; + PyRef return_value; + PyRef &main_module = GetMainModule(); + PythonDictionary globals(PyRefType::Borrowed, PyModule_GetDict(main_module.get())); + PyRef py_error; + + PythonDictionary locals = GetSessionDictionary(); - PythonDictionary locals = GetSessionDictionary (); - if (!locals) { - locals = PyObject_GetAttrString (globals.get(), m_dictionary_name.c_str()); + locals.Reset(PyRefType::Owned, PyObject_GetAttrString(globals.get(), m_dictionary_name.c_str())); } if (!locals) - { locals = globals; - } - py_error = PyErr_Occurred(); - if (py_error != nullptr) + py_error.Reset(PyRefType::Borrowed, PyErr_Occurred()); + if (py_error) PyErr_Clear(); if (in_string != nullptr) { + PyRef code_object; + code_object.Reset(PyRefType::Owned, Py_CompileString(in_string, "temp.py", Py_file_input)); + + if (code_object) + { + // In Python 2.x, PyEval_EvalCode takes a PyCodeObject, but in Python 3.x, it takes + // a PyObject. They are convertible (hence the function PyCode_Check(PyObject*), so + // we have to do the cast for Python 2.x #if PY_MAJOR_VERSION >= 3 - PyObject *code_object = Py_CompileString(in_string, "temp.py", Py_file_input); + PyObject *py_code_obj = code_object; #else - PyCodeObject *code_object = nullptr; - struct _node *compiled_node = PyParser_SimpleParseString(in_string, Py_file_input); - if (compiled_node) - code_object = PyNode_Compile(compiled_node, "temp.py"); + PyCodeObject *py_code_obj = reinterpret_cast(code_object.get()); #endif - if (code_object) - return_value.Reset(PyEval_EvalCode(code_object, globals.get(), locals.get())); + return_value.Reset(PyRefType::Owned, PyEval_EvalCode(py_code_obj, globals, locals)); + } } - py_error = PyErr_Occurred (); - if (py_error != nullptr) + py_error.Reset(PyRefType::Borrowed, PyErr_Occurred()); + if (py_error) { -// puts(in_string); -// _PyObject_Dump (py_error); -// PyErr_Print(); -// success = false; - - PyObject *type = nullptr; - PyObject *value = nullptr; - PyObject *traceback = nullptr; - PyErr_Fetch (&type,&value,&traceback); - + // puts(in_string); + // _PyObject_Dump (py_error); + // PyErr_Print(); + // success = false; + + PyObject *py_type = nullptr; + PyObject *py_value = nullptr; + PyObject *py_traceback = nullptr; + PyErr_Fetch(&py_type, &py_value, &py_traceback); + + PyRef type(PyRefType::Owned, py_type); + PyRef value(PyRefType::Owned, py_value); + PyRef traceback(PyRefType::Owned, py_traceback); + // get the backtrace std::string bt = ReadPythonBacktrace(traceback); - - if (value && value != Py_None) + + if (!value.IsNULLOrNone()) { - PythonString str(value); + PythonString str = value.Str(); llvm::StringRef value_str(str.GetString()); error.SetErrorStringWithFormat("%s\n%s", value_str.str().c_str(), bt.c_str()); } else error.SetErrorStringWithFormat("%s",bt.c_str()); - Py_XDECREF(type); - Py_XDECREF(value); - Py_XDECREF(traceback); if (options.GetMaskoutErrors()) - { PyErr_Clear(); - } } return error; @@ -1462,53 +1461,42 @@ if (!generic) return nullptr; - PyObject *implementor = (PyObject *)generic->GetValue(); + PyRef implementor(PyRefType::Borrowed, (PyObject *)generic->GetValue()); - if (implementor == nullptr || implementor == Py_None) + if (implementor.IsNULLOrNone()) return StructuredData::DictionarySP(); - PyObject* pmeth = PyObject_GetAttrString(implementor, callee_name); - + PyRef pmeth(PyRefType::Owned, PyObject_GetAttrString(implementor, callee_name)); + if (PyErr_Occurred()) - { PyErr_Clear(); - } - - if (pmeth == nullptr || pmeth == Py_None) - { - Py_XDECREF(pmeth); + + if (pmeth.IsNULLOrNone()) return StructuredData::DictionarySP(); - } if (PyCallable_Check(pmeth) == 0) { if (PyErr_Occurred()) - { PyErr_Clear(); - } - Py_XDECREF(pmeth); return StructuredData::DictionarySP(); } if (PyErr_Occurred()) - { PyErr_Clear(); - } - - Py_XDECREF(pmeth); // right now we know this function exists and is callable.. - PyObject* py_return = PyObject_CallMethod(implementor, callee_name, nullptr); - + PyRef py_return(PyRefType::Owned, PyObject_CallMethod(implementor, callee_name, nullptr)); + // if it fails, print the error but otherwise go on if (PyErr_Occurred()) { PyErr_Print(); PyErr_Clear(); } + assert(PythonDictionary::Check(py_return) && "get_register_info returned unknown object type!"); - PythonDictionary result_dict(py_return); + PythonDictionary result_dict(PyRefType::Borrowed, py_return); return result_dict.CreateStructuredDictionary(); } @@ -1527,45 +1515,34 @@ StructuredData::Generic *generic = os_plugin_object_sp->GetAsGeneric(); if (!generic) return nullptr; - PyObject *implementor = (PyObject *)generic->GetValue(); - if (implementor == nullptr || implementor == Py_None) + PyRef implementor(PyRefType::Borrowed, (PyObject *)generic->GetValue()); + + if (implementor.IsNULLOrNone()) return StructuredData::ArraySP(); - PyObject* pmeth = PyObject_GetAttrString(implementor, callee_name); - + PyRef pmeth(PyRefType::Owned, PyObject_GetAttrString(implementor, callee_name)); + if (PyErr_Occurred()) - { PyErr_Clear(); - } - - if (pmeth == nullptr || pmeth == Py_None) - { - Py_XDECREF(pmeth); + + if (pmeth.IsNULLOrNone()) return StructuredData::ArraySP(); - } if (PyCallable_Check(pmeth) == 0) { if (PyErr_Occurred()) - { PyErr_Clear(); - } - Py_XDECREF(pmeth); return StructuredData::ArraySP(); } if (PyErr_Occurred()) - { PyErr_Clear(); - } - - Py_XDECREF(pmeth); // right now we know this function exists and is callable.. - PyObject* py_return = PyObject_CallMethod(implementor, callee_name, nullptr); - + PyRef py_return(PyRefType::Owned, PyObject_CallMethod(implementor, callee_name, nullptr)); + // if it fails, print the error but otherwise go on if (PyErr_Occurred()) { @@ -1573,8 +1550,10 @@ PyErr_Clear(); } - PythonList ResultList(py_return); - return ResultList.CreateStructuredArray(); + assert(PythonList::Check(py_return) && "get_thread_info returned unknown object type!"); + + PythonList result_list(PyRefType::Borrowed, py_return); + return result_list.CreateStructuredArray(); } // GetPythonValueFormatString provides a system independent type safe way to @@ -1619,44 +1598,31 @@ StructuredData::Generic *generic = os_plugin_object_sp->GetAsGeneric(); if (!generic) return nullptr; - PyObject *implementor = (PyObject *)generic->GetValue(); + PyRef implementor(PyRefType::Borrowed, (PyObject *)generic->GetValue()); - if (implementor == nullptr || implementor == Py_None) + if (implementor.IsNULLOrNone()) return StructuredData::StringSP(); - PyObject* pmeth = PyObject_GetAttrString(implementor, callee_name); - + PyRef pmeth(PyRefType::Owned, PyObject_GetAttrString(implementor, callee_name)); + if (PyErr_Occurred()) - { PyErr_Clear(); - } - - if (pmeth == nullptr || pmeth == Py_None) - { - Py_XDECREF(pmeth); + + if (pmeth.IsNULLOrNone()) return StructuredData::StringSP(); - } if (PyCallable_Check(pmeth) == 0) { if (PyErr_Occurred()) - { PyErr_Clear(); - } - - Py_XDECREF(pmeth); return StructuredData::StringSP(); } if (PyErr_Occurred()) - { PyErr_Clear(); - } - - Py_XDECREF(pmeth); // right now we know this function exists and is callable.. - PyObject* py_return = PyObject_CallMethod(implementor, callee_name, param_format, tid); + PyRef py_return(PyRefType::Owned, PyObject_CallMethod(implementor, callee_name, param_format, tid)); // if it fails, print the error but otherwise go on if (PyErr_Occurred()) @@ -1664,7 +1630,10 @@ PyErr_Print(); PyErr_Clear(); } - PythonString result_string(py_return); + + assert(PythonString::Check(py_return) && "get_register_data returned unknown object type!"); + + PythonString result_string(PyRefType::Borrowed, py_return); return result_string.CreateStructuredString(); } @@ -1686,45 +1655,33 @@ StructuredData::Generic *generic = os_plugin_object_sp->GetAsGeneric(); if (!generic) return nullptr; - PyObject *implementor = (PyObject *)generic->GetValue(); - if (implementor == nullptr || implementor == Py_None) + PyRef implementor(PyRefType::Borrowed, (PyObject *)generic->GetValue()); + + if (implementor.IsNULLOrNone()) return StructuredData::DictionarySP(); - PyObject* pmeth = PyObject_GetAttrString(implementor, callee_name); - + PyRef pmeth(PyRefType::Owned, PyObject_GetAttrString(implementor, callee_name)); + if (PyErr_Occurred()) - { PyErr_Clear(); - } - - if (pmeth == nullptr || pmeth == Py_None) - { - Py_XDECREF(pmeth); + + if (pmeth.IsNULLOrNone()) return StructuredData::DictionarySP(); - } if (PyCallable_Check(pmeth) == 0) { if (PyErr_Occurred()) - { PyErr_Clear(); - } - - Py_XDECREF(pmeth); return StructuredData::DictionarySP(); } if (PyErr_Occurred()) - { PyErr_Clear(); - } - - Py_XDECREF(pmeth); // right now we know this function exists and is callable.. - PyObject* py_return = PyObject_CallMethod(implementor, callee_name, ¶m_format[0], tid, context); - + PyRef py_return(PyRefType::Owned, PyObject_CallMethod(implementor, callee_name, ¶m_format[0], tid, context)); + // if it fails, print the error but otherwise go on if (PyErr_Occurred()) { @@ -1732,7 +1689,9 @@ PyErr_Clear(); } - PythonDictionary result_dict(py_return); + assert(PythonDictionary::Check(py_return) && "create_thread returned unknown object type!"); + + PythonDictionary result_dict(PyRefType::Borrowed, py_return); return result_dict.CreateStructuredDictionary(); } @@ -1846,16 +1805,15 @@ if (!generic) return StructuredData::DictionarySP(); - PyObject *reply_pyobj = nullptr; - + PyRef reply_pyobj; { Locker py_lock(this, Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN); TargetSP target_sp(target->shared_from_this()); - reply_pyobj = (PyObject *)g_swig_plugin_get(generic->GetValue(), setting_name, target_sp); + reply_pyobj.Reset(PyRefType::Owned, + (PyObject *)g_swig_plugin_get(generic->GetValue(), setting_name, target_sp)); } - PythonDictionary py_dict(reply_pyobj); - + PythonDictionary py_dict(PyRefType::Borrowed, reply_pyobj); return py_dict.CreateStructuredDictionary(); } @@ -2335,63 +2293,54 @@ } static std::string -ReadPythonBacktrace (PyObject* py_backtrace) -{ - PyObject* traceback_module = nullptr, - *stringIO_module = nullptr, - *stringIO_builder = nullptr, - *stringIO_buffer = nullptr, - *printTB = nullptr, - *printTB_args = nullptr, - *printTB_result = nullptr, - *stringIO_getvalue = nullptr, - *printTB_string = nullptr; +ReadPythonBacktrace(PyRef py_backtrace) +{ + PyRef traceback_module; + PyRef stringIO_module; + PyRef stringIO_builder; + PyRef stringIO_buffer; + PyRef printTB; + PyRef printTB_args; + PyRef printTB_result; + PyRef stringIO_getvalue; + PyRef printTB_string; std::string retval("backtrace unavailable"); - - if (py_backtrace && py_backtrace != Py_None) - { - traceback_module = PyImport_ImportModule("traceback"); - stringIO_module = PyImport_ImportModule("StringIO"); - - if (traceback_module && traceback_module != Py_None && stringIO_module && stringIO_module != Py_None) - { - stringIO_builder = PyObject_GetAttrString(stringIO_module, "StringIO"); - if (stringIO_builder && stringIO_builder != Py_None) - { - stringIO_buffer = PyObject_CallObject(stringIO_builder, nullptr); - if (stringIO_buffer && stringIO_buffer != Py_None) - { - printTB = PyObject_GetAttrString(traceback_module, "print_tb"); - if (printTB && printTB != Py_None) - { - printTB_args = Py_BuildValue("OOO",py_backtrace,Py_None,stringIO_buffer); - printTB_result = PyObject_CallObject(printTB, printTB_args); - stringIO_getvalue = PyObject_GetAttrString(stringIO_buffer, "getvalue"); - if (stringIO_getvalue && stringIO_getvalue != Py_None) - { - printTB_string = PyObject_CallObject (stringIO_getvalue,nullptr); - if (printTB_string && PythonString::Check(printTB_string)) - { - PythonString str(printTB_string); - llvm::StringRef string_data(str.GetString()); - retval.assign(string_data.data(), string_data.size()); - } - } - } - } - } - } - } - Py_XDECREF(traceback_module); - Py_XDECREF(stringIO_module); - Py_XDECREF(stringIO_builder); - Py_XDECREF(stringIO_buffer); - Py_XDECREF(printTB); - Py_XDECREF(printTB_args); - Py_XDECREF(printTB_result); - Py_XDECREF(stringIO_getvalue); - Py_XDECREF(printTB_string); + + if (py_backtrace.IsNULLOrNone()) + return retval; + + traceback_module.Reset(PyRefType::Owned, PyImport_ImportModule("traceback")); + stringIO_module.Reset(PyRefType::Owned, PyImport_ImportModule("StringIO")); + if (traceback_module.IsNULLOrNone() || stringIO_module.IsNULLOrNone()) + return retval; + + stringIO_builder.Reset(PyRefType::Owned, PyObject_GetAttrString(stringIO_module, "StringIO")); + if (stringIO_builder.IsNULLOrNone()) + return retval; + + stringIO_buffer.Reset(PyRefType::Owned, PyObject_CallObject(stringIO_builder, nullptr)); + if (stringIO_buffer.IsNULLOrNone()) + return retval; + + printTB.Reset(PyRefType::Owned, PyObject_GetAttrString(traceback_module, "print_tb")); + if (printTB.IsNULLOrNone()) + return retval; + + printTB_args.Reset(PyRefType::Owned, Py_BuildValue("OOO", py_backtrace.get(), Py_None, stringIO_buffer.get())); + printTB_result.Reset(PyRefType::Owned, PyObject_CallObject(printTB, printTB_args)); + stringIO_getvalue.Reset(PyRefType::Owned, PyObject_GetAttrString(stringIO_buffer, "getvalue")); + if (stringIO_getvalue.IsNULLOrNone()) + return retval; + + printTB_string.Reset(PyRefType::Owned, PyObject_CallObject(stringIO_getvalue, nullptr)); + if (printTB_string.IsNULLOrNone()) + return retval; + + PythonString str(PyRefType::Borrowed, printTB_string); + llvm::StringRef string_data(str.GetString()); + retval.assign(string_data.data(), string_data.size()); + return retval; } @@ -2911,46 +2860,33 @@ if (!cmd_obj_sp) return false; - - PyObject* implementor = (PyObject*)cmd_obj_sp->GetValue(); - - if (implementor == nullptr || implementor == Py_None) + + PyRef implementor(PyRefType::Borrowed, (PyObject *)cmd_obj_sp->GetValue()); + + if (implementor.IsNULLOrNone()) return false; - - PyObject* pmeth = PyObject_GetAttrString(implementor, callee_name); - + + PyRef pmeth(PyRefType::Owned, PyObject_GetAttrString(implementor, callee_name)); + if (PyErr_Occurred()) - { PyErr_Clear(); - } - - if (pmeth == nullptr || pmeth == Py_None) - { - Py_XDECREF(pmeth); + + if (pmeth.IsNULLOrNone()) return false; - } if (PyCallable_Check(pmeth) == 0) { if (PyErr_Occurred()) - { PyErr_Clear(); - } - - Py_XDECREF(pmeth); return false; } if (PyErr_Occurred()) - { PyErr_Clear(); - } - - Py_XDECREF(pmeth); // right now we know this function exists and is callable.. - PyObject* py_return = PyObject_CallMethod(implementor, callee_name, nullptr); - + PyRef py_return(PyRefType::Owned, PyObject_CallMethod(implementor, callee_name, nullptr)); + // if it fails, print the error but otherwise go on if (PyErr_Occurred()) { @@ -2958,15 +2894,13 @@ PyErr_Clear(); } - if (py_return != Py_None && PythonString::Check(py_return)) + if (!py_return.IsNULLOrNone() && PythonString::Check(py_return)) { - PythonString py_string(py_return); + PythonString py_string(PyRefType::Borrowed, py_return); llvm::StringRef return_data(py_string.GetString()); dest.assign(return_data.data(), return_data.size()); got_string = true; } - Py_XDECREF(py_return); - return got_string; } @@ -2983,46 +2917,33 @@ if (!cmd_obj_sp) return result; - - PyObject* implementor = (PyObject*)cmd_obj_sp->GetValue(); - - if (implementor == nullptr || implementor == Py_None) + + PyRef implementor(PyRefType::Borrowed, (PyObject *)cmd_obj_sp->GetValue()); + + if (implementor.IsNULLOrNone()) return result; - - PyObject* pmeth = PyObject_GetAttrString(implementor, callee_name); - + + PyRef pmeth(PyRefType::Owned, PyObject_GetAttrString(implementor, callee_name)); + if (PyErr_Occurred()) - { PyErr_Clear(); - } - - if (pmeth == nullptr || pmeth == Py_None) - { - Py_XDECREF(pmeth); + + if (pmeth.IsNULLOrNone()) return result; - } if (PyCallable_Check(pmeth) == 0) { if (PyErr_Occurred()) - { PyErr_Clear(); - } - - Py_XDECREF(pmeth); return result; } if (PyErr_Occurred()) - { PyErr_Clear(); - } - - Py_XDECREF(pmeth); // right now we know this function exists and is callable.. - PyObject* py_return = PyObject_CallMethod(implementor, callee_name, nullptr); - + PyRef py_return(PyRefType::Owned, PyObject_CallMethod(implementor, callee_name, nullptr)); + // if it fails, print the error but otherwise go on if (PyErr_Occurred()) { @@ -3030,12 +2951,11 @@ PyErr_Clear(); } - if (py_return != Py_None && PythonInteger::Check(py_return)) + if (!py_return.IsNULLOrNone() && PythonInteger::Check(py_return)) { - PythonInteger int_value(py_return); + PythonInteger int_value(PyRefType::Borrowed, py_return); result = int_value.GetInteger(); } - Py_XDECREF(py_return); return result; } @@ -3055,46 +2975,34 @@ if (!cmd_obj_sp) return false; - - PyObject* implementor = (PyObject*)cmd_obj_sp->GetValue(); - - if (implementor == nullptr || implementor == Py_None) + + PyRef implementor(PyRefType::Borrowed, (PyObject *)cmd_obj_sp->GetValue()); + + if (implementor.IsNULLOrNone()) return false; - - PyObject* pmeth = PyObject_GetAttrString(implementor, callee_name); - + + PyRef pmeth(PyRefType::Owned, PyObject_GetAttrString(implementor, callee_name)); + if (PyErr_Occurred()) - { PyErr_Clear(); - } - - if (pmeth == nullptr || pmeth == Py_None) - { - Py_XDECREF(pmeth); + + if (pmeth.IsNULLOrNone()) return false; - } if (PyCallable_Check(pmeth) == 0) { if (PyErr_Occurred()) - { PyErr_Clear(); - } - Py_XDECREF(pmeth); return false; } if (PyErr_Occurred()) - { PyErr_Clear(); - } - - Py_XDECREF(pmeth); // right now we know this function exists and is callable.. - PyObject* py_return = PyObject_CallMethod(implementor, callee_name, nullptr); - + PyRef py_return(PyRefType::Owned, PyObject_CallMethod(implementor, callee_name, nullptr)); + // if it fails, print the error but otherwise go on if (PyErr_Occurred()) { @@ -3102,14 +3010,13 @@ PyErr_Clear(); } - if (py_return != Py_None && PythonString::Check(py_return)) + if (!py_return.IsNULLOrNone() && PythonString::Check(py_return)) { - PythonString str(py_return); + PythonString str(PyRefType::Borrowed, py_return); llvm::StringRef str_data(str.GetString()); dest.assign(str_data.data(), str_data.size()); got_string = true; } - Py_XDECREF(py_return); return got_string; } Index: unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp =================================================================== --- unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp +++ unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp @@ -23,19 +23,75 @@ SetUp() override { HostInfoBase::Initialize(); - // ScriptInterpreterPython::Initialize() depends on things like HostInfo being initialized - // so it can compute the python directory etc, so we need to do this after - // SystemInitializerCommon::Initialize(). + // ScriptInterpreterPython::Initialize() depends on HostInfo being + // initializedso it can compute the python directory etc. ScriptInterpreterPython::Initialize(); + + // Although we don't care about concurrency for the purposes of running + // this test suite, Python requires the GIL to be locked even for + // deallocating memory, which can happen when you call Py_DECREF or + // Py_INCREF. So acquire the GIL for the entire duration of this + // test suite. + m_gil_state = PyGILState_Ensure(); } void TearDown() override { + PyGILState_Release(m_gil_state); + ScriptInterpreterPython::Terminate(); } + + private: + PyGILState_STATE m_gil_state; }; +TEST_F(PythonDataObjectsTest, TestOwnedReferences) +{ + // After creating a new object, the refcount should be 1 + PyObject *obj = PyLong_FromLong(3); + EXPECT_EQ(1, obj->ob_refcnt); + + // If we take an owned reference, the refcount should still be 1 + PyRef owned_long(PyRefType::Owned, obj); + EXPECT_EQ(1, owned_long.get()->ob_refcnt); + + // Take another reference and verify that the refcount increases + PyRef strong_ref(owned_long); + EXPECT_EQ(2, strong_ref.get()->ob_refcnt); + + // If we reset the first one, the refcount should be 1 again. + owned_long.Reset(); + EXPECT_EQ(1, strong_ref.get()->ob_refcnt); +} + +TEST_F(PythonDataObjectsTest, TestResetting) +{ + PythonDictionary dict(PyInitialValue::Empty); + + PyObject *new_dict = PyDict_New(); + dict.Reset(PyRefType::Owned, new_dict); + EXPECT_EQ(new_dict, dict.get()); + + dict.Reset(PyRefType::Owned, nullptr); + EXPECT_EQ(nullptr, dict.get()); + + dict.Reset(PyRefType::Owned, PyDict_New()); + EXPECT_NE(nullptr, dict.get()); + dict.Reset(); + EXPECT_EQ(nullptr, dict.get()); +} + +TEST_F(PythonDataObjectsTest, TestBorrowedReferences) +{ + PythonInteger long_value(PyRefType::Owned, PyLong_FromLong(3)); + EXPECT_EQ(1, long_value.get()->ob_refcnt); + + PythonInteger borrowed_long(PyRefType::Borrowed, long_value.get()); + EXPECT_EQ(2, borrowed_long.get()->ob_refcnt); +} + TEST_F(PythonDataObjectsTest, TestPythonInteger) { // Test that integers behave correctly when wrapped by a PythonInteger. @@ -45,7 +101,7 @@ // Note that PyInt doesn't exist in Python 3.x, so this is only for 2.x PyObject *py_int = PyInt_FromLong(12); EXPECT_TRUE(PythonInteger::Check(py_int)); - PythonInteger python_int(py_int); + PythonInteger python_int(PyRefType::Owned, py_int); EXPECT_EQ(PyObjectType::Integer, python_int.GetObjectType()); EXPECT_EQ(12, python_int.GetInteger()); @@ -54,7 +110,7 @@ // Verify that `PythonInt` works correctly when given a PyLong object. PyObject *py_long = PyLong_FromLong(12); EXPECT_TRUE(PythonInteger::Check(py_long)); - PythonInteger python_long(py_long); + PythonInteger python_long(PyRefType::Owned, py_long); EXPECT_EQ(PyObjectType::Integer, python_long.GetObjectType()); // Verify that you can reset the value and that it is reflected properly. @@ -74,7 +130,7 @@ // Note that PyString doesn't exist in Python 3.x, so this is only for 2.x PyObject *py_string = PyString_FromString(test_string); EXPECT_TRUE(PythonString::Check(py_string)); - PythonString python_string(py_string); + PythonString python_string(PyRefType::Owned, py_string); EXPECT_EQ(PyObjectType::String, python_string.GetObjectType()); EXPECT_STREQ(test_string, python_string.GetString().data()); @@ -83,7 +139,7 @@ // Verify that `PythonString` works correctly when given a PyUnicode object. PyObject *py_unicode = PyUnicode_FromString(test_string); EXPECT_TRUE(PythonString::Check(py_unicode)); - PythonString python_unicode(py_unicode); + PythonString python_unicode(PyRefType::Owned, py_unicode); EXPECT_EQ(PyObjectType::String, python_unicode.GetObjectType()); EXPECT_STREQ(test_string, python_unicode.GetString().data()); @@ -101,18 +157,17 @@ static const long long_idx0 = 5; static const char *const string_idx1 = "String Index 1"; - PyObject *list_items[list_size]; - PyObject *py_list = PyList_New(2); - list_items[0] = PyLong_FromLong(long_idx0); - list_items[1] = PyString_FromString(string_idx1); + EXPECT_TRUE(PythonList::Check(py_list)); + PythonList list(PyRefType::Owned, py_list); - for (int i = 0; i < list_size; ++i) - PyList_SetItem(py_list, i, list_items[i]); + PyRef list_items[list_size]; + list_items[0].Reset(PyRefType::Owned, PyLong_FromLong(long_idx0)); + list_items[1].Reset(PyRefType::Owned, PyString_FromString(string_idx1)); - EXPECT_TRUE(PythonList::Check(py_list)); + for (int i = 0; i < list_size; ++i) + list.SetItemAtIndex(i, list_items[i]); - PythonList list(py_list); EXPECT_EQ(list_size, list.GetSize()); EXPECT_EQ(PyObjectType::List, list.GetObjectType()); @@ -129,21 +184,20 @@ // Python API behaves correctly when wrapped by a PythonDictionary. static const int dict_entries = 2; - PyObject *keys[dict_entries]; - PyObject *values[dict_entries]; + PyRef keys[dict_entries]; + PyRef values[dict_entries]; - keys[0] = PyString_FromString("Key 0"); - keys[1] = PyLong_FromLong(1); - values[0] = PyLong_FromLong(0); - values[1] = PyString_FromString("Value 1"); + keys[0].Reset(PyRefType::Owned, PyString_FromString("Key 0")); + keys[1].Reset(PyRefType::Owned, PyLong_FromLong(1)); + values[0].Reset(PyRefType::Owned, PyLong_FromLong(0)); + values[1].Reset(PyRefType::Owned, PyString_FromString("Value 1")); PyObject *py_dict = PyDict_New(); - for (int i = 0; i < dict_entries; ++i) - PyDict_SetItem(py_dict, keys[i], values[i]); - EXPECT_TRUE(PythonDictionary::Check(py_dict)); + PythonDictionary dict(PyRefType::Owned, py_dict); - PythonDictionary dict(py_dict); + for (int i = 0; i < dict_entries; ++i) + PyDict_SetItem(py_dict, keys[i], values[i]); EXPECT_EQ(dict.GetSize(), dict_entries); EXPECT_EQ(PyObjectType::Dictionary, dict.GetObjectType()); @@ -162,8 +216,7 @@ static const long long_idx0 = 5; static const char *const string_idx1 = "String Index 1"; - PyObject *py_list = PyList_New(0); - PythonList list(py_list); + PythonList list(PyInitialValue::Empty); PythonInteger integer(long_idx0); PythonString string(string_idx1); @@ -184,19 +237,17 @@ // by a PythonDictionary. static const int dict_entries = 2; - PyObject *keys[dict_entries]; - PyObject *values[dict_entries]; - - keys[0] = PyString_FromString("Key 0"); - keys[1] = PyString_FromString("Key 1"); - values[0] = PyLong_FromLong(1); - values[1] = PyString_FromString("Value 1"); + PythonString keys[dict_entries]; + PyRef values[dict_entries]; - PyObject *py_dict = PyDict_New(); + keys[0].Reset(PyRefType::Owned, PyString_FromString("Key 0")); + keys[1].Reset(PyRefType::Owned, PyString_FromString("Key 1")); + values[0].Reset(PyRefType::Owned, PyLong_FromLong(1)); + values[1].Reset(PyRefType::Owned, PyString_FromString("Value 1")); - PythonDictionary dict(py_dict); + PythonDictionary dict(PyInitialValue::Empty); for (int i = 0; i < 2; ++i) - dict.SetItemForKey(PythonString(keys[i]), values[i]); + dict.SetItemForKey(keys[i], values[i]); EXPECT_EQ(dict_entries, dict.GetSize()); @@ -225,9 +276,9 @@ static const char *const nested_dict_key1 = "Nested Key 1"; static const long nested_dict_value1 = 2; - PythonList list; - PythonList nested_list; - PythonDictionary nested_dict; + PythonList list(PyInitialValue::Empty); + PythonList nested_list(PyInitialValue::Empty); + PythonDictionary nested_dict(PyInitialValue::Empty); nested_list.AppendItem(PythonInteger(nested_list_long_idx0)); nested_list.AppendItem(PythonString(nested_list_str_idx1)); @@ -324,9 +375,9 @@ static const char *const nested_dict_value0 = "Nested Dict Value 0"; static const long nested_dict_value1 = 7; - PythonDictionary dict; - PythonDictionary nested_dict; - PythonList nested_list; + PythonDictionary dict(PyInitialValue::Empty); + PythonDictionary nested_dict(PyInitialValue::Empty); + PythonList nested_list(PyInitialValue::Empty); nested_dict.SetItemForKey(PythonString(nested_dict_keys[0]), PythonString(nested_dict_value0)); nested_dict.SetItemForKey(PythonString(nested_dict_keys[1]), PythonInteger(nested_dict_value1));