Issues, reported by PVS-Studio static analyzer

[pavel-belikov]

BPO	27587
Nosy	@pfmoore, @vstinner, @tiran, @tjguk, @skrah, @Rosuav, @berkerpeksag, @vadmium, @zware, @zooba
Files	issue27587_pystate_addmodule.diff issue27587_pystate_addmodule_v2.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2016-08-22.15:06:49.347>
created_at = <Date 2016-07-22.07:22:12.274>
labels = ['build', 'OS-windows']
title = 'Issues, reported by PVS-Studio static analyzer'
updated_at = <Date 2016-08-22.15:06:49.346>
user = 'https://bugs.python.org/pavel-belikov'

bugs.python.org fields:

activity = <Date 2016-08-22.15:06:49.346>
actor = 'berker.peksag'
assignee = 'none'
closed = True
closed_date = <Date 2016-08-22.15:06:49.347>
closer = 'berker.peksag'
components = ['Windows']
creation = <Date 2016-07-22.07:22:12.274>
creator = 'pavel-belikov'
dependencies = []
files = ['43870', '44141']
hgrepos = []
issue_num = 27587
keywords = ['patch']
message_count = 12.0
messages = ['270974', '270978', '271002', '271042', '271046', '271079', '271205', '272666', '272669', '273026', '273045', '273379']
nosy_count = 13.0
nosy_names = ['paul.moore', 'vstinner', 'christian.heimes', 'tim.golden', 'cvrebert', 'skrah', 'python-dev', 'Rosuav', 'berker.peksag', 'martin.panter', 'zach.ware', 'steve.dower', 'pavel-belikov']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'compile error'
url = 'https://bugs.python.org/issue27587'
versions = ['Python 3.5', 'Python 3.6']

[pavel-belikov]

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.

[vadmium]

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).

[berkerpeksag]

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 bpo-27591.

[vadmium]

Christian Heimes posted a patch for _PyState_AddModule() on Python-dev: https://marc.info/?l=python-dev&m=146922730716425&w=2

[vadmium]

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.

[tiran]

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.

[berkerpeksag]

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.

[skrah]

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.

[skrah]

Sorry, I missed issue27587_pystate_addmodule.diff: no new issue in the
updated analysis.

[berkerpeksag]

Here is an updated patch.

[vstinner]

issue27587_pystate_addmodule_v2.diff LGTM.

[python-dev]

New changeset 51627344fc58 by Berker Peksag in branch '3.5':
Issue bpo-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 bpo-27587: Merge from 3.5
https://hg.python.org/cpython/rev/7d90bf4780ff