diff options
author | Stefan Behnel <stefan_ml@behnel.de> | 2018-09-29 09:32:09 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-09-29 09:32:09 +0200 |
commit | a6990434f908b9aa075a79ebe23d3daf0efccaa8 (patch) | |
tree | ec7878801f222ab944a612ac191c3248b1fe0d7a | |
parent | ba9d273aa7526b179dc4f5cb6b65ceedc5b41210 (diff) | |
parent | 9e7fdef1de7b185f3c6925cecefea746e6e83853 (diff) | |
download | cython-a6990434f908b9aa075a79ebe23d3daf0efccaa8.tar.gz |
Merge pull request #2627 from mattip/check_size2
ENH: add check_size option to ctypedef class for external classes
-rw-r--r-- | CHANGES.rst | 4 | ||||
-rw-r--r-- | Cython/Compiler/ModuleNode.py | 22 | ||||
-rw-r--r-- | Cython/Compiler/Nodes.py | 4 | ||||
-rw-r--r-- | Cython/Compiler/Parsing.py | 18 | ||||
-rw-r--r-- | Cython/Compiler/PyrexTypes.py | 6 | ||||
-rw-r--r-- | Cython/Compiler/Symtab.py | 6 | ||||
-rw-r--r-- | Cython/Utility/ImportExport.c | 37 | ||||
-rw-r--r-- | docs/src/userguide/extension_types.rst | 28 | ||||
-rw-r--r-- | tests/run/check_size.srctree | 84 |
9 files changed, 181 insertions, 28 deletions
diff --git a/CHANGES.rst b/CHANGES.rst index 784e1a235..d67cdab95 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -85,6 +85,10 @@ Features added * Constants in ``libc.math`` are now declared as ``const`` to simplify their handling. +* An additional ``check_size`` clause was added to the ``ctypedef class`` name + specification to allow suppressing warnings when importing modules with + backward-compatible `PyTypeObject`` size changes. + Bugs fixed ---------- diff --git a/Cython/Compiler/ModuleNode.py b/Cython/Compiler/ModuleNode.py index f170803fd..6764e6db9 100644 --- a/Cython/Compiler/ModuleNode.py +++ b/Cython/Compiler/ModuleNode.py @@ -3059,10 +3059,24 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode): else: code.put('sizeof(%s), ' % objstruct) - code.putln('%i); if (!%s) %s' % ( - not type.is_external or type.is_subclassed, - type.typeptr_cname, - error_code)) + # check_size + if not type.is_external or type.is_subclassed: + if type.check_size != 'min': + raise AttributeError("unexpected check_size value '%s' when " + "compiling %s.%s" % (type.check_size, module_name, type.name)) + cs = 0 + elif type.check_size == 'min': + cs = 1 + elif type.check_size == True: + cs = 0 + elif type.check_size == False: + cs = 2 + else: + raise AttributeError("invalid value for check_size '%s' when compiling " + "%s.%s" % (type.check_size, module_name, type.name)) + code.putln('%d);' % cs) + + code.putln(' if (!%s) %s' % (type.typeptr_cname, error_code)) def generate_type_ready_code(self, entry, code): Nodes.CClassDefNode.generate_type_ready_code(entry, code) diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py index 9bbd7bb2f..b36a7c1dd 100644 --- a/Cython/Compiler/Nodes.py +++ b/Cython/Compiler/Nodes.py @@ -4629,6 +4629,7 @@ class CClassDefNode(ClassDefNode): # bases TupleNode Base class(es) # objstruct_name string or None Specified C name of object struct # typeobj_name string or None Specified C name of type object + # check_size 'min' or boolean What to do if tp_basicsize does not match # in_pxd boolean Is in a .pxd file # decorators [DecoratorNode] list of decorators or None # doc string or None @@ -4645,6 +4646,7 @@ class CClassDefNode(ClassDefNode): api = False objstruct_name = None typeobj_name = None + check_size = 'min' decorators = None shadow = False @@ -4680,6 +4682,7 @@ class CClassDefNode(ClassDefNode): typeobj_cname=self.typeobj_name, visibility=self.visibility, typedef_flag=self.typedef_flag, + check_size = self.check_size, api=self.api, buffer_defaults=self.buffer_defaults(env), shadow=self.shadow) @@ -4765,6 +4768,7 @@ class CClassDefNode(ClassDefNode): base_type=self.base_type, objstruct_cname=self.objstruct_name, typeobj_cname=self.typeobj_name, + check_size=self.check_size, visibility=self.visibility, typedef_flag=self.typedef_flag, api=self.api, diff --git a/Cython/Compiler/Parsing.py b/Cython/Compiler/Parsing.py index d47677587..25a7b97fb 100644 --- a/Cython/Compiler/Parsing.py +++ b/Cython/Compiler/Parsing.py @@ -3471,6 +3471,7 @@ def p_c_class_definition(s, pos, ctx): objstruct_name = None typeobj_name = None bases = None + check_size = 'min' if s.sy == '(': positional_args, keyword_args = p_call_parse_args(s, allow_genexp=False) if keyword_args: @@ -3482,7 +3483,7 @@ def p_c_class_definition(s, pos, ctx): if s.sy == '[': if ctx.visibility not in ('public', 'extern') and not ctx.api: error(s.position(), "Name options only allowed for 'public', 'api', or 'extern' C class") - objstruct_name, typeobj_name = p_c_class_options(s) + objstruct_name, typeobj_name, check_size = p_c_class_options(s) if s.sy == ':': if ctx.level == 'module_pxd': body_level = 'c_class_pxd' @@ -3521,6 +3522,7 @@ def p_c_class_definition(s, pos, ctx): bases = bases, objstruct_name = objstruct_name, typeobj_name = typeobj_name, + check_size = check_size, in_pxd = ctx.level == 'module_pxd', doc = doc, body = body) @@ -3528,6 +3530,7 @@ def p_c_class_definition(s, pos, ctx): def p_c_class_options(s): objstruct_name = None typeobj_name = None + check_size = 'min' s.expect('[') while 1: if s.sy != 'IDENT': @@ -3538,11 +3541,22 @@ def p_c_class_options(s): elif s.systring == 'type': s.next() typeobj_name = p_ident(s) + elif s.systring == 'check_size': + s.next() + check_size = p_ident(s) + if check_size == 'False': + check_size = False + elif check_size == 'True': + check_size = True + elif check_size == 'min': + pass + else: + s.error('Expected False, True, or min, not %r' % check_size) if s.sy != ',': break s.next() s.expect(']', "Expected 'object' or 'type'") - return objstruct_name, typeobj_name + return objstruct_name, typeobj_name, check_size def p_property_decl(s): diff --git a/Cython/Compiler/PyrexTypes.py b/Cython/Compiler/PyrexTypes.py index 9b6b02456..548b8f98d 100644 --- a/Cython/Compiler/PyrexTypes.py +++ b/Cython/Compiler/PyrexTypes.py @@ -1345,14 +1345,15 @@ class PyExtensionType(PyObjectType): # vtable_cname string Name of C method table definition # early_init boolean Whether to initialize early (as opposed to during module execution). # defered_declarations [thunk] Used to declare class hierarchies in order - + # check_size 'min' or boolean What to do if tp_basicsize does not match is_extension_type = 1 has_attributes = 1 early_init = 1 objtypedef_cname = None - def __init__(self, name, typedef_flag, base_type, is_external=0): + def __init__(self, name, typedef_flag, base_type, is_external=0, + check_size='min'): self.name = name self.scope = None self.typedef_flag = typedef_flag @@ -1368,6 +1369,7 @@ class PyExtensionType(PyObjectType): self.vtabptr_cname = None self.vtable_cname = None self.is_external = is_external + self.check_size = check_size self.defered_declarations = [] def set_scope(self, scope): diff --git a/Cython/Compiler/Symtab.py b/Cython/Compiler/Symtab.py index eab11e05f..7221c3875 100644 --- a/Cython/Compiler/Symtab.py +++ b/Cython/Compiler/Symtab.py @@ -1481,7 +1481,8 @@ class ModuleScope(Scope): def declare_c_class(self, name, pos, defining = 0, implementing = 0, module_name = None, base_type = None, objstruct_cname = None, - typeobj_cname = None, typeptr_cname = None, visibility = 'private', typedef_flag = 0, api = 0, + typeobj_cname = None, typeptr_cname = None, visibility = 'private', + typedef_flag = 0, api = 0, check_size='min', buffer_defaults = None, shadow = 0): # If this is a non-extern typedef class, expose the typedef, but use # the non-typedef struct internally to avoid needing forward @@ -1514,7 +1515,8 @@ class ModuleScope(Scope): # Make a new entry if needed # if not entry or shadow: - type = PyrexTypes.PyExtensionType(name, typedef_flag, base_type, visibility == 'extern') + type = PyrexTypes.PyExtensionType(name, typedef_flag, base_type, + visibility == 'extern', check_size=check_size) type.pos = pos type.buffer_defaults = buffer_defaults if objtypedef_cname is not None: diff --git a/Cython/Utility/ImportExport.c b/Cython/Utility/ImportExport.c index 40c383820..77ff3b850 100644 --- a/Cython/Utility/ImportExport.c +++ b/Cython/Utility/ImportExport.c @@ -308,15 +308,22 @@ set_path: /////////////// TypeImport.proto /////////////// -static PyTypeObject *__Pyx_ImportType(PyObject* module, const char *module_name, const char *class_name, size_t size, int strict); /*proto*/ + +static PyTypeObject *__Pyx_ImportType(PyObject* module, const char *module_name, const char *class_name, size_t size, int check_size); /*proto*/ /////////////// TypeImport /////////////// #ifndef __PYX_HAVE_RT_ImportType #define __PYX_HAVE_RT_ImportType static PyTypeObject *__Pyx_ImportType(PyObject *module, const char *module_name, const char *class_name, - size_t size, int strict) + size_t size, int check_size) { + /* + * check_size tells what to do if tp_basicsize is different from size: + * 0 - Error (originates in check_size=True) + * 1 - Error if tp_basicsize is smaller, warn if larger (originates in check_size='min') + * 2 - Error if tp_basicsize is smaller (originates in check_size=False) + */ PyObject *result = 0; char warning[200]; Py_ssize_t basicsize; @@ -345,18 +352,28 @@ static PyTypeObject *__Pyx_ImportType(PyObject *module, const char *module_name, if (basicsize == (Py_ssize_t)-1 && PyErr_Occurred()) goto bad; #endif - if (!strict && (size_t)basicsize > size) { - PyOS_snprintf(warning, sizeof(warning), - "%s.%s size changed, may indicate binary incompatibility. Expected %zd, got %zd", - module_name, class_name, basicsize, size); - if (PyErr_WarnEx(NULL, warning, 0) < 0) goto bad; + if ((size_t)basicsize < size) { + PyErr_Format(PyExc_ValueError, + "%.200s.%.200s size changed, may indicate binary incompatibility. " + "Expected %zd from C header, got %zd from PyObject", + module_name, class_name, size, basicsize); + goto bad; } - else if ((size_t)basicsize != size) { + if (check_size == 0 && (size_t)basicsize != size) { PyErr_Format(PyExc_ValueError, - "%.200s.%.200s has the wrong size, try recompiling. Expected %zd, got %zd", - module_name, class_name, basicsize, size); + "%.200s.%.200s size changed, may indicate binary incompatibility. " + "Expected %zd from C header, got %zd from PyObject", + module_name, class_name, size, basicsize); goto bad; } + else if (check_size == 1 && (size_t)basicsize > size) { + PyOS_snprintf(warning, sizeof(warning), + "%s.%s size changed, may indicate binary incompatibility. " + "Expected %zd from C header, got %zd from PyObject", + module_name, class_name, size, basicsize); + if (PyErr_WarnEx(NULL, warning, 0) < 0) goto bad; + } + /* check_size == 2 does not warn nor error */ return (PyTypeObject *)result; bad: Py_XDECREF(result); diff --git a/docs/src/userguide/extension_types.rst b/docs/src/userguide/extension_types.rst index 9b79c4517..62f9a502b 100644 --- a/docs/src/userguide/extension_types.rst +++ b/docs/src/userguide/extension_types.rst @@ -736,6 +736,14 @@ built-in complex object.:: ... } PyComplexObject; + At runtime, a check will be performed when importing the Cython + c-extension module that ``__builtin__.complex``'s ``tp_basicsize`` + matches ``sizeof(`PyComplexObject)``. This check can fail if the Cython + c-extension module was compiled with one version of the + ``complexobject.h`` header but imported into a Python with a changed + header. This check can be tweaked by using ``check_size`` in the name + specification clause. + 2. As well as the name of the extension type, the module in which its type object can be found is also specified. See the implicit importing section below. @@ -756,11 +764,23 @@ The part of the class declaration in square brackets is a special feature only available for extern or public extension types. The full form of this clause is:: - [object object_struct_name, type type_object_name ] + [object object_struct_name, type type_object_name, check_size cs_option] + +Where: + +- ``object_struct_name`` is the name to assume for the type's C struct. +- ``type_object_name`` is the name to assume for the type's statically + declared type object. +- ``cs_option`` is ``min`` (the default), ``True``, or ``False`` and is only + used for external extension types. If ``True``, ``sizeof(object_struct)`` must + match the type's ``tp_basicsize``. If ``False``, or ``min``, the + ``object_struct`` may be smaller than the type's ``tp_basicsize``, which + indicates the type allocated at runtime may be part of an updated module, and + that the external module's developers extended the object in a + backward-compatible fashion (only adding new fields to the end of the + object). If ``min``, a warning will be emitted. -where ``object_struct_name`` is the name to assume for the type's C struct, -and type_object_name is the name to assume for the type's statically declared -type object. (The object and type clauses can be written in either order.) +The clauses can be written in any order. If the extension type declaration is inside a :keyword:`cdef` extern from block, the object clause is required, because Cython must be able to generate diff --git a/tests/run/check_size.srctree b/tests/run/check_size.srctree index 1cc9912f8..8391534e4 100644 --- a/tests/run/check_size.srctree +++ b/tests/run/check_size.srctree @@ -4,6 +4,7 @@ PYTHON -c "import runner" ######## setup.py ######## from Cython.Build.Dependencies import cythonize +from Cython.Compiler.Errors import CompileError from distutils.core import setup # force the build order @@ -11,6 +12,12 @@ setup(ext_modules= cythonize("check_size.pyx")) setup(ext_modules = cythonize("_check_size*.pyx")) +try: + setup(ext_modules= cythonize("check_size6.pyx")) + assert False +except CompileError as e: + pass + ######## check_size_nominal.h ######## #include <Python.h> @@ -119,6 +126,58 @@ cdef extern from "check_size_smaller.h": cpdef public int testme(Foo f) except -1: return f.f9 +######## _check_size3.pyx ######## + +cdef extern from "check_size_smaller.h": + + # make sure missing check_size is equivalent to min + ctypedef class check_size.Foo [object FooStructSmall, check_size min]: + cdef: + int f9 + + +cpdef public int testme(Foo f) except -1: + return f.f9 + +######## _check_size4.pyx ######## + +cdef extern from "check_size_smaller.h": + + # Disable size check + ctypedef class check_size.Foo [object FooStructSmall, check_size False]: + cdef: + int f9 + + +cpdef public int testme(Foo f) except -1: + return f.f9 + +######## _check_size5.pyx ######## + +cdef extern from "check_size_smaller.h": + + # Strict checking, will raise an error + ctypedef class check_size.Foo [object FooStructSmall, check_size True]: + cdef: + int f9 + + +cpdef public int testme(Foo f) except -1: + return f.f9 + +######## check_size6.pyx ######## + +cdef extern from "check_size_smaller.h": + + # Raise Compilerror when using bad value + ctypedef class check_size.Foo [object FooStructSmall, check_size max]: + cdef: + int f9 + + +cpdef public int testme(Foo f) except -1: + return f.f9 + ######## runner.py ######## import check_size, _check_size0, warnings @@ -137,19 +196,36 @@ try: import _check_size1 assert False except ValueError as e: - assert str(e).startswith('check_size.Foo has the wrong size, try recompiling') + assert str(e).startswith('check_size.Foo size changed') # Warining since check_size.Foo's tp_basicsize is larger than what is needed # for FooStructSmall. There is "spare", accessing FooStructSmall's fields will # never access invalid memory. This can happen, for instance, when using old -# headers with a newer runtime, or when using an old _check_size2 with a newer +# headers with a newer runtime, or when using an old _check_size{2,3} with a newer # check_size, where the developers of check_size are careful to be backward # compatible. with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") import _check_size2 - assert len(w) == 1, 'expected one warning, got %d' % len(w) - assert str(w[-1].message).startswith('check_size.Foo size changed') + import _check_size3 + assert len(w) == 2, 'expected two warnings, got %d' % len(w) + assert str(w[0].message).startswith('check_size.Foo size changed') + assert str(w[1].message).startswith('check_size.Foo size changed') ret = _check_size2.testme(foo) assert ret == 23 +ret = _check_size3.testme(foo) +assert ret == 23 + +with warnings.catch_warnings(record=True) as w: + # No warning, runtime vendor must provide backward compatibility + import _check_size4 + assert len(w) == 0 + +try: + # Enforce strict checking + import _check_size5 + assert False +except ValueError as e: + assert str(e).startswith('check_size.Foo size changed') |