This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: abort when passing certain structs by value using ctypes
Type: crash Stage: resolved
Components: ctypes Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Ilya.Kulakov, alexei.romanov, amaury.forgeotdarc, belopolsky, eryksun, meador.inge, vinay.sajip, vstinner, weeble
Priority: high Keywords: patch

Created on 2014-08-25 17:33 by weeble, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
fix-22273-01.diff vinay.sajip, 2017-02-19 15:33 Test for structure with array seems to work on Windows, abort on LInux review
fix-22273-02.diff vinay.sajip, 2017-02-22 07:36 First cut patch to address issue.
fix-22273-03.diff vinay.sajip, 2017-02-23 09:04 Patch updated to address Eryk's comments.
Pull Requests
URL Status Linked Edit
PR 15839 closed vinay.sajip, 2019-09-10 11:36
PR 16369 merged miss-islington, 2019-09-25 03:40
PR 16370 merged miss-islington, 2019-09-25 03:40
PR 16377 merged vinay.sajip, 2019-09-25 06:35
PR 16381 merged vinay.sajip, 2019-09-25 09:51
PR 16388 merged vinay.sajip, 2019-09-25 13:20
PR 16399 merged vinay.sajip, 2019-09-25 19:31
PR 16400 merged vinay.sajip, 2019-09-25 20:16
PR 16401 merged vinay.sajip, 2019-09-25 21:19
PR 16492 merged vstinner, 2019-09-30 14:17
Messages (38)
msg225881 - (view) Author: Weeble (weeble) Date: 2014-08-25 17:33
I'm not 100% certain this is a bug yet, but I'm beginning to think it's likely.

On 64-bit Linux, I can't pass a struct like this:

    struct S { uint8_t data[16]; };

...to a function declared like this:

    void f(struct S);

From experimentation with various integer types and array sizes, it seems this causes an abort somewhere in libffi any time the array is between 9 and 16 bytes in size. If the array is smaller or larger than that, the calls work as expected.

I've asked about this here: http://stackoverflow.com/questions/25487928/is-this-the-correct-way-to-pass-a-struct-by-value-in-ctypes

Here's some test code:

## sum.cpp

    #include <cstdint>

    using std::size_t;

    struct ArrayStruct {
        // We'll define ARRAY_TYPE and ARRAY_SIZE on the
        // command-line when we compile.
        std::ARRAY_TYPE data[ARRAY_SIZE];
    };

    extern "C" int64_t sum(struct ArrayStruct array)
    {
        int64_t acc=0;
        for (size_t i=0; i!=ARRAY_SIZE; ++i)
        {
            acc+=array.data[i];
        }
        return acc;
    }

## sum.py
    import ctypes
    import sys

    def main():
        array_size = int(sys.argv[1])
        array_type = sys.argv[2]

        libsum = ctypes.cdll.LoadLibrary('./libsum.so')

        ArrType = getattr(ctypes, 'c_' + array_type) * array_size

        class MyStruct(ctypes.Structure):
            _fields_ = [("data", ArrType)]

        m=MyStruct()
        for i in range(array_size):
            m.data[i]=i

        print(libsum.sum(m))

    if __name__ == '__main__':
        main()

## Build/run

    $ g++ -g -shared -Wall -fPIC sum.cpp -o libsum.so -std=c++11 -D ARRAY_SIZE=16 -D ARRAY_TYPE=uint8_t && python3 sum.py 16 uint8
    Aborted (core dumped)

I poked around a little bit in gdb. It's aborting in libffi's "ffi_call" function: https://github.com/atgreen/libffi/blob/v3.0.13/src/x86/ffi64.c#L516

(gdb) bt
#0  0x00007ffff782cf79 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x00007ffff7830388 in __GI_abort () at abort.c:89
#2  0x00007ffff67134f5 in ffi_call (cif=0x7fffffffd7b0, fn=0x7ffff650c625 <sum(ArrayStruct)>, rvalue=0x7fffffffd6f0, avalue=0x7fffffffd6d0) at ../src/x86/ffi64.c:516
#3  0x00007ffff691fee3 in _ctypes_callproc () from /usr/lib/python3.4/lib-dynload/_ctypes.cpython-34m-x86_64-linux-gnu.so
#4  0x00007ffff6920578 in ?? () from /usr/lib/python3.4/lib-dynload/_ctypes.cpython-34m-x86_64-linux-gnu.so
#5  0x000000000043810a in PyObject_Call ()
#6  0x0000000000579f45 in PyEval_EvalFrameEx ()
#7  0x000000000057d3d3 in PyEval_EvalCodeEx ()
#8  0x000000000057bfaa in PyEval_EvalFrameEx ()
#9  0x000000000057d3d3 in PyEval_EvalCodeEx ()
#10 0x000000000060ba83 in PyRun_FileExFlags ()
#11 0x000000000060bc85 in PyRun_SimpleFileExFlags ()
#12 0x000000000060d3ac in Py_Main ()
#13 0x000000000041ec0d in main ()
(gdb) frame 2
#2  0x00007ffff67134f5 in ffi_call (cif=0x7fffffffd7b0, fn=0x7ffff650c625 <sum(ArrayStruct)>, rvalue=0x7fffffffd6f0, avalue=0x7fffffffd6d0) at ../src/x86/ffi64.c:516
516			  abort();
(gdb) info args
cif = 0x7fffffffd7b0
fn = 0x7ffff650c625 <sum(ArrayStruct)>
rvalue = 0x7fffffffd6f0
avalue = 0x7fffffffd6d0
(gdb) info locals
a = <optimized out>
j = <optimized out>
size = 8
n = <optimized out>
classes = {X86_64_INTEGER_CLASS, X86_64_NO_CLASS, 4294956784, 32767}
stack = 0x7fffffffd4f0 ""
argp = 0x7fffffffd5a0 "\001"
arg_types = 0x7fffffffd6b0
gprcount = 1
ssecount = <optimized out>
ngpr = 1
nsse = 0
i = <optimized out>
avn = <optimized out>
ret_in_memory = <optimized out>
reg_args = 0x7fffffffd4f0
(gdb) print *cif
$2 = {abi = FFI_UNIX64, nargs = 1, arg_types = 0x7fffffffd6b0, rtype = 0x7ffff6b5e228, bytes = 0, flags = 10}

It looks like we're trying to pass the struct in two registers, which I think is what's supposed to happen, but something is going wrong with the second register. It aborted because it has class X86_64_NO_CLASS and that's not handled by the switch.

I don't know if this is a bug in libffi, or if ctypes is feeding it bad information, or if I'm feeding ctypes bad information. I hope this information is useful for anyone investigating.

I get the same abort in both Python 2.7.6 and 3.4.0.

I originally stumbled across this issue trying to use PySDL2:

http://pysdl2.readthedocs.org/en/rel_0_9_3/

I was trying to call SDL_JoystickGetGUIDString, which uses a similar struct-by-value call:

http://hg.libsdl.org/SDL/file/92ca74200ea5/include/SDL_joystick.h
msg226045 - (view) Author: Weeble (weeble) Date: 2014-08-28 22:02
I had a closer look at the cif object in gdb. The ffi_type of the argument in question has size 16, alignment 1, type FFI_TYPE_STRUCT and elements contains a single nested ffi_type, of size 8, alignment 8, type FFI_TYPE_POINTER.

I think this pointer type is wrong. The struct should indeed be size 16, but its contents (in this case) should be 16 bytes of uint8s, rather than a single pointer. I'm not certain how to correctly describe an array to libffi. While you might be able to hack it with a nested struct filled with 16 individual integers, I have no idea if that would work consistently across platforms.
msg235598 - (view) Author: Ilya Kulakov (Ilya.Kulakov) * Date: 2015-02-09 09:17
The structure hack does not work on Windows 8, x64.
msg287881 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2017-02-15 19:12
ctypes defines arrays as a pointer FFI type because they degenerate as pointers in C calls. But it's generally wrong to set a pointer FFI type for an array in the `elements` of a struct's FFI type. An integer array in a struct that's 16 bytes or less should be packed in one or two general-purpose registers (rdi, rsi, rdx, rcx, r8, r9). 

For the example 16-byte struct, classify_argument() in ffi64.c expects to classify two 8-byte words. But the struct's FFI type only has one element, which we've incorrectly defined as a pointer element. Thus the second word is left at the default classification X86_64_NO_CLASS. Back in ffi_call() it expects two classified words, so it aborts when it sees X86_64_NO_CLASS.

I think we can special-case small arrays in PyCStructUnionType_update_stgdict when assigning the `elements` of the FFI type of a struct or union. If we have an array that's 32 bytes or less, unpack it as individual FFI elements, e.g. a c_ushort * 8 array would be stored as 8 ffi_type_uint16 elements in the struct's FFI type.
msg288142 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2017-02-19 15:33
> I think we can special-case small arrays in PyCStructUnionType_update_stgdict

Is that definitely the right place? And is doing it only for small arrays going to be enough? Currently, PyCStructUnionType_update_stgdict does

dict = PyType_stgdict(desc);

and then

stgdict->ffi_type_pointer.elements[ffi_ofs + i] = &dict->ffi_type_pointer;

where dict is the ctypes object for the field type. If the ffi_type_pointer is used all over the place because arrays usually degenerate to pointers, and changing it would cause breakage elsewhere, maybe the answer is to have a new ffi_type_array field which is NULL for non-array types and set correctly for array types; then the above code can check for a non-NULL ffi_type_array and use that instead of the ffi_type_pointer? Or am I talking nonsense?

Oddly (or perhaps not), this failure doesn't seem to occur on Windows - no crash happens and the correct value is returned from a function which sums the array, as in this example. See attached patch.
msg288143 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2017-02-19 15:35
I've not marked it "patch review" as the patch isn't complete. Just wanted to see if anyone can reproduce/explain the working on Windows/failing on Linux.
msg288163 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2017-02-19 20:29
Structs that are larger than 32 bytes get copied to the stack (see classify_argument in ffi64.c), so we don't have to worry about classifying their elements for register passing. Thus if a new field is added for this in StgDictObject, then PyCArrayType_new should only allocate it for array types that are 32 bytes or less. Using it for larger array types would serve no point.

> explain the working on Windows/failing on Linux.

In the Windows libffi we don't have examine_argument() and classify_argument(). The Win64 ABI is fairly simple [1]. A struct that's 8 bytes or less gets passed as an integer, so if it's in the first four arguments it gets passed in rcx, rdx, r8, or r9. Otherwise it gets copied and passed by reference. Unlike the 64-bit Unix ABI, we don't have to worry about packing struct elements across multiple registers or passing floating-point elements in vector registers.

[1]: https://msdn.microsoft.com/en-us/library/zthk2dkh.aspx
msg288221 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2017-02-20 17:10
Thanks for spelling it out for me, that's helpful. But I'm still confused about a couple of things: I can't find classify_argument in the Python source tree other than in

Modules/_ctypes/libffi_osx/x86/x86-ffi64.c

Is that the file you referred to as ffi64.c? I assumed this is only used on OS X. Do we just use the system libffi on Linux?

I also note that if I use the following:

typedef struct {
    int foo;
    int bar;
    unsigned char data[8];
} Test;

which is certainly the same size of struct, there's no abort and the sum is correctly calculated and returned as 28, which is printed by the Python script. If I swap things around so that the array comes first in the structure, that also works. If I increase the array size back to 16 (giving a total structure size of 24), that also works. If I then comment out the 'int foo' and 'int bar' fields in both C and Python, the abort reappears.
msg288237 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2017-02-20 20:34
Possibly also relevant: #16575 and

https://github.com/libffi/libffi/issues/33
msg288239 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2017-02-20 20:48
classify_argument is in Modules/_ctypes/libffi/src/x86/ffi64.c. This file is for the 64-bit Unix ABI. libffi doesn't use it for 64-bit Windows. 

Some (all?) Linux distros link ctypes to the system libffi. In my experience, building 3.6 on Linux defaults to the system libffi, and I don't personally know how to override this. Configuring "--without-system-ffi" seems to be ignored.

Regarding your Test struct example, it's ok in this particular case to classify an 8-byte integer array the same as an 8-byte pointer. It doesn't abort() because the 2nd word isn't left unclassified (i.e. X86_64_NO_CLASS). 

    import ctypes

    class Test(ctypes.Structure):
        _fields_ = (('foo', ctypes.c_int),
                    ('bar', ctypes.c_int),
                    ('data', ctypes.c_uint8 * 8))

    @ctypes.CFUNCTYPE(None, Test)
    def func(t):
        print('foo:', t.foo)
        print('bar:', t.bar)
        print('data:', t.data[:])

    t = Test(5, 10, tuple(range(8)))

    >>> hex(id(Test))
    '0x9d8ad8'

The ctypes Structure has 3 elements. The first two are ffi_type_sint32 (FFI_TYPE_SINT32 == 10), and the third is ffi_type_pointer (FFI_TYPE_POINTER == 14):

    (gdb) set $dict = (StgDictObject *)(((PyTypeObject *)0x9d8ad8)->tp_dict)
    (gdb) p *$dict->ffi_type_pointer->elements[0]
    $1 = {size = 4, alignment = 4, type = 10, elements = 0x0}
    (gdb) p *$dict->ffi_type_pointer->elements[1]
    $2 = {size = 4, alignment = 4, type = 10, elements = 0x0}
    (gdb) p *$dict->ffi_type_pointer->elements[2]
    $3 = {size = 8, alignment = 8, type = 14, elements = 0x0}
    (gdb) p $dict->ffi_type_pointer->elements[3]
    $4 = (struct _ffi_type *) 0x0

classify_argument() recursively classifies and merges these elements. The first two get merged as X86_64_INTEGER_CLASS, and the 'pointer' (actually an array) is X86_64_INTEGER_CLASS.

    >>> func(t)

    Breakpoint 1, ffi_call (cif=cif@entry=0x7fffffffd570, fn=fn@entry=0x7ffff7fee010,
        rvalue=rvalue@entry=0x7fffffffd630, avalue=avalue@entry=0x7fffffffd610)
        at ../src/x86/ffi64.c:424

    [...snip...]

    458	  for (i = 0; i < avn; ++i)
    (gdb) 
    462	      n = examine_argument (arg_types[i], classes, 0, &ngpr, &nsse);
    (gdb) 
    463	      if (n == 0
    (gdb) p n
    $6 = 2
    (gdb) p ngpr
    $7 = 2
    (gdb) p classes[0]
    $8 = X86_64_INTEGER_CLASS
    (gdb) p classes[1]
    $9 = X86_64_INTEGER_CLASS

The struct is passed in two general-purpose integer registers, rdi and rsi:

    Breakpoint 2, ffi_call_unix64 () at ../src/x86/unix64.S:49
    49		movq	(%rsp), %r10		/* Load return address.  */

    [...snip...]
    76		call	*%r11

    (gdb) p/x $rdi
    $10 = 0xa00000005
    (gdb) p/x $rsi
    $11 = 0x706050403020100
    (gdb) c
    Continuing.

    foo: 5
    bar: 10
    data: [0, 1, 2, 3, 4, 5, 6, 7]
msg288241 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2017-02-20 21:08
I see now why you couldn't find ffi64.c. I've been using a 3.6 worktree. The libffi sources have been removed from master.

For the union and bitfield problem, also see the crash reported in #26628.
msg288244 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2017-02-20 21:36
I'm learning a bit about Linux calling conventions :-)

But it also works when a 16-byte array is followed by 2 ints; if the two ints are removed, then it fails again.

ctypes sets elements up in the first case to be a FFI_TYPE_POINTER slot followed by two slots of FFI_TYPE_SINT32, and classify_argument seemingly does the right thing. But remove the two integers, and classify_argument seems to not do the right thing. Isn't this looking like a problem in classify_argument? In the first case:

p *stgdict->ffi_type_pointer.elements[0]
$3 = {size = 8, alignment = 8, type = 14, elements = 0x0}
p *stgdict->ffi_type_pointer.elements[1]
$4 = {size = 4, alignment = 4, type = 10, elements = 0x0}
p *stgdict->ffi_type_pointer.elements[2]
$5 = {size = 4, alignment = 4, type = 10, elements = 0x0}
p stgdict->ffi_type_pointer.elements[3]
$6 = (struct _ffi_type *) 0x0

and the second case:

p *stgdict->ffi_type_pointer.elements[0]
$2 = {size = 8, alignment = 8, type = 14, elements = 0x0}
p stgdict->ffi_type_pointer.elements[1]
$3 = (struct _ffi_type *) 0x0

It's like this on the way into ffi_call (can't step into it at the moment).
msg288298 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2017-02-21 13:27
The 24-byte struct gets passed on the stack, as it should be. In this case ffi_call doesn't abort() because examine_argument returns 0, which is due to the following code in classify_argument:

    if (words > 2)
    {
        /* When size > 16 bytes, if the first one isn't
           X86_64_SSE_CLASS or any other ones aren't
           X86_64_SSEUP_CLASS, everything should be passed in
           memory.  */
        if (classes[0] != X86_64_SSE_CLASS)
            return 0;

        for (i = 1; i < words; i++)
            if (classes[i] != X86_64_SSEUP_CLASS)
                return 0;
    }

It looks like X86_64_SSEUP_CLASS is never actually assigned by classify_argument(), in which case libffi never uses registers to pass structs that are larger than 16 bytes. 

Regarding floating-point values, we get a similar abort for passing a struct containing an array of two doubles because ctypes passes one ffi_type_pointer element instead of two ffi_type_double elements. 

Also, a struct with an array of one double (weird but should be supported) doesn't abort, but instead gets passed incorrectly like a pointer, i.e. as an integer in register rdi, instead of in the expected xmm0 register. The call thus uses whatever garbage value is currently in xmm0. You have to use a test lib to reproduce this. It's not apparent with a ctypes callback because ffi_closure_unix64 (unix64.S) and ffi_closure_unix64_inner (ffi64.c) use the same incorrect classification before calling ctypes closure_fcn and _CallPythonObject.
msg288341 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2017-02-22 07:36
Patch attached, including tests. If it looks halfway sensible, I can work up a PR.
msg288385 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2017-02-22 21:34
Notes on fix-22273-02.diff:

In the second pass over _fields_, you can (should) use dict->length and dict->proto for array types instead of the _length_ and _type_ attributes. 

When reassigning stgdict->ffi_type_pointer.elements, if use_broken_old_ctypes_semantics is false, then you also have to allocate space for and copy the elements from the base class if any (i.e. if basedict && basedict->length > 0).

Regarding structs with bitfields and unions, we could add an stgdict flag to prevent passing them as arguments in the Unix X86_64 ABI -- e.g.   add a flag named TYPEFLAG_NONARGTYPE (0x400). ConvParam (callproc.c) and converters_from_argtypes (_ctypes.c) would raise an ArgumentError or TypeError in this case. Subclasses of structs and unions would inherit this flag value in StructUnionType_new.

The first pass in PyCStructUnionType_update_stgdict can set arrays_seen and bitfields_seen. Also, per the above suggestion, isArgType can be added. Moreover, since we don't have to worry about bitfields if we forbid passing structs with bitfields in this ABI, then MAX_ELEMENTS can be reduced to 8. For example:

    #ifdef X86_64
    #define MAX_ELEMENTS 8
    isArgType = (!(stgdict->flags & TYPEFLAG_NONARGTYPE) &&
                 isStruct && !bitfields_seen);
    if (!isArgType) {
        stgdict->flags |= TYPEFLAG_NONARGTYPE;
    } else if (size <= 16 && arrays_seen) {
        ffi_type *actual_types[MAX_ELEMENTS + 1];
        int actual_type_index = 0;

        /* second pass over _fields_ */

    }

This is speculative based on how we address passing unions and structs with bitfields in the 64-bit Unix ABI. Raising a descriptive exception is at least an improvement over abruptly aborting the process.
msg288426 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2017-02-23 09:04
Thanks for the comments. Using your suggestions simplifies things quite a bit. Still finding my way around :-)

> Regarding structs with bitfields and unions, we could add an stgdict flag to prevent passing them as arguments in the Unix X86_64 ABI

Is this to deal with issues #16575, #16576 and #26628? Issue #26628 seems to be a duplicate of #16575. I can add the flag and set it here, but the checks should probably be in a separate patch.

> if we forbid passing structs with bitfields in this ABI, then MAX_ELEMENTS can be reduced to 8.

Not sure that's the case. For example, if we have to handle

    typedef struct {
        unsigned char data[12];
    } Test;

that would use up 12 slots for the unrolled array. Perhaps you mean 16 rather than 8?

Updated patch attached.
msg288437 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2017-02-23 10:06
> Perhaps you mean 16 rather than 8?

Sorry, that was a misfire. It should be 16.
msg288448 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2017-02-23 15:00
Just a thought - the TYPEFLAG_NONARGTYPE needs to be copied from the base class if set there, right?
msg288452 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2017-02-23 15:32
I had suggested inheriting the TYPEFLAG_NONARGTYPE flag in StructUnionType_new. It requires a minor change to get basedict unconditionally, and then assign 

    if (basedict)
        dict->flags |= basedict->flags & TYPEFLAG_NONARGTYPE;

We need more feedback on this suggested flag, especially to stay consistent with CFFI if possible. Do you know whether CFFI supports passing unions and structs with bitfields in its ABI mode for 64-bit Unix?
msg288461 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2017-02-23 16:52
> We need more feedback on this suggested flag, especially to stay consistent with CFFI if possible.

Undoubtedly, more feedback would be very helpful. I'm not sure using this flag impacts on consistency with CFFI particularly, since it's an internal implementation detail. Its main purpose would be for ctypes to raise exceptions rather than leading to crashes or undefined behaviour, as we have at the moment.

> Do you know whether CFFI supports passing unions and structs with bitfields in its ABI mode for 64-bit Unix?

I don't believe so, but I'm relatively new to this area. I'm not sure if things have changed recently, but an analogous CFFI issue was closed as WONTFIX in 2015, citing lack of support in libffi:

https://bitbucket.org/cffi/cffi/issues/150/structs-with-bit-fields-as-arguments

Also, the latest CFFI documentation, near the bottom of this section:

https://cffi.readthedocs.io/en/latest/using.html#function-calls

says:

"The limitations are that you cannot pass directly as argument or return type:

* a union (but a pointer to a union is fine);
* a struct which uses bitfields (but a pointer to such a struct is fine);"

The documentation applies these limitations regardless of any specific ABI (presumably to provide consistency).

So, I would guess that, as with ctypes, lack of libffi support is the main obstacle. I suppose one would have to seriously consider contributing there to make much headway here. In this still-open issue from 2013:

https://github.com/libffi/libffi/issues/33

Anthony Green of libffi said he'd welcome a patch, in response to a question by Eli Bendersky. Of course, it may be hard for individual contributors to support this for the range of architectures that libffi covers.
msg288471 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2017-02-23 19:26
> I'm not sure using this flag impacts on consistency with CFFI 

I meant consistency with respect to supported argument types. If someone contributed a workaround to CFFI, then I would rather port it, but it looks like they're also waiting for this to be addressed upstream. 

It occurs to me that in the 1st pass, it also needs to propagate the non-argument flag from any field that has it set. This should be done for all platforms, not just X86_64.
msg288482 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2017-02-23 20:16
> It occurs to me that in the 1st pass, it also needs to propagate the non-argument flag from any field that has it set.

So does that mean disallowing a structure which contains a union? What about if the final structure is large enough to require passing in memory rather than registers, so that libffi doesn't need to do any clever marshalling, even if some part of the structure wouldn't by itself be able to be passed as an argument in a call? Won't that end up being too restrictive?
msg288491 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2017-02-23 22:14
Perhaps it should instead use two specific flags, TYPEFLAG_HASBITFIELD and TYPEFLAG_HASUNION, which are propagated unconditionally from the base class and fields. As a base case, a union itself is flagged TYPEFLAG_HASUNION. Arrays are unrolled on X86_64 only if neither flag is present and the size is 16 bytes or less.

If ConvParam or converters_from_argtypes see either flag on X86_64 and the size is 16 bytes or less, then they raise an exception. As before, this rejects some call signatures that would actually succeed. We're not accounting for the case in which the limited number of registers forces an argument to be passed on the stack even though it's small enough to be passed in registers.
msg288493 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2017-02-23 22:46
> Perhaps it should instead use two specific flags, TYPEFLAG_HASBITFIELD and TYPEFLAG_HASUNION

This seems better at first sight. It's not making any suitability decisions (apart from doing the unrolling), and the meaning of these flags will be less volatile than TYPEFLAG_NONARGTYPE because that assessment depends on current limitations, and those limitations might change over time.

I'm going into a period of two weeks where I may not have much time to work on this due to other time commitments, so if you want to press on with it, go right ahead :-)
msg309113 - (view) Author: Ilya Kulakov (Ilya.Kulakov) * Date: 2017-12-28 00:24
Is there anything to be done for this patch to get merged?
msg309126 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2017-12-28 12:05
Yes, the patch needs improving as per the suggestion in msg288493 (not had the time since to do any work on it), followed by a review of the changes.
msg353135 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-09-25 03:38
New changeset 12f209eccb1587e8c98057d9c5f865c21f4a16c1 by Vinay Sajip in branch 'master':
bpo-22273: Update ctypes to correctly handle arrays in small structur… (GH-15839)
https://github.com/python/cpython/commit/12f209eccb1587e8c98057d9c5f865c21f4a16c1
msg353138 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-09-25 04:10
New changeset ce62dcc460cf88f663c34c4a0948c1ee1dc53f4d by Vinay Sajip (Miss Islington (bot)) in branch '3.8':
bpo-22273: Update ctypes to correctly handle arrays in small structur… (GH-15839) (GH-16370)
https://github.com/python/cpython/commit/ce62dcc460cf88f663c34c4a0948c1ee1dc53f4d
msg353139 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-09-25 04:10
New changeset 16c0f6df62a39f9f7712b1c0577de4eefcb4c1bf by Vinay Sajip (Miss Islington (bot)) in branch '3.7':
bpo-22273: Update ctypes to correctly handle arrays in small structur… (GH-15839) (GH-16369)
https://github.com/python/cpython/commit/16c0f6df62a39f9f7712b1c0577de4eefcb4c1bf
msg353150 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-09-25 06:58
New changeset 57dc7d5ae8ebfb6da1ea2b25e61260ecb9c79faf by Vinay Sajip in branch 'master':
bpo-22273: Disabled tests while investigating buildbot failures on ARM7L/PPC64. (GH-16377)
https://github.com/python/cpython/commit/57dc7d5ae8ebfb6da1ea2b25e61260ecb9c79faf
msg353176 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-09-25 11:21
Please check if bpo-38272 regression is caused by this issue.

"FAIL: test_array_in_struct (ctypes.test.test_structures.StructureTestCase)" on ARMv7.
msg353200 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-09-25 14:06
New changeset 417089e88bd4ea146b9497e06e8edeb58a59cd65 by Vinay Sajip in branch 'master':
bpo-22273: Re-enabled ctypes test on ARM machines. (GH-16388)
https://github.com/python/cpython/commit/417089e88bd4ea146b9497e06e8edeb58a59cd65
msg353225 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-09-25 19:53
> Please check if bpo-38272 regression is caused by this issue.

Yes, it is. Being worked on right now. Sorry for the noise.
msg353227 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-09-25 19:57
New changeset cc28ed2421bf3953efc0fbde387f28722f3801e2 by Vinay Sajip in branch 'master':
bpo-22273: Removed temporary test skipping on PPC platforms. (GH-16399)
https://github.com/python/cpython/commit/cc28ed2421bf3953efc0fbde387f28722f3801e2
msg353228 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-09-25 20:37
New changeset d015714f89af5c57b7071a74eeca618577b3dfe4 by Vinay Sajip in branch '3.7':
[3.7] bpo-22273: Changed conditions for ctypes array-in-struct handling. (GH-16381) (GH-16400)
https://github.com/python/cpython/commit/d015714f89af5c57b7071a74eeca618577b3dfe4
msg353238 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-09-25 21:41
New changeset b92b8c53f63d2e805288ac2c665bf5887d1b7ca6 by Vinay Sajip in branch '3.8':
[3.8] bpo-22273: Changed conditions for ctypes array-in-struct handling. (GH-16381) (GH-16401)
https://github.com/python/cpython/commit/b92b8c53f63d2e805288ac2c665bf5887d1b7ca6
msg353589 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-09-30 15:50
New changeset c9a413ede47171a224c72dd34122005170caaad4 by Vinay Sajip (Victor Stinner) in branch 'master':
bpo-38321: Fix PyCStructUnionType_update_stgdict() warning (GH-16492)
https://github.com/python/cpython/commit/c9a413ede47171a224c72dd34122005170caaad4
msg353679 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-10-01 11:52
New changeset bfe1f74e39d0049a829962050e86a6a2d2a2781e by Victor Stinner in branch '3.8':
[3.8] bpo-3832: Fix compiler warnings (GH-16518)
https://github.com/python/cpython/commit/bfe1f74e39d0049a829962050e86a6a2d2a2781e
History
Date User Action Args
2022-04-11 14:58:07adminsetgithub: 66469
2019-10-01 11:52:03vstinnersetmessages: + msg353679
2019-09-30 15:50:05vinay.sajipsetmessages: + msg353589
2019-09-30 14:17:32vstinnersetpull_requests: + pull_request16079
2019-09-26 06:55:39vinay.sajipsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2019-09-25 21:41:08vinay.sajipsetmessages: + msg353238
2019-09-25 21:19:39vinay.sajipsetpull_requests: + pull_request15983
2019-09-25 20:37:44vinay.sajipsetmessages: + msg353228
2019-09-25 20:16:27vinay.sajipsetpull_requests: + pull_request15982
2019-09-25 19:57:26vinay.sajipsetmessages: + msg353227
2019-09-25 19:53:48vinay.sajipsetmessages: + msg353225
2019-09-25 19:31:20vinay.sajipsetpull_requests: + pull_request15981
2019-09-25 14:06:05vinay.sajipsetmessages: + msg353200
2019-09-25 13:20:16vinay.sajipsetpull_requests: + pull_request15970
2019-09-25 11:21:13vstinnersetnosy: + vstinner
messages: + msg353176
2019-09-25 09:51:49vinay.sajipsetpull_requests: + pull_request15961
2019-09-25 06:58:35vinay.sajipsetmessages: + msg353150
2019-09-25 06:35:50vinay.sajipsetpull_requests: + pull_request15956
2019-09-25 04:10:46vinay.sajipsetmessages: + msg353139
2019-09-25 04:10:23vinay.sajipsetmessages: + msg353138
2019-09-25 03:40:15miss-islingtonsetpull_requests: + pull_request15950
2019-09-25 03:40:09miss-islingtonsetpull_requests: + pull_request15949
2019-09-25 03:38:48vinay.sajipsetmessages: + msg353135
2019-09-10 11:36:10vinay.sajipsetpull_requests: + pull_request15484
2019-05-13 02:19:38cheryl.sabellasetversions: + Python 3.8, - Python 2.7, Python 3.4, Python 3.5, Python 3.6
2017-12-28 12:05:05vinay.sajipsetmessages: + msg309126
2017-12-28 00:24:57Ilya.Kulakovsetmessages: + msg309113
2017-02-23 22:46:07vinay.sajipsetmessages: + msg288493
2017-02-23 22:14:52eryksunsetmessages: + msg288491
2017-02-23 20:16:01vinay.sajipsetmessages: + msg288482
2017-02-23 19:26:24eryksunsetmessages: + msg288471
2017-02-23 16:52:07vinay.sajipsetmessages: + msg288461
2017-02-23 15:32:34eryksunsetmessages: + msg288452
2017-02-23 15:00:47vinay.sajipsetmessages: + msg288448
2017-02-23 10:06:08eryksunsetmessages: + msg288437
2017-02-23 09:04:28vinay.sajipsetfiles: + fix-22273-03.diff

messages: + msg288426
2017-02-22 21:34:50eryksunsetmessages: + msg288385
2017-02-22 07:36:36vinay.sajipsetstage: needs patch -> patch review
2017-02-22 07:36:07vinay.sajipsetfiles: + fix-22273-02.diff

messages: + msg288341
2017-02-21 13:27:59eryksunsetmessages: + msg288298
2017-02-20 21:36:07vinay.sajipsetmessages: + msg288244
2017-02-20 21:08:37eryksunsetmessages: + msg288241
2017-02-20 20:48:21eryksunsetmessages: + msg288239
2017-02-20 20:34:17vinay.sajipsetmessages: + msg288237
2017-02-20 17:10:22vinay.sajipsetmessages: + msg288221
2017-02-19 20:29:42eryksunsetmessages: + msg288163
2017-02-19 15:35:22vinay.sajipsetmessages: + msg288143
2017-02-19 15:33:13vinay.sajipsetfiles: + fix-22273-01.diff
keywords: + patch
messages: + msg288142
2017-02-19 10:24:24vinay.sajipsetnosy: + vinay.sajip
2017-02-15 19:12:23eryksunsetnosy: + eryksun
messages: + msg287881
2017-02-15 18:33:01eryksunsetstage: needs patch
2017-02-15 18:32:36eryksunsetpriority: normal -> high
versions: + Python 3.5, Python 3.6, Python 3.7
2015-02-09 10:22:10alexei.romanovsetnosy: + alexei.romanov

versions: + Python 2.7
2015-02-09 09:17:39Ilya.Kulakovsetnosy: + Ilya.Kulakov
messages: + msg235598
2014-08-28 22:09:03ned.deilysetnosy: + amaury.forgeotdarc, belopolsky, meador.inge
2014-08-28 22:02:31weeblesetmessages: + msg226045
2014-08-25 17:33:22weeblecreate