Bug 47108 - Reference leak in _dbus_bindings/abstract.c:dbus_py_variant_level found using gcc-with-cpychecker static analyzer
Summary: Reference leak in _dbus_bindings/abstract.c:dbus_py_variant_level found using...
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: python (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: John (J5) Palmieri
URL:
Whiteboard:
Keywords: NEEDINFO
Depends on:
Blocks:
 
Reported: 2012-03-08 10:13 UTC by Dave Malcolm
Modified: 2016-10-01 11:34 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:

Attachments

Description Dave Malcolm 2012-03-08 10:13:35 UTC 
Description of problem:
I've been writing an experimental static analysis tool to detect bugs commonly occurring within C Python extension modules:
  https://fedorahosted.org/gcc-python-plugin/
  http://gcc-python-plugin.readthedocs.org/en/latest/cpychecker.html
  http://fedoraproject.org/wiki/Features/StaticAnalysisOfPythonRefcounts

I ran the latest version of the tool (in git master; post 0.9) on
dbus-1.0.0 (vs cpython 2.7), and it reports various errors.

You can see a list of errors here, triaged into categories (from most significant to least significant):
http://fedorapeople.org/~dmalcolm/gcc-python-plugin/2012-03-08/dbus-1.0.0/

I've manually reviewed the issues reported by the tool.

Within the category "Reference leaks" the 1 issue reported
 abstract.c:dbus_py_variant_level_set:ob_refcnt of '*vl_obj' is 1 too high
appears to be a true reference leak on the PyIntObject (though if it's in the range -5..256 it's likely to be inconsequential)

Within the category "Returning (PyObject*)NULL without setting an exception" the 2 issues reported may or may not be true bugs: does dbus_py_variant_level_set() set an exception if it returns 0?

There may of course be other bugs in my checker tool.

Hope this is helpful; let me know if you need help reading the logs that the tool generates - I know that it could use some improvement.

Version-Release number of selected component (if applicable):
dbus-1.0.0
gcc-python-plugin post-0.9 git 5c50eef77b4dd7ceb4f950fa24599264cbd21346
Comment 1 Simon McVittie 2012-03-12 04:42:22 UTC 
(In reply to comment #0)
> Within the category "Reference leaks" the 1 issue reported
>  abstract.c:dbus_py_variant_level_set:ob_refcnt of '*vl_obj' is 1 too high
> appears to be a true reference leak on the PyIntObject (though if it's in the
> range -5..256 it's likely to be inconsequential)

Indeed. Fixed in git, I believe.

> Within the category "Returning (PyObject*)NULL without setting an exception"
> the 2 issues reported may or may not be true bugs: does
> dbus_py_variant_level_set() set an exception if it returns 0?

It's meant to. It can fail because PyLong_FromVoidPtr, PyDict_DelItem, PyInt_FromLong or PyDict_SetItem fails; all of those set the exception, I believe (but I'd welcome independent review of that function).

Is there a way to annotate C helper functions that return int (analogous to PyDict_DelItem) or boolean, so tools like yours know they're meant to set the exception?
Comment 2 Dave Malcolm 2012-05-09 09:20:01 UTC 
(In reply to comment #1)
> (In reply to comment #0)
> > Within the category "Returning (PyObject*)NULL without setting an exception"
> > the 2 issues reported may or may not be true bugs: does
> > dbus_py_variant_level_set() set an exception if it returns 0?
> 
> It's meant to. It can fail because PyLong_FromVoidPtr, PyDict_DelItem,
> PyInt_FromLong or PyDict_SetItem fails; all of those set the exception, I
> believe (but I'd welcome independent review of that function).
I quickly doublechecked, and it does indeed look correct.

> Is there a way to annotate C helper functions that return int (analogous to
> PyDict_DelItem) or boolean, so tools like yours know they're meant to set the
> exception?
Somewhat, see:
http://gcc-python-plugin.readthedocs.org/en/latest/cpychecker.html#errors-in-exception-handling

It sounds like we may need another version of that attribute to handle the specific behaviors of that function.

Alternatively, I'm working on interprocedural analysis to eliminate this kind of false positive:
  https://fedorahosted.org/gcc-python-plugin/ticket/40
which when done ought to work in this case, given that the function is defined in the same source file (abstract.c)

Hope that this is helpful
Dave
Comment 3 Philip Withnall 2016-10-01 11:34:24 UTC 
Looks like gcc-python-plugin is dormant now, and hasn’t grown interprocedural analysis, but has grown a cpychecker_negative_result_sets_exception attribute which should satisfy the PyDict_* calls. I guess this can be closed now since any remaining changes need to be done in gcc-python-plugin, not D-Bus.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.