[Lldb-commits] [lldb] [lldb] Use PyBytes and PyByteArray in Python Data Objects unittest (PR #82098)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/82098

Use a Python Bytes and ByteArray object instead of Integers for
TestOwnedReferences and TestBorrowedReferences. These two tests were
failing when building against Python 3.12 because these Integer objects
had a refcount of 4294967296 (-1). I didn't dig into it, but I suspect
the Python runtime has adopted an optimization to decrease refcounting
traffic for these simple objects.

>From e0abbba764acd5e7685780e79628219728052005 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Fri, 16 Feb 2024 23:20:40 -0800
Subject: [PATCH 1/2] [lldb] Use Py_REFCNT instead of accessing ob_refcnt
 directly

---
 .../Python/PythonDataObjectsTests.cpp           | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
index efb8f725f6739a..d9e39ac9fcd9a1 100644
--- a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -52,20 +52,23 @@ class PythonDataObjectsTest : public PythonTestSuite {
 TEST_F(PythonDataObjectsTest, TestOwnedReferences) {
   // After creating a new object, the refcount should be >= 1
   PyObject *obj = PyLong_FromLong(3);
-  Py_ssize_t original_refcnt = obj->ob_refcnt;
+  Py_ssize_t original_refcnt = Py_REFCNT(obj);
   EXPECT_LE(1, original_refcnt);
 
   // If we take an owned reference, the refcount should be the same
   PythonObject owned_long(PyRefType::Owned, obj);
-  EXPECT_EQ(original_refcnt, owned_long.get()->ob_refcnt);
+  Py_ssize_t owned_refcnt = Py_REFCNT(owned_long.get());
+  EXPECT_EQ(original_refcnt, owned_refcnt);
 
   // Take another reference and verify that the refcount increases by 1
   PythonObject strong_ref(owned_long);
-  EXPECT_EQ(original_refcnt + 1, strong_ref.get()->ob_refcnt);
+  Py_ssize_t strong_refcnt = Py_REFCNT(strong_ref.get());
+  EXPECT_EQ(original_refcnt + 1, strong_refcnt);
 
   // If we reset the first one, the refcount should be the original value.
   owned_long.Reset();
-  EXPECT_EQ(original_refcnt, strong_ref.get()->ob_refcnt);
+  strong_refcnt = Py_REFCNT(strong_ref.get());
+  EXPECT_EQ(original_refcnt, strong_refcnt);
 }
 
 TEST_F(PythonDataObjectsTest, TestResetting) {
@@ -83,11 +86,13 @@ TEST_F(PythonDataObjectsTest, TestResetting) {
 
 TEST_F(PythonDataObjectsTest, TestBorrowedReferences) {
   PythonInteger long_value(PyRefType::Owned, PyLong_FromLong(3));
-  Py_ssize_t original_refcnt = long_value.get()->ob_refcnt;
+  Py_ssize_t original_refcnt = Py_REFCNT(long_value.get());
   EXPECT_LE(1, original_refcnt);
 
   PythonInteger borrowed_long(PyRefType::Borrowed, long_value.get());
-  EXPECT_EQ(original_refcnt + 1, borrowed_long.get()->ob_refcnt);
+  Py_ssize_t borrowed_refcnt = Py_REFCNT(borrowed_long.get());
+
+  EXPECT_EQ(original_refcnt + 1, borrowed_refcnt);
 }
 
 TEST_F(PythonDataObjectsTest, TestGlobalNameResolutionNoDot) {

>From bf1e1d469b20614ca7ec4819b5f77e23cfbb7441 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Fri, 16 Feb 2024 23:35:01 -0800
Subject: [PATCH 2/2] [lldb] Use PyBytes and PyByteArray in Python Data Objects
 unittest

Use a Python Bytes and ByteArray object instead of Integers for
TestOwnedReferences and TestBorrowedReferences. These two tests were
failing when building against Python 3.12 because these Integer objects
had a refcount of 4294967296 (-1). I didn't dig into it, but I suspect
the Python runtime has adopted an optimization to decrease refcounting
traffic for these simple objects.
---
 .../Python/PythonDataObjectsTests.cpp         | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
index d9e39ac9fcd9a1..a4db4627f935b4 100644
--- a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -51,22 +51,22 @@ class PythonDataObjectsTest : public PythonTestSuite {
 
 TEST_F(PythonDataObjectsTest, TestOwnedReferences) {
   // After creating a new object, the refcount should be >= 1
-  PyObject *obj = PyLong_FromLong(3);
+  PyObject *obj = PyBytes_FromString("foo");
   Py_ssize_t original_refcnt = Py_REFCNT(obj);
   EXPECT_LE(1, original_refcnt);
 
   // If we take an owned reference, the refcount should be the same
-  PythonObject owned_long(PyRefType::Owned, obj);
-  Py_ssize_t owned_refcnt = Py_REFCNT(owned_long.get());
+  PythonObject owned(PyRefType::Owned, obj);
+  Py_ssize_t owned_refcnt = Py_REFCNT(owned.get());
   EXPECT_EQ(original_refcnt, owned_refcnt);
 
   // Take another reference and verify that the refcount increases by 1
-  PythonObject strong_ref(owned_long);
+  PythonObject strong_ref(owned);
   Py_ssize_t strong_refcnt = Py_REFCNT(strong_ref.get());
   EXPECT_EQ(original_refcnt + 1, strong_refcnt);
 
   // If we reset the first one, the refcount should be the original value.
-  owned_long.Reset();
+  owned.Reset();
   strong_refcnt = Py_REFCNT(strong_ref.get());
   EXPECT_EQ(original_refcnt, strong_refcnt);
 }
@@ -85,12 +85,13 @@ TEST_F(PythonDataObjectsTest, TestResetting) {
 }
 
 TEST_F(PythonDataObjectsTest, TestBorrowedReferences) {
-  PythonInteger long_value(PyRefType::Owned, PyLong_FromLong(3));
-  Py_ssize_t original_refcnt = Py_REFCNT(long_value.get());
+  PythonByteArray byte_value(PyRefType::Owned,
+                             PyByteArray_FromStringAndSize("foo", 3));
+  Py_ssize_t original_refcnt = Py_REFCNT(byte_value.get());
   EXPECT_LE(1, original_refcnt);
 
-  PythonInteger borrowed_long(PyRefType::Borrowed, long_value.get());
-  Py_ssize_t borrowed_refcnt = Py_REFCNT(borrowed_long.get());
+  PythonByteArray borrowed_byte(PyRefType::Borrowed, byte_value.get());
+  Py_ssize_t borrowed_refcnt = Py_REFCNT(borrowed_byte.get());
 
   EXPECT_EQ(original_refcnt + 1, borrowed_refcnt);
 }