[pypy-commit] cffi default: Issue 364

arigo pypy.commits at gmail.com
Sat Jun 16 06:08:38 EDT 2018


Author: Armin Rigo <arigo at tunes.org>
Branch: 
Changeset: r3122:24bc0029493c
Date: 2018-06-16 12:08 +0200
http://bitbucket.org/cffi/cffi/changeset/24bc0029493c/

Log:	Issue 364

	Add support for MSVC's "#pragma pack(n)" in ABI mode

diff --git a/c/_cffi_backend.c b/c/_cffi_backend.c
--- a/c/_cffi_backend.c
+++ b/c/_cffi_backend.c
@@ -4712,6 +4712,12 @@
 #define SF_PACKED             0x08
 #define SF_STD_FIELD_POS      0x80
 
+#ifdef MS_WIN32
+#  define SF_DEFAULT_PACKING     8
+#else
+#  define SF_DEFAULT_PACKING   0x40000000   /* a huge power of two */
+#endif
+
 static int complete_sflags(int sflags)
 {
     /* add one of the SF_xxx_BITFIELDS flags if none is specified */
@@ -4771,14 +4777,22 @@
     CFieldObject **previous;
     int prev_bitfield_size, prev_bitfield_free;
     int sflags = 0, fflags;
-
-    if (!PyArg_ParseTuple(args, "O!O!|Onii:complete_struct_or_union",
+    int pack = 0;
+
+    if (!PyArg_ParseTuple(args, "O!O!|Oniii:complete_struct_or_union",
                           &CTypeDescr_Type, &ct,
                           &PyList_Type, &fields,
-                          &ignored, &totalsize, &totalalignment, &sflags))
+                          &ignored, &totalsize, &totalalignment, &sflags,
+                          &pack))
         return NULL;
 
     sflags = complete_sflags(sflags);
+    if (sflags & SF_PACKED)
+        pack = 1;
+    else if (pack <= 0)
+        pack = SF_DEFAULT_PACKING;
+    else
+        sflags |= SF_PACKED;
 
     if ((ct->ct_flags & (CT_STRUCT|CT_IS_OPAQUE)) ==
                         (CT_STRUCT|CT_IS_OPAQUE)) {
@@ -4841,7 +4855,7 @@
         falignorg = get_alignment(ftype);
         if (falignorg < 0)
             goto error;
-        falign = (sflags & SF_PACKED) ? 1 : falignorg;
+        falign = (pack < falignorg) ? pack : falignorg;
 
         do_align = 1;
         if (!(sflags & SF_GCC_ARM_BITFIELDS) && fbitsize >= 0) {
diff --git a/c/test_c.py b/c/test_c.py
--- a/c/test_c.py
+++ b/c/test_c.py
@@ -3552,30 +3552,50 @@
     BLong = new_primitive_type("long")
     BChar = new_primitive_type("char")
     BShort = new_primitive_type("short")
-    BStruct = new_struct_type("struct foo")
-    complete_struct_or_union(BStruct, [('a1', BLong, -1),
-                                       ('a2', BChar, -1),
-                                       ('a3', BShort, -1)],
-                             None, -1, -1, SF_PACKED)
-    d = BStruct.fields
-    assert len(d) == 3
-    assert d[0][0] == 'a1'
-    assert d[0][1].type is BLong
+    for extra_args in [(SF_PACKED,), (0, 1)]:
+        BStruct = new_struct_type("struct foo")
+        complete_struct_or_union(BStruct, [('a1', BLong, -1),
+                                           ('a2', BChar, -1),
+                                           ('a3', BShort, -1)],
+                                 None, -1, -1, *extra_args)
+        d = BStruct.fields
+        assert len(d) == 3
+        assert d[0][0] == 'a1'
+        assert d[0][1].type is BLong
+        assert d[0][1].offset == 0
+        assert d[0][1].bitshift == -1
+        assert d[0][1].bitsize == -1
+        assert d[1][0] == 'a2'
+        assert d[1][1].type is BChar
+        assert d[1][1].offset == sizeof(BLong)
+        assert d[1][1].bitshift == -1
+        assert d[1][1].bitsize == -1
+        assert d[2][0] == 'a3'
+        assert d[2][1].type is BShort
+        assert d[2][1].offset == sizeof(BLong) + sizeof(BChar)
+        assert d[2][1].bitshift == -1
+        assert d[2][1].bitsize == -1
+        assert sizeof(BStruct) == sizeof(BLong) + sizeof(BChar) + sizeof(BShort)
+        assert alignof(BStruct) == 1
+    #
+    BStruct2 = new_struct_type("struct foo")
+    complete_struct_or_union(BStruct2, [('b1', BChar, -1),
+                                        ('b2', BLong, -1)],
+                             None, -1, -1, 0, 2)
+    d = BStruct2.fields
+    assert len(d) == 2
+    assert d[0][0] == 'b1'
+    assert d[0][1].type is BChar
     assert d[0][1].offset == 0
     assert d[0][1].bitshift == -1
     assert d[0][1].bitsize == -1
-    assert d[1][0] == 'a2'
-    assert d[1][1].type is BChar
-    assert d[1][1].offset == sizeof(BLong)
+    assert d[1][0] == 'b2'
+    assert d[1][1].type is BLong
+    assert d[1][1].offset == 2
     assert d[1][1].bitshift == -1
     assert d[1][1].bitsize == -1
-    assert d[2][0] == 'a3'
-    assert d[2][1].type is BShort
-    assert d[2][1].offset == sizeof(BLong) + sizeof(BChar)
-    assert d[2][1].bitshift == -1
-    assert d[2][1].bitsize == -1
-    assert sizeof(BStruct) == sizeof(BLong) + sizeof(BChar) + sizeof(BShort)
-    assert alignof(BStruct) == 1
+    assert sizeof(BStruct2) == 2 + sizeof(BLong)
+    assert alignof(BStruct2) == 2
 
 def test_packed_with_bitfields():
     if sys.platform == "win32":
diff --git a/cffi/api.py b/cffi/api.py
--- a/cffi/api.py
+++ b/cffi/api.py
@@ -96,18 +96,21 @@
             self.CData, self.CType = backend._get_types()
         self.buffer = backend.buffer
 
-    def cdef(self, csource, override=False, packed=False):
+    def cdef(self, csource, override=False, packed=False, pack=None):
         """Parse the given C source.  This registers all declared functions,
         types, and global variables.  The functions and global variables can
         then be accessed via either 'ffi.dlopen()' or 'ffi.verify()'.
         The types can be used in 'ffi.new()' and other functions.
         If 'packed' is specified as True, all structs declared inside this
         cdef are packed, i.e. laid out without any field alignment at all.
+        Alternatively, 'pack' can be a small integer, and requests for
+        alignment greater than that are ignored (pack=1 is equivalent to
+        packed=True).
         """
-        self._cdef(csource, override=override, packed=packed)
+        self._cdef(csource, override=override, packed=packed, pack=pack)
 
-    def embedding_api(self, csource, packed=False):
-        self._cdef(csource, packed=packed, dllexport=True)
+    def embedding_api(self, csource, packed=False, pack=None):
+        self._cdef(csource, packed=packed, pack=pack, dllexport=True)
         if self._embedding is None:
             self._embedding = ''
 
diff --git a/cffi/backend_ctypes.py b/cffi/backend_ctypes.py
--- a/cffi/backend_ctypes.py
+++ b/cffi/backend_ctypes.py
@@ -730,7 +730,8 @@
         return self._new_struct_or_union('union', name, ctypes.Union)
 
     def complete_struct_or_union(self, CTypesStructOrUnion, fields, tp,
-                                 totalsize=-1, totalalignment=-1, sflags=0):
+                                 totalsize=-1, totalalignment=-1, sflags=0,
+                                 pack=0):
         if totalsize >= 0 or totalalignment >= 0:
             raise NotImplementedError("the ctypes backend of CFFI does not support "
                                       "structures completed by verify(); please "
@@ -751,6 +752,8 @@
                 bfield_types[fname] = Ellipsis
         if sflags & 8:
             struct_or_union._pack_ = 1
+        elif pack:
+            struct_or_union._pack_ = pack
         struct_or_union._fields_ = cfields
         CTypesStructOrUnion._bfield_types = bfield_types
         #
diff --git a/cffi/cparser.py b/cffi/cparser.py
--- a/cffi/cparser.py
+++ b/cffi/cparser.py
@@ -306,11 +306,25 @@
             msg = 'parse error\n%s' % (msg,)
         raise CDefError(msg)
 
-    def parse(self, csource, override=False, packed=False, dllexport=False):
+    def parse(self, csource, override=False, packed=False, pack=None,
+                    dllexport=False):
+        if packed:
+            if packed != True:
+                raise ValueError("'packed' should be False or True; use "
+                                 "'pack' to give another value")
+            if pack:
+                raise ValueError("cannot give both 'pack' and 'packed'")
+            pack = 1
+        elif pack:
+            if pack & (pack - 1):
+                raise ValueError("'pack' must be a power of two, not %r" %
+                    (pack,))
+        else:
+            pack = 0
         prev_options = self._options
         try:
             self._options = {'override': override,
-                             'packed': packed,
+                             'packed': pack,
                              'dllexport': dllexport}
             self._internal_parse(csource)
         finally:
diff --git a/cffi/model.py b/cffi/model.py
--- a/cffi/model.py
+++ b/cffi/model.py
@@ -342,7 +342,7 @@
     fixedlayout = None
     completed = 0
     partial = False
-    packed = False
+    packed = 0
 
     def __init__(self, name, fldnames, fldtypes, fldbitsize, fldquals=None):
         self.name = name
@@ -414,11 +414,14 @@
             fldtypes = [tp.get_cached_btype(ffi, finishlist)
                         for tp in self.fldtypes]
             lst = list(zip(self.fldnames, fldtypes, self.fldbitsize))
-            sflags = 0
+            extra_flags = ()
             if self.packed:
-                sflags = 8    # SF_PACKED
+                if self.packed == 1:
+                    extra_flags = (8,)    # SF_PACKED
+                else:
+                    extra_flags = (0, self.packed)
             ffi._backend.complete_struct_or_union(BType, lst, self,
-                                                  -1, -1, sflags)
+                                                  -1, -1, *extra_flags)
             #
         else:
             fldtypes = []
diff --git a/cffi/recompiler.py b/cffi/recompiler.py
--- a/cffi/recompiler.py
+++ b/cffi/recompiler.py
@@ -893,6 +893,12 @@
             else:
                 flags.append("_CFFI_F_CHECK_FIELDS")
             if tp.packed:
+                if tp.packed > 1:
+                    raise NotImplementedError(
+                        "%r is declared with 'pack=%r'; only 0 or 1 are "
+                        "supported in API mode (try to use \"...;\", which "
+                        "does not require a 'pack' declaration)" %
+                        (tp, tp.packed))
                 flags.append("_CFFI_F_PACKED")
         else:
             flags.append("_CFFI_F_EXTERNAL")
diff --git a/doc/source/cdef.rst b/doc/source/cdef.rst
--- a/doc/source/cdef.rst
+++ b/doc/source/cdef.rst
@@ -214,8 +214,9 @@
 slow to call ``ffi.cdef()`` a lot of times, a consideration that is
 important mainly in in-line mode.
 
-The ``ffi.cdef()`` call takes an optional
-argument ``packed``: if True, then all structs declared within
+The ``ffi.cdef()`` call optionally takes an extra argument: either
+``packed`` or ``pack``.  If you pass ``packed=True``,
+then all structs declared within
 this cdef are "packed".  (If you need both packed and non-packed
 structs, use several cdefs in sequence.)  This
 has a meaning similar to ``__attribute__((packed))`` in GCC.  It
@@ -224,6 +225,13 @@
 far, which mean that they may be packed differently than on GCC.
 Also, this has no effect on structs declared with ``"...;"``---more
 about it later in `Letting the C compiler fill the gaps`_.)
+*New in version 1.12:*  In ABI mode, you can also pass ``pack=n``,
+with an integer ``n`` which must be a power of two.  Then the
+alignment of any field is limited to ``n`` if it would otherwise be
+greater than ``n``.  Passing ``pack=1`` is equivalent to passing
+``packed=True``.  This is meant to emulate ``#pragma pack(n)`` from
+the MSVC compiler.  On Windows, the default is ``pack=8`` (from cffi
+1.12 onwards); on other platforms, the default is ``pack=None``.
 
 Note that you can use the type-qualifiers ``const`` and ``restrict``
 (but not ``__restrict`` or ``__restrict__``) in the ``cdef()``, but
diff --git a/doc/source/whatsnew.rst b/doc/source/whatsnew.rst
--- a/doc/source/whatsnew.rst
+++ b/doc/source/whatsnew.rst
@@ -3,6 +3,18 @@
 ======================
 
 
+v1.12
+=====
+
+* Support for ``ffi.cdef(..., pack=N)`` where N is a power of two.
+  Means to emulate ``#pragma pack(N)`` on MSVC.  Also, the default on
+  Windows is now ``pack=8``, like on MSVC.  This might make a difference
+  in corner cases, although I can't think of one in the context of CFFI.
+  The old way ``ffi.cdef(..., packed=True)`` remains and is equivalent
+  to ``pack=1`` (saying e.g. that fields like ``int`` should be aligned
+  to 1 byte instead of 4).
+
+
 v1.11.5
 =======
 
diff --git a/testing/cffi0/backend_tests.py b/testing/cffi0/backend_tests.py
--- a/testing/cffi0/backend_tests.py
+++ b/testing/cffi0/backend_tests.py
@@ -1823,19 +1823,39 @@
         ffi = FFI(backend=self.Backend())
         ffi.cdef("struct nonpacked { char a; int b; };")
         ffi.cdef("struct is_packed { char a; int b; };", packed=True)
+        ffi.cdef("struct is_packed1 { char a; int b; };", pack=1)
+        ffi.cdef("struct is_packed2 { char a; int b; };", pack=2)
+        ffi.cdef("struct is_packed4 { char a; int b; };", pack=4)
+        ffi.cdef("struct is_packed8 { char a; int b; };", pack=8)
         assert ffi.sizeof("struct nonpacked") == 8
         assert ffi.sizeof("struct is_packed") == 5
+        assert ffi.sizeof("struct is_packed1") == 5
+        assert ffi.sizeof("struct is_packed2") == 6
+        assert ffi.sizeof("struct is_packed4") == 8
+        assert ffi.sizeof("struct is_packed8") == 8
         assert ffi.alignof("struct nonpacked") == 4
         assert ffi.alignof("struct is_packed") == 1
-        s = ffi.new("struct is_packed[2]")
-        s[0].b = 42623381
-        s[0].a = b'X'
-        s[1].b = -4892220
-        s[1].a = b'Y'
-        assert s[0].b == 42623381
-        assert s[0].a == b'X'
-        assert s[1].b == -4892220
-        assert s[1].a == b'Y'
+        assert ffi.alignof("struct is_packed1") == 1
+        assert ffi.alignof("struct is_packed2") == 2
+        assert ffi.alignof("struct is_packed4") == 4
+        assert ffi.alignof("struct is_packed8") == 4
+        for name in ['is_packed', 'is_packed1', 'is_packed2',
+                     'is_packed4', 'is_packed8']:
+            s = ffi.new("struct %s[2]" % name)
+            s[0].b = 42623381
+            s[0].a = b'X'
+            s[1].b = -4892220
+            s[1].a = b'Y'
+            assert s[0].b == 42623381
+            assert s[0].a == b'X'
+            assert s[1].b == -4892220
+            assert s[1].a == b'Y'
+
+    def test_pack_valueerror(self):
+        ffi = FFI(backend=self.Backend())
+        py.test.raises(ValueError, ffi.cdef, "", pack=3)
+        py.test.raises(ValueError, ffi.cdef, "", packed=2)
+        py.test.raises(ValueError, ffi.cdef, "", packed=True, pack=1)
 
     def test_define_integer_constant(self):
         ffi = FFI(backend=self.Backend())
diff --git a/testing/cffi1/test_recompiler.py b/testing/cffi1/test_recompiler.py
--- a/testing/cffi1/test_recompiler.py
+++ b/testing/cffi1/test_recompiler.py
@@ -2242,6 +2242,12 @@
         "'API mode' and non-variadic (i.e. declared inside ffibuilder.cdef()"
         "+ffibuilder.set_source() and not taking a final '...' argument)")
 
+def test_pack_not_supported():
+    ffi = FFI()
+    ffi.cdef("""struct foo { char y; int x; };""", pack=2)
+    py.test.raises(NotImplementedError, verify,
+                   ffi, "test_pack_not_supported", "")
+
 def test_gcc_visibility_hidden():
     if sys.platform == 'win32':
         py.test.skip("test for gcc/clang")


More information about the pypy-commit mailing list