From 52712d3ff7a2f7bcf737996d6ab59ef2cc29c20d Mon Sep 17 00:00:00 2001 From: Lawrence D'Anna Date: Fri, 8 May 2020 10:56:30 -0700 Subject: Re-land "get rid of PythonInteger::GetInteger()" This was reverted due to a python2-specific bug. Re-landing with a fix for python2. Summary: One small step in my long running quest to improve python exception handling in LLDB. Replace GetInteger() which just returns an int with As and friends, which return Expected types that can track python exceptions Reviewers: labath, jasonmolenda, JDevlieghere, vadimcn, omjavaid Reviewed By: labath, omjavaid Subscribers: omjavaid, lldb-commits Tags: #lldb Differential Revision: https://reviews.llvm.org/D78462 --- .../ScriptInterpreter/Python/PythonDataObjects.cpp | 89 +++++++++++++++++----- 1 file changed, 68 insertions(+), 21 deletions(-) (limited to 'lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp') diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp index 40ed22a..6f040fd 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp @@ -44,7 +44,15 @@ template <> Expected python::As(Expected &&obj) { if (!obj) return obj.takeError(); - return obj.get().AsLongLong(); + return obj->AsLongLong(); +} + +template <> +Expected +python::As(Expected &&obj) { + if (!obj) + return obj.takeError(); + return obj->AsUnsignedLongLong(); } template <> @@ -61,6 +69,55 @@ Expected python::As(Expected &&obj) { return std::string(utf8.get()); } +Expected PythonObject::AsLongLong() const { + if (!m_py_obj) + return nullDeref(); +#if PY_MAJOR_VERSION < 3 + if (!PyLong_Check(m_py_obj)) { + PythonInteger i(PyRefType::Borrowed, m_py_obj); + return i.AsLongLong(); + } +#endif + assert(!PyErr_Occurred()); + long long r = PyLong_AsLongLong(m_py_obj); + if (PyErr_Occurred()) + return exception(); + return r; +} + +Expected PythonObject::AsUnsignedLongLong() const { + if (!m_py_obj) + return nullDeref(); +#if PY_MAJOR_VERSION < 3 + if (!PyLong_Check(m_py_obj)) { + PythonInteger i(PyRefType::Borrowed, m_py_obj); + return i.AsUnsignedLongLong(); + } +#endif + assert(!PyErr_Occurred()); + long long r = PyLong_AsUnsignedLongLong(m_py_obj); + if (PyErr_Occurred()) + return exception(); + return r; +} + +// wraps on overflow, instead of raising an error. +Expected PythonObject::AsModuloUnsignedLongLong() const { + if (!m_py_obj) + return nullDeref(); +#if PY_MAJOR_VERSION < 3 + if (!PyLong_Check(m_py_obj)) { + PythonInteger i(PyRefType::Borrowed, m_py_obj); + return i.AsModuloUnsignedLongLong(); + } +#endif + assert(!PyErr_Occurred()); + unsigned long long r = PyLong_AsUnsignedLongLongMask(m_py_obj); + if (PyErr_Occurred()) + return exception(); + return r; +} + void StructuredPythonObject::Serialize(llvm::json::OStream &s) const { s.value(llvm::formatv("Python Obj: {0:X}", GetValue()).str()); } @@ -463,32 +520,22 @@ void PythonInteger::Convert(PyRefType &type, PyObject *&py_obj) { #endif } -int64_t PythonInteger::GetInteger() const { - if (m_py_obj) { - assert(PyLong_Check(m_py_obj) && - "PythonInteger::GetInteger has a PyObject that isn't a PyLong"); - - int overflow = 0; - int64_t result = PyLong_AsLongLongAndOverflow(m_py_obj, &overflow); - if (overflow != 0) { - // We got an integer that overflows, like 18446744072853913392L we can't - // use PyLong_AsLongLong() as it will return 0xffffffffffffffff. If we - // use the unsigned long long it will work as expected. - const uint64_t uval = PyLong_AsUnsignedLongLong(m_py_obj); - result = static_cast(uval); - } - return result; - } - return UINT64_MAX; -} - void PythonInteger::SetInteger(int64_t value) { *this = Take(PyLong_FromLongLong(value)); } StructuredData::IntegerSP PythonInteger::CreateStructuredInteger() const { StructuredData::IntegerSP result(new StructuredData::Integer); - result->SetValue(GetInteger()); + // FIXME this is really not ideal. Errors are silently converted to 0 + // and overflows are silently wrapped. But we'd need larger changes + // to StructuredData to fix it, so that's how it is for now. + llvm::Expected value = AsModuloUnsignedLongLong(); + if (!value) { + llvm::consumeError(value.takeError()); + result->SetValue(0); + } else { + result->SetValue(value.get()); + } return result; } -- cgit v1.1