Author: Muhammad Omair Javaid Date: 2020-04-23T04:38:32+05:00 New Revision: 478619cf9a24ad8eac806959ca6a289a9beb71ae
URL: https://github.com/llvm/llvm-project/commit/478619cf9a24ad8eac806959ca6a289a9beb71ae DIFF: https://github.com/llvm/llvm-project/commit/478619cf9a24ad8eac806959ca6a289a9beb71ae.diff LOG: Revert "get rid of PythonInteger::GetInteger()" This reverts commit 7375212172951d2fc283c81d03c1a8588c3280c6. This causes multiple test failures on LLDB AArch64 Linux buildbot. http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/3695 Differential Revision: https://reviews.llvm.org/D78462 Added: Modified: lldb/bindings/python/python-typemaps.swig lldb/bindings/python/python-wrapper.swig lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp Removed: ################################################################################ diff --git a/lldb/bindings/python/python-typemaps.swig b/lldb/bindings/python/python-typemaps.swig index c08aeab71f78..46dcaf611a4f 100644 --- a/lldb/bindings/python/python-typemaps.swig +++ b/lldb/bindings/python/python-typemaps.swig @@ -59,25 +59,37 @@ $result = list.release(); } + %typemap(in) lldb::tid_t { - PythonObject obj = Retain<PythonObject>($input); - lldb::tid_t value = unwrapOrSetPythonException(As<unsigned long long>(obj)); - if (PyErr_Occurred()) + if (PythonInteger::Check($input)) + { + PythonInteger py_int(PyRefType::Borrowed, $input); + $1 = static_cast<lldb::tid_t>(py_int.GetInteger()); + } + else + { + PyErr_SetString(PyExc_ValueError, "Expecting an integer"); return nullptr; - $1 = value; + } } %typemap(in) lldb::StateType { - PythonObject obj = Retain<PythonObject>($input); - unsigned long long state_type_value = - unwrapOrSetPythonException(As<unsigned long long>(obj)); - if (PyErr_Occurred()) - return nullptr; - if (state_type_value > lldb::StateType::kLastStateType) { - PyErr_SetString(PyExc_ValueError, "Not a valid StateType value"); + if (PythonInteger::Check($input)) + { + PythonInteger py_int(PyRefType::Borrowed, $input); + int64_t state_type_value = py_int.GetInteger() ; + + if (state_type_value > lldb::StateType::kLastStateType) { + PyErr_SetString(PyExc_ValueError, "Not a valid StateType value"); + return nullptr; + } + $1 = static_cast<lldb::StateType>(state_type_value); + } + else + { + PyErr_SetString(PyExc_ValueError, "Expecting an integer"); return nullptr; } - $1 = static_cast<lldb::StateType>(state_type_value); } /* Typemap definitions to allow SWIG to properly handle char buffer. */ diff --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig index f9e89373fe25..3a63165cf58d 100644 --- a/lldb/bindings/python/python-wrapper.swig +++ b/lldb/bindings/python/python-wrapper.swig @@ -444,7 +444,6 @@ LLDBSwigPythonCallBreakpointResolver if (PyErr_Occurred()) { PyErr_Print(); - PyErr_Clear(); return 0; } @@ -458,13 +457,11 @@ LLDBSwigPythonCallBreakpointResolver return 1; } - long long ret_val = unwrapOrSetPythonException(As<long long>(result)); - - if (PyErr_Occurred()) { - PyErr_Print(); - PyErr_Clear(); + PythonInteger int_result = result.AsType<PythonInteger>(); + if (!int_result.IsAllocated()) return 0; - } + + unsigned int ret_val = int_result.GetInteger(); return ret_val; } @@ -518,17 +515,26 @@ LLDBSwigPython_CalculateNumChildren return 0; } - size_t ret_val; + PythonObject result; + if (arg_info.get().max_positional_args < 1) - ret_val = unwrapOrSetPythonException(As<long long>(pfunc.Call())); + result = pfunc(); else - ret_val = unwrapOrSetPythonException(As<long long>(pfunc.Call(PythonInteger(max)))); + result = pfunc(PythonInteger(max)); - if (PyErr_Occurred()) + if (!result.IsAllocated()) + return 0; + + PythonInteger int_result = result.AsType<PythonInteger>(); + if (!int_result.IsAllocated()) + return 0; + + size_t ret_val = int_result.GetInteger(); + + if (PyErr_Occurred()) //FIXME use Expected to catch python exceptions { PyErr_Print(); PyErr_Clear(); - return 0; } if (arg_info.get().max_positional_args < 1) @@ -582,15 +588,16 @@ LLDBSwigPython_GetIndexOfChildWithName if (!pfunc.IsAllocated()) return UINT32_MAX; - llvm::Expected<PythonObject> result = pfunc.Call(PythonString(child_name)); + PythonObject result = pfunc(PythonString(child_name)); - long long retval = unwrapOrSetPythonException(As<long long>(std::move(result))); + if (!result.IsAllocated()) + return UINT32_MAX; - if (PyErr_Occurred()) { - PyErr_Clear(); // FIXME print this? do something else + PythonInteger int_result = result.AsType<PythonInteger>(); + if (!int_result.IsAllocated()) return UINT32_MAX; - } + int64_t retval = int_result.GetInteger(); if (retval >= 0) return (uint32_t)retval; diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp index 3f00f671d88d..40ed22aceebf 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp @@ -44,15 +44,7 @@ template <> Expected<long long> python::As<long long>(Expected<PythonObject> &&obj) { if (!obj) return obj.takeError(); - return obj->AsLongLong(); -} - -template <> -Expected<unsigned long long> -python::As<unsigned long long>(Expected<PythonObject> &&obj) { - if (!obj) - return obj.takeError(); - return obj->AsUnsignedLongLong(); + return obj.get().AsLongLong(); } template <> @@ -471,22 +463,32 @@ 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<int64_t>(uval); + } + return result; + } + return UINT64_MAX; +} + void PythonInteger::SetInteger(int64_t value) { *this = Take<PythonInteger>(PyLong_FromLongLong(value)); } StructuredData::IntegerSP PythonInteger::CreateStructuredInteger() const { StructuredData::IntegerSP result(new StructuredData::Integer); - // 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<unsigned long long> value = AsModuloUnsignedLongLong(); - if (!value) { - llvm::consumeError(value.takeError()); - result->SetValue(0); - } else { - result->SetValue(value.get()); - } + result->SetValue(GetInteger()); return result; } diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h index b09f42e0d540..16896803e136 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h @@ -370,27 +370,6 @@ class PythonObject { return r; } - llvm::Expected<long long> AsUnsignedLongLong() { - if (!m_py_obj) - return nullDeref(); - assert(!PyErr_Occurred()); - long long r = PyLong_AsUnsignedLongLong(m_py_obj); - if (PyErr_Occurred()) - return exception(); - return r; - } - - llvm::Expected<unsigned long long> AsModuloUnsignedLongLong() const { - // wraps on overflow, instead of raising an error. - if (!m_py_obj) - return nullDeref(); - assert(!PyErr_Occurred()); - unsigned long long r = PyLong_AsUnsignedLongLongMask(m_py_obj); - if (PyErr_Occurred()) - return exception(); - return r; - } - llvm::Expected<bool> IsInstance(const PythonObject &cls) { if (!m_py_obj || !cls.IsValid()) return nullDeref(); @@ -420,10 +399,6 @@ template <> llvm::Expected<bool> As<bool>(llvm::Expected<PythonObject> &&obj); template <> llvm::Expected<long long> As<long long>(llvm::Expected<PythonObject> &&obj); -template <> -llvm::Expected<unsigned long long> -As<unsigned long long>(llvm::Expected<PythonObject> &&obj); - template <> llvm::Expected<std::string> As<std::string>(llvm::Expected<PythonObject> &&obj); @@ -516,6 +491,8 @@ class PythonInteger : public TypedPythonObject<PythonInteger> { static bool Check(PyObject *py_obj); static void Convert(PyRefType &type, PyObject *&py_obj); + int64_t GetInteger() const; + void SetInteger(int64_t value); StructuredData::IntegerSP CreateStructuredInteger() const; diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp index 6f266772624a..c53b3bd0fb65 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -3150,15 +3150,20 @@ uint32_t ScriptInterpreterPythonImpl::GetFlagsForCommandObject( if (PyErr_Occurred()) PyErr_Clear(); - long long py_return = unwrapOrSetPythonException( - As<long long>(implementor.CallMethod(callee_name))); + // right now we know this function exists and is callable.. + PythonObject py_return( + PyRefType::Owned, + PyObject_CallMethod(implementor.get(), callee_name, nullptr)); // if it fails, print the error but otherwise go on if (PyErr_Occurred()) { PyErr_Print(); PyErr_Clear(); - } else { - result = py_return; + } + + if (py_return.IsAllocated() && PythonInteger::Check(py_return.get())) { + PythonInteger int_value(PyRefType::Borrowed, py_return.get()); + result = int_value.GetInteger(); } return result; diff --git a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp index 75a1f5e67d32..fe3b423b4842 100644 --- a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp +++ b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp @@ -123,11 +123,13 @@ TEST_F(PythonDataObjectsTest, TestInstanceNameResolutionNoDot) { EXPECT_TRUE(major_version_field.IsAllocated()); EXPECT_TRUE(minor_version_field.IsAllocated()); - auto major_version_value = As<long long>(major_version_field); - auto minor_version_value = As<long long>(minor_version_field); + PythonInteger major_version_value = + major_version_field.AsType<PythonInteger>(); + PythonInteger minor_version_value = + minor_version_field.AsType<PythonInteger>(); - EXPECT_THAT_EXPECTED(major_version_value, llvm::HasValue(PY_MAJOR_VERSION)); - EXPECT_THAT_EXPECTED(minor_version_value, llvm::HasValue(PY_MINOR_VERSION)); + EXPECT_EQ(PY_MAJOR_VERSION, major_version_value.GetInteger()); + EXPECT_EQ(PY_MINOR_VERSION, minor_version_value.GetInteger()); } TEST_F(PythonDataObjectsTest, TestGlobalNameResolutionWithDot) { @@ -135,14 +137,16 @@ TEST_F(PythonDataObjectsTest, TestGlobalNameResolutionWithDot) { EXPECT_TRUE(sys_path.IsAllocated()); EXPECT_TRUE(PythonList::Check(sys_path.get())); - auto version_major = - As<long long>(m_main_module.ResolveName("sys.version_info.major")); - - auto version_minor = - As<long long>(m_main_module.ResolveName("sys.version_info.minor")); - - EXPECT_THAT_EXPECTED(version_major, llvm::HasValue(PY_MAJOR_VERSION)); - EXPECT_THAT_EXPECTED(version_minor, llvm::HasValue(PY_MINOR_VERSION)); + PythonInteger version_major = + m_main_module.ResolveName("sys.version_info.major") + .AsType<PythonInteger>(); + PythonInteger version_minor = + m_main_module.ResolveName("sys.version_info.minor") + .AsType<PythonInteger>(); + EXPECT_TRUE(version_major.IsAllocated()); + EXPECT_TRUE(version_minor.IsAllocated()); + EXPECT_EQ(PY_MAJOR_VERSION, version_major.GetInteger()); + EXPECT_EQ(PY_MINOR_VERSION, version_minor.GetInteger()); } TEST_F(PythonDataObjectsTest, TestDictionaryResolutionWithDot) { @@ -151,14 +155,14 @@ TEST_F(PythonDataObjectsTest, TestDictionaryResolutionWithDot) { dict.SetItemForKey(PythonString("sys"), m_sys_module); // Now use that dictionary to resolve `sys.version_info.major` - auto version_major = As<long long>( - PythonObject::ResolveNameWithDictionary("sys.version_info.major", dict)); - - auto version_minor = As<long long>( - PythonObject::ResolveNameWithDictionary("sys.version_info.minor", dict)); - - EXPECT_THAT_EXPECTED(version_major, llvm::HasValue(PY_MAJOR_VERSION)); - EXPECT_THAT_EXPECTED(version_minor, llvm::HasValue(PY_MINOR_VERSION)); + PythonInteger version_major = + PythonObject::ResolveNameWithDictionary("sys.version_info.major", dict) + .AsType<PythonInteger>(); + PythonInteger version_minor = + PythonObject::ResolveNameWithDictionary("sys.version_info.minor", dict) + .AsType<PythonInteger>(); + EXPECT_EQ(PY_MAJOR_VERSION, version_major.GetInteger()); + EXPECT_EQ(PY_MINOR_VERSION, version_minor.GetInteger()); } TEST_F(PythonDataObjectsTest, TestPythonInteger) { @@ -172,8 +176,7 @@ TEST_F(PythonDataObjectsTest, TestPythonInteger) { PythonInteger python_int(PyRefType::Owned, py_int); EXPECT_EQ(PyObjectType::Integer, python_int.GetObjectType()); - auto python_int_value = As<long long>(python_int); - EXPECT_THAT_EXPECTED(python_int_value, llvm::HasValue(12)); + EXPECT_EQ(12, python_int.GetInteger()); #endif // Verify that `PythonInteger` works correctly when given a PyLong object. @@ -184,14 +187,12 @@ TEST_F(PythonDataObjectsTest, TestPythonInteger) { // Verify that you can reset the value and that it is reflected properly. python_long.SetInteger(40); - auto e = As<long long>(python_long); - EXPECT_THAT_EXPECTED(e, llvm::HasValue(40)); + EXPECT_EQ(40, python_long.GetInteger()); // Test that creating a `PythonInteger` object works correctly with the // int constructor. PythonInteger constructed_int(7); - auto value = As<long long>(constructed_int); - EXPECT_THAT_EXPECTED(value, llvm::HasValue(7)); + EXPECT_EQ(7, constructed_int.GetInteger()); } TEST_F(PythonDataObjectsTest, TestPythonBoolean) { @@ -338,8 +339,7 @@ TEST_F(PythonDataObjectsTest, TestPythonListValueEquality) { PythonInteger chk_int(PyRefType::Borrowed, chk_value1.get()); PythonString chk_str(PyRefType::Borrowed, chk_value2.get()); - auto chkint = As<long long>(chk_value1); - ASSERT_THAT_EXPECTED(chkint, llvm::HasValue(long_value0)); + EXPECT_EQ(long_value0, chk_int.GetInteger()); EXPECT_EQ(string_value1, chk_str.GetString()); } @@ -367,8 +367,7 @@ TEST_F(PythonDataObjectsTest, TestPythonListManipulation) { PythonInteger chk_int(PyRefType::Borrowed, chk_value1.get()); PythonString chk_str(PyRefType::Borrowed, chk_value2.get()); - auto e = As<long long>(chk_int); - EXPECT_THAT_EXPECTED(e, llvm::HasValue(long_value0)); + EXPECT_EQ(long_value0, chk_int.GetInteger()); EXPECT_EQ(string_value1, chk_str.GetString()); } @@ -488,10 +487,10 @@ TEST_F(PythonDataObjectsTest, TestPythonDictionaryValueEquality) { EXPECT_TRUE(PythonInteger::Check(chk_value1.get())); EXPECT_TRUE(PythonString::Check(chk_value2.get())); + PythonInteger chk_int(PyRefType::Borrowed, chk_value1.get()); PythonString chk_str(PyRefType::Borrowed, chk_value2.get()); - auto chkint = As<long long>(chk_value1); - EXPECT_THAT_EXPECTED(chkint, llvm::HasValue(value_0)); + EXPECT_EQ(value_0, chk_int.GetInteger()); EXPECT_EQ(value_1, chk_str.GetString()); } @@ -525,10 +524,10 @@ TEST_F(PythonDataObjectsTest, TestPythonDictionaryManipulation) { EXPECT_TRUE(PythonInteger::Check(chk_value1.get())); EXPECT_TRUE(PythonString::Check(chk_value2.get())); - auto chkint = As<long long>(chk_value1); + PythonInteger chk_int(PyRefType::Borrowed, chk_value1.get()); PythonString chk_str(PyRefType::Borrowed, chk_value2.get()); - EXPECT_THAT_EXPECTED(chkint, llvm::HasValue(value_0)); + EXPECT_EQ(value_0, chk_int.GetInteger()); EXPECT_EQ(value_1, chk_str.GetString()); } @@ -595,9 +594,10 @@ TEST_F(PythonDataObjectsTest, TestObjectAttributes) { EXPECT_TRUE(py_int.HasAttribute("numerator")); EXPECT_FALSE(py_int.HasAttribute("this_should_not_exist")); - auto numerator_attr = As<long long>(py_int.GetAttributeValue("numerator")); - - EXPECT_THAT_EXPECTED(numerator_attr, llvm::HasValue(42)); + PythonInteger numerator_attr = + py_int.GetAttributeValue("numerator").AsType<PythonInteger>(); + EXPECT_TRUE(numerator_attr.IsAllocated()); + EXPECT_EQ(42, numerator_attr.GetInteger()); } TEST_F(PythonDataObjectsTest, TestExtractingUInt64ThroughStructuredData) { _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
