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

PyModuleDef_HEAD_INIT does not explicitly initialize all fields of m_base #53727

Closed
davidmalcolm opened this issue Aug 4, 2010 · 7 comments
Closed
Labels
build The build process and cross-build extension-modules C modules in the Modules dir

Comments

@davidmalcolm
Copy link
Member

BPO 9518
Nosy @loewis, @amauryfa, @pitrou, @scoder, @davidmalcolm
Files
  • py3k-initialize-all-of-m_base.patch
  • 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 2010-12-08.22:40:27.224>
    created_at = <Date 2010-08-04.23:43:31.087>
    labels = ['extension-modules', 'build']
    title = 'PyModuleDef_HEAD_INIT does not explicitly initialize all fields of m_base'
    updated_at = <Date 2010-12-08.22:40:49.219>
    user = 'https://github.com/davidmalcolm'

    bugs.python.org fields:

    activity = <Date 2010-12-08.22:40:49.219>
    actor = 'dmalcolm'
    assignee = 'none'
    closed = True
    closed_date = <Date 2010-12-08.22:40:27.224>
    closer = 'dmalcolm'
    components = ['Extension Modules']
    creation = <Date 2010-08-04.23:43:31.087>
    creator = 'dmalcolm'
    dependencies = []
    files = ['18396']
    hgrepos = []
    issue_num = 9518
    keywords = ['patch']
    message_count = 7.0
    messages = ['112926', '117553', '117560', '121372', '121382', '121385', '123658']
    nosy_count = 5.0
    nosy_names = ['loewis', 'amaury.forgeotdarc', 'pitrou', 'scoder', 'dmalcolm']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'compile error'
    url = 'https://bugs.python.org/issue9518'
    versions = ['Python 3.1', 'Python 3.2']

    @davidmalcolm
    Copy link
    Member Author

    Attempting to compile Python 3 extension modules on GCC with "-Wmissing-field-initializers" enabled leads to warnings from the PyModuleDef_HEAD_INIT macro

    Seen attempting to build SELinux python bindings against python 3.1 with "-W -Werror", the "-W" implies "-Wmissing-field-initializers":

    cc -Werror -Wall -W -Wundef -Wshadow -Wmissing-noreturn
    > -Wmissing-format-attribute -I../include -I/usr/include -D_GNU_SOURCE
    > -D_FILE_OFFSET_BITS=64 -I/usr/include/python3.1 -fPIC -DSHARED -c -o
    > audit2why.lo audit2why.c
    > cc1: warnings being treated as errors
    > audit2why.c:439: error: missing initializer
    > audit2why.c:439: error: (near initialization for ?moduledef.m_base.m_init¹)
    > make: *** [audit2why.lo] Error 1

    The PyModuleDef_HEAD_INIT is intended to initialize m_base within a PyModuleDef, but only explicitly initializes the PyObject_HEAD fields:

    #define PyModuleDef_HEAD_INIT {PyObject_HEAD_INIT(NULL)}
    
    typedef struct PyModuleDef_Base {
      PyObject_HEAD
      PyObject* (*m_init)(void);
      Py_ssize_t m_index;
      PyObject* m_copy;
    } PyModuleDef_Base;
    
    typedef struct PyModuleDef{
      PyModuleDef_Base m_base;
      const char* m_name;
      const char* m_doc;
      Py_ssize_t m_size;
      PyMethodDef *m_methods;
      inquiry m_reload;
      traverseproc m_traverse;
      inquiry m_clear;
      freefunc m_free;
    } PyModuleDef;

    The attached patch extends it to also explicitly zero the other m_base fields.

    @davidmalcolm davidmalcolm added extension-modules C modules in the Modules dir build The build process and cross-build labels Aug 4, 2010
    @davidmalcolm davidmalcolm changed the title PyModuleDef_HEAD_INIT does not explicitly initial all fields of m_base PyModuleDef_HEAD_INIT does not explicitly initialize all fields of m_base Aug 4, 2010
    @amauryfa
    Copy link
    Member

    the patch looks OK, but out of curiosity: do you really declare all the fields of a PyTypeObject?
    This structure is really designed so that newer members are left at the end; most types don't need to initialize them, C standard ensures that they will be zero.

    @davidmalcolm
    Copy link
    Member Author

    Thanks. The code in question is a wrapper to a security-sensitive library (user-space SELinux code), hence the compilation warnings have been turned up as much as possible.

    The .c code in question is generated by SWIG, and that does indeed appear to be writing out full initializers for PyTypeObject instances (and the other associated structs).

    It appears to be just Python 3's PyModuleDef_HEAD_INIT macro that leaves fields uninitialized (hence this patch).

    The gory details of the SWIG-generated code can be seen at:
    http://userspace.selinuxproject.org/trac/browser/libselinux/src/selinuxswig_wrap.c
    (and the .i files in that directory)

    Although it's not on by default gcc will issue a "missing initializer" warning when fields aren't initialized when "-Wmissing-field-initializers" is enabled (in this case, due to the use of "-W"). This becomes an error with -Werror.

    Whether or not this is a useful warning isn't clear to me, but it seems to be reasonable to suppress the warning given that as-is, people who use gcc's "-W" catch-all will run into this on all Python 3 modules, and that the patch is trivial, and that this case gives no warnings when building such code against Python 2.*

    Hope this makes sense.

    @scoder
    Copy link
    Contributor

    scoder commented Nov 17, 2010

    I agree that this is annoying, we get the same thing in Cython's test suite all over the place. Any foreign warning that doesn't get triggered helps in debugging your own code. And this one is easy to avoid.

    @pitrou
    Copy link
    Member

    pitrou commented Nov 17, 2010

    This looks ok to me.

    @davidmalcolm
    Copy link
    Member Author

    Fix committed to py3k as r86499

    @davidmalcolm
    Copy link
    Member Author

    Forgot to close this one out

    @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
    Labels
    build The build process and cross-build extension-modules C modules in the Modules dir
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants