[Bug python/23841] New: cpychecker reference count checking issues

["vries at gcc dot gnu.org" <sourceware-bugzilla at sourceware dot org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=23841

            Bug ID: 23841
           Summary: cpychecker reference count checking issues
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: python
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

Gdb supports a python interface, which is implemented using Python C
extensions.

The extensions are written in C, and interact with the Python interpreter. 

Certain aspects of the Python runtime environment need to be taken into account
in the extensions, which is difficult to get right manually.

Using the static checker cpychecker, we try to find such problems in the
extension implementations. [ We can use any tool, but cpychecker seems the one
typically used, given that gdb source code contains cpychecker annotations. ]

One example of something the extensions need to take into account is the
reference counting aspect of objects, and cpychecker has a check for that.

The typical example is:
...
     1  #include "Python.h"
     2
     3  PyObject *
     4  test (PyObject *self, PyObject *args)
     5  {
     6    PyObject *list;
     7    PyObject *item;
     8
     9    list = PyList_New (1);
    10    if (list == NULL)
    11      return NULL;
    12
    13    item = PyLong_FromLong (42);
    14
    15    /* This error handling is incorrect: it's missing an
    16       invocation of Py_DECREF(list): */
    17    if (!item)
    18      return NULL;
    19
    20    /* This steals a reference to item; item is not leaked when we get
here: */
    21    PyList_SetItem (list, 0, item);
    22
    23    return list;
    24  }
...
where cpychecker produces an error indicating the missing Py_DECREF at line 18,
which can then be fixed using:
...
$ diff -u test.c.initial test.c.fixed 
--- test.c.initial      2018-10-29 10:45:30.215131722 +0100
+++ test.c.fixed        2018-10-29 10:42:29.211132413 +0100
@@ -15,7 +15,10 @@
   /* This error handling is incorrect: it's missing an
      invocation of Py_DECREF(list): */
   if (!item)
-    return NULL;
+    {
+      Py_DECREF (list);
+      return NULL;
+    }

   /* This steals a reference to item; item is not leaked when we get here: */
   PyList_SetItem (list, 0, item);
...

However, gdb has a solution for easier writing of correct ref counting code:
gdbpy_ref, a smart pointer that calls Py_DECREF when the destructor is called:
...
PyObject *
test (PyObject *self, PyObject *args)
{
  PyObject *item;

  gdbpy_ref<> list (PyList_New (1));
  if (list == NULL)
    return NULL;

  item = PyLong_FromLong (42);

  if (!item)
    return NULL;

  /* This steals a reference to item; item is not leaked when we get here: */
  PyList_SetItem (list.get (), 0, item);

  return list.release ();
}
...
This means we don't have to add the Py_DECREF to the return NULL as before, but
OTOH we now need to manage more explicitly the regular execution path (here
using list.get and list.release).

However, cpychecker does not support interprocedural analysis (
https://github.com/davidmalcolm/gcc-python-plugin/issues/80 ), and consequently
generates false positives for the gdbpy_ref code (since it fails to recognize
that the list destructor calls Py_DECREF).

-- 
You are receiving this mail because:
You are on the CC list for the bug.