Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issues, reported by PVS-Studio static analyzer #71774

Closed
pavel-belikov mannequin opened this issue Jul 22, 2016 · 12 comments
Closed

Issues, reported by PVS-Studio static analyzer #71774

pavel-belikov mannequin opened this issue Jul 22, 2016 · 12 comments
Labels
build The build process and cross-build OS-windows

Comments

@pavel-belikov
Copy link
Mannequin

pavel-belikov mannequin commented Jul 22, 2016

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
    Copy link
    Mannequin Author

    pavel-belikov mannequin commented Jul 22, 2016

    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.

    @pavel-belikov pavel-belikov mannequin added the type-feature A feature request or enhancement label Jul 22, 2016
    @vadmium
    Copy link
    Member

    vadmium commented Jul 22, 2016

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

    @vadmium vadmium added OS-windows build The build process and cross-build and removed type-feature A feature request or enhancement labels Jul 22, 2016
    @berkerpeksag
    Copy link
    Member

    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
    Copy link
    Member

    vadmium commented Jul 22, 2016

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

    @vadmium
    Copy link
    Member

    vadmium commented Jul 23, 2016

    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
    Copy link
    Member

    tiran commented Jul 23, 2016

    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
    Copy link
    Member

    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
    Copy link
    Mannequin

    skrah mannequin commented Aug 14, 2016

    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
    Copy link
    Mannequin

    skrah mannequin commented Aug 14, 2016

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

    @berkerpeksag
    Copy link
    Member

    Here is an updated patch.

    @vstinner
    Copy link
    Member

    issue27587_pystate_addmodule_v2.diff LGTM.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 22, 2016

    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

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    build The build process and cross-build OS-windows
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @berkerpeksag @vstinner @tiran @vadmium