Red Hat Bugzilla – Full Text Bug Listing
|Summary:||Bugs found in veusz-1.14-3.fc17 using gcc-with-cpychecker static analyzer|
|Product:||[Fedora] Fedora||Reporter:||Dave Malcolm <dmalcolm>|
|Component:||veusz||Assignee:||Jeremy Sanders <jeremy>|
|Status:||CLOSED ERRATA||QA Contact:||Fedora Extras Quality Assurance <extras-qa>|
|Fixed In Version:||veusz-1.15-1.fc17||Doc Type:||Bug Fix|
|Doc Text:||Story Points:||---|
|Last Closed:||2012-04-04 15:18:18 EDT||Type:||---|
|oVirt Team:||---||RHEL 7.3 requirements from Atomic Host:|
|Bug Depends On:|
Description Dave Malcolm 2012-03-05 17:52:35 EST
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 veusz-1.14-3.fc17.src.rpm, 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-05/veusz-1.14-3.fc17/ I've manually reviewed the issues reported by the tool. Within the category "Segfaults within error-handling paths" the 3 issues reported appear to be genuine bugs where a call can fail under low memory conditions, returning NULL, but then gets used. Within the category "Returning (PyObject*)NULL without setting an exception" the 1 issue reported appears to be a genuine bug: PyMem_Malloc does not set the thread's exception state when it fails. The error-handling near line 1510 needs a call to PyErr_NoMemory(). 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): veusz-1.14-3.fc17 gcc-python-plugin post-0.9 git 11462291a66c8db693c8884cb84b795bb5988ffb running the checker in an *f16* chroot
Comment 1 Jeremy Sanders 2012-03-19 17:57:50 EDT
Thanks for the helpful report - I think I've fixed this upstream with this patch: https://github.com/jeremysanders/veusz/commit/3e000ca5fd892c15ef4338cda0ec1760bb08ed19 I didn't fix this problem because it looks like a false positive "Returning (PyObject*)NULL without setting an exception These messages are often false-positives: the analysis tool has no knowledge about internal API calls that can lead to an exception being set helpers/src/_nc_cntr.c cntr_trace returning (PyObject*)NULL without setting an exception" I can't see any way for the error part of cntr_trace to be called without an exception being set. If any PyMem_Alloc fails this should set the exception. The fix should be in 1.15 which should be released shortly.
Comment 2 Dave Malcolm 2012-03-21 15:14:00 EDT
(In reply to comment #1) > Thanks for the helpful report - I think I've fixed this upstream with this > patch: > https://github.com/jeremysanders/veusz/commit/3e000ca5fd892c15ef4338cda0ec1760bb08ed19 Excellent; thanks! > I didn't fix this problem because it looks like a false positive > > "Returning (PyObject*)NULL without setting an exception > > These messages are often false-positives: the analysis tool has no knowledge > about internal API calls that can lead to an exception being set > helpers/src/_nc_cntr.c cntr_trace returning (PyObject*)NULL without setting > an exception" > > I can't see any way for the error part of cntr_trace to be called without an > exception being set. If any PyMem_Alloc fails this should set the exception. Although these are typically false positives, I think that in this case, it actually *is* a bug. PyMem_Malloc() *doesn't* set an exception when it fails. http://docs.python.org/c-api/memory.html#PyMem_Malloc doesn't spell this out, but review of the source code shows it to be the case: it's a thin macro wrapper around malloc(); indeed the example code on that page shows an explicit call to PyErr_NoMemory() for the case when PyMem_Malloc fails. So I think that error-handling path needs a call to PyErr_NoMemory(): http://docs.python.org/c-api/exceptions.html#PyErr_NoMemory Hope this is helpful Dave
Comment 3 Jeremy Sanders 2012-03-24 05:49:02 EDT
That's very helpful, thanks. I've fixed the PyMem_Malloc bug upstream in another commit. https://github.com/jeremysanders/veusz/commit/f61d1cb2c42dba5f23c4ba2110d5be4ac01ec974 I've checked the other PyMem_Malloc cases. None were marked by your checker and I think they always set an exception on failure.
Comment 4 Jeremy Sanders 2012-04-04 15:18:18 EDT
Upstream Veusz 1.15 fixes this. I've updated rawhide and f17-candidate to Veusz 1.15. http://koji.fedoraproject.org/koji/buildinfo?buildID=311679 http://koji.fedoraproject.org/koji/buildinfo?buildID=311678 Thanks.
Comment 5 Fedora Update System 2012-04-04 15:23:08 EDT
veusz-1.15-1.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/veusz-1.15-1.fc17
Comment 6 Fedora Update System 2012-04-11 23:17:43 EDT
veusz-1.15-1.fc17 has been pushed to the Fedora 17 stable repository. If problems still persist, please make note of it in this bug report.