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.

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

Fix maybe uninitialized value in get_value and get_value_no_default. #4361

Merged

Conversation

nicopauss
Copy link
Contributor

@nicopauss nicopauss commented Sep 2, 2021

ff16389 introduced convenient functions get_value() and get_value_no_default().

Unfortunately, the variable value in these functions was not set before usage with PyContextVar_Get().
This triggered some warnings:


Error compiling Cython file:
------------------------------------------------------------
...
    """Return a new reference to the value of the context variable,
    or the default value of the context variable,
    or None if no such value or default was found.
    """
    cdef PyObject *value
    PyContextVar_Get(var, NULL, &value)
                                ^
------------------------------------------------------------

Cython/Includes/cpython/contextvars.pxd:118:33: local variable 'value' might be referenced before assignment

Error compiling Cython file:
------------------------------------------------------------
...
    or the provided default value if no such value was found.

    Ignores the default value of the context variable, if any.
    """
    cdef PyObject *value
    PyContextVar_Get(var, <PyObject*>default_value, &value)
                                                    ^
------------------------------------------------------------

Cython/Includes/cpython/contextvars.pxd:136:53: local variable 'value' might be referenced before assignment

It can be replicated by simply importing cpython:
echo "cimport cpython" >/tmp/mod.pyx && ./cython.py -Werror -Wextra /tmp/mod.pyx

The solution is simply to assign NULL to value on declaration.

@nicopauss
Copy link
Contributor Author

The check fails don't seem related to my patch:

runTest (__main__.CythonCompileTestCase)
Error: The operation was canceled.

ff16389 introduced convenient functions get_value() and
get_value_no_default().

Unfortunately, the variable `value` in these functions was not set
before usage with PyContextVar_Get().
This triggered some warnings:

Error compiling Cython file:
------------------------------------------------------------
...
    """Return a new reference to the value of the context variable,
    or the default value of the context variable,
    or None if no such value or default was found.
    """
    cdef PyObject *value
    PyContextVar_Get(var, NULL, &value)
                                ^
------------------------------------------------------------

Cython/Includes/cpython/contextvars.pxd:118:33: local variable 'value' might be referenced before assignment

Error compiling Cython file:
------------------------------------------------------------
...
    or the provided default value if no such value was found.

    Ignores the default value of the context variable, if any.
    """
    cdef PyObject *value
    PyContextVar_Get(var, <PyObject*>default_value, &value)
                                                    ^
------------------------------------------------------------

Cython/Includes/cpython/contextvars.pxd:136:53: local variable 'value' might be referenced before assignment

It can be replicated by simply importing `cpython`:
echo "cimport cpython" >/tmp/mod.pyx && ./cython.py -Werror -Wextra /tmp/mod.pyx

The solution is simply to assign NULL to `value` on declaration.
@nicopauss nicopauss force-pushed the np-fix-maybe-uninitialized-contextvar branch from 925f001 to 6279cc9 Compare September 6, 2021 13:09
@da-woods
Copy link
Contributor

da-woods commented Sep 6, 2021

The check fails don't seem related to my patch:

runTest (__main__.CythonCompileTestCase)
Error: The operation was canceled.

This is most likely just one of the tests hitting a 30 min limit (a couple of tests are very close to the time limit) rather than anything to do with your PR

@scoder scoder added this to the 3.0 milestone Sep 6, 2021
@scoder
Copy link
Contributor

scoder commented Sep 6, 2021

Thanks

@scoder scoder merged commit 16a8247 into cython:master Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants