classification
Title: Issues, reported by PVS-Studio static analyzer
Type: compile error Stage: resolved
Components: Windows Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Rosuav, berker.peksag, christian.heimes, cvrebert, martin.panter, paul.moore, pavel-belikov, python-dev, skrah, steve.dower, tim.golden, vstinner, zach.ware
Priority: normal Keywords: patch

Created on 2016-07-22 07:22 by pavel-belikov, last changed 2016-08-22 15:06 by berker.peksag. This issue is now closed.

Files
File name Uploaded Description Edit
issue27587_pystate_addmodule.diff berker.peksag, 2016-07-25 02:03 review
issue27587_pystate_addmodule_v2.diff berker.peksag, 2016-08-18 13:10 review
Messages (12)
msg270974 - (view) Author: Pavel Belikov (pavel-belikov) Date: 2016-07-22 07:22
To demonstrate the capabilities of our analyzer, we regularly perform analysis of open source projects. We had recently checked the CPython project.

Here is the link to the article about it: http://www.viva64.com/en/b/0414/
Official page of the analyzer: http://www.viva64.com/en/pvs-studio/

If you have any questions, or if you are interested in the evaluation of our static analyzer or in any other source code quality control services that our company provides, please contact us at support@viva64.com.
msg270978 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-07-22 08:14
Thanks for the report. You seem to have identified some code from Open SSL as being from Python (e.g. ASN1_PRINTABLE_type() function in a_print.c).

Here’s a quick copy of the details relevant to Python:

V547 Expression 's->sock_fd < 0' is always false. Unsigned type value is never < 0. Modules/socketmodule.c:655
V547 Expression 's->sock_fd < 0' is always false. Unsigned type value is never < 0. Modules/_ssl.c:1702
V547 Expression 'sock->sock_fd < 0' is always false. Unsigned type value is never < 0. Modules/_ssl.c:2018
Suggestion: compare with INVALID_SOCKET

V614 Potentially uninitialized pointer 'sigint_event' used. Modules/_multiprocessing/semaphore.c:120

V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions 'quotetabs' and '!quotetabs'. Modules/binascii.c:1453

Null pointer check after use of “def” in _PyState_AddModule(), Python/pystate.c
V595 The 'self->extra' pointer was utilized before it was verified against nullptr. Check lines: 917, 923. Modules/_elementtree.c:917

The first two groups (sock_fd and sigint_event) look like Windows-specific code, and I suspect would be diagnosed with GCC (but building Python with GCC on Windows needs work).
msg271002 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-07-22 17:31
> V547 Expression 's->sock_fd < 0' is always false. Unsigned type value is never < 0. Modules/socketmodule.c:655
> V547 Expression 's->sock_fd < 0' is always false. Unsigned type value is never < 0. Modules/_ssl.c:1702
> V547 Expression 'sock->sock_fd < 0' is always false. Unsigned type value is never < 0. Modules/_ssl.c:2018

Victor fixed these issues in 6c11f52ab9db and 025281485318.

> V614 Potentially uninitialized pointer 'sigint_event' used. Modules/_multiprocessing/semaphore.c:120

See issue 27591.
msg271042 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-07-22 23:55
Christian Heimes posted a patch for _PyState_AddModule() on Python-dev: https://marc.info/?l=python-dev&m=146922730716425&w=2
msg271046 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-07-23 00:15
Also on python-dev, Chris Angelico pointed out that the _elementtree.c case is a false positive. So that would leave the binascii one, which I think is worth simpifying, but is probably not very serious.
msg271079 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2016-07-23 13:47
Hi Pavel,

The issues in ASN1_PRINTABLE_type() [N2], BN_mask_bits() [N4 bn_lib.c,
digest.c, evp_enc.c], dh_cms_set_peerkey() [N5, dh_ameth.c] and
cms_env_set_version() [N6, cms_env.c] are all OpenSSL issues and should
be reported to OpenSSL. The Windows build system also builds a static version of OpenSSL and a couple of other dependencies.
msg271205 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-07-25 02:03
I'm attaching Christian's patch at https://marc.info/?l=python-dev&m=146922730716425&w=2 as issue27587_pystate_addmodule.diff to make code reviewing easier.
msg272666 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2016-08-14 11:33
Pavel did another analysis with the external packages removed. Thanks
for this!

  http://www.viva64.com/en/b/0418/


The new analysis found another glitch.  Also see my message to
python-committers.
msg272669 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2016-08-14 12:09
Sorry, I missed issue27587_pystate_addmodule.diff: no new issue in the
updated analysis.
msg273026 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-08-18 13:10
Here is an updated patch.
msg273045 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-08-18 16:21
issue27587_pystate_addmodule_v2.diff LGTM.
msg273379 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-08-22 15:06
New changeset 51627344fc58 by Berker Peksag in branch '3.5':
Issue #27587: Move null pointer check earlier in _PyState_AddModule()
https://hg.python.org/cpython/rev/51627344fc58

New changeset 7d90bf4780ff by Berker Peksag in branch 'default':
Issue #27587: Merge from 3.5
https://hg.python.org/cpython/rev/7d90bf4780ff
History
Date User Action Args
2016-08-22 15:06:49berker.peksagsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2016-08-22 15:06:15python-devsetnosy: + python-dev
messages: + msg273379
2016-08-18 16:21:38vstinnersetmessages: + msg273045
2016-08-18 13:10:53berker.peksagsetfiles: + issue27587_pystate_addmodule_v2.diff

messages: + msg273026
2016-08-14 12:09:29skrahsetmessages: + msg272669
2016-08-14 11:33:52skrahsetnosy: + skrah
messages: + msg272666
2016-07-25 02:03:54berker.peksagsetfiles: + issue27587_pystate_addmodule.diff
keywords: + patch
2016-07-25 02:03:42berker.peksagsetmessages: + msg271205
stage: needs patch -> patch review
2016-07-23 13:57:36Rosuavsetnosy: + Rosuav
2016-07-23 13:47:15christian.heimessetnosy: + christian.heimes
messages: + msg271079
2016-07-23 00:15:07martin.pantersetmessages: + msg271046
2016-07-22 23:55:28martin.pantersetmessages: + msg271042
2016-07-22 17:42:12vstinnersetnosy: + vstinner
2016-07-22 17:31:36berker.peksagsetversions: + Python 3.5, Python 3.6
nosy: + berker.peksag

messages: + msg271002

stage: needs patch
2016-07-22 16:49:48cvrebertsetnosy: + cvrebert
2016-07-22 08:14:56martin.pantersetnosy: + paul.moore, tim.golden, martin.panter, zach.ware, steve.dower
messages: + msg270978

components: + Windows
type: enhancement -> compile error
2016-07-22 07:22:12pavel-belikovcreate