[issue22273] abort when passing certain structs by value using ctypes

[STINNER Victor]

STINNER Victor  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +16079
pull_request: https://github.com/python/cpython/pull/16492

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Change by Vinay Sajip :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Change by Vinay Sajip :


--
pull_requests: +15983
pull_request: https://github.com/python/cpython/pull/16401

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Change by Vinay Sajip :


--
pull_requests: +15982
pull_request: https://github.com/python/cpython/pull/16400

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip  added the comment:

> Please check if bpo-38272 regression is caused by this issue.

Yes, it is. Being worked on right now. Sorry for the noise.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Change by Vinay Sajip :


--
pull_requests: +15981
pull_request: https://github.com/python/cpython/pull/16399

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Change by Vinay Sajip :


--
pull_requests: +15970
pull_request: https://github.com/python/cpython/pull/16388

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[STINNER Victor]

STINNER Victor  added the comment:

Please check if bpo-38272 regression is caused by this issue.

"FAIL: test_array_in_struct (ctypes.test.test_structures.StructureTestCase)" on 
ARMv7.

--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Change by Vinay Sajip :


--
pull_requests: +15961
pull_request: https://github.com/python/cpython/pull/16381

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Change by Vinay Sajip :


--
pull_requests: +15956
pull_request: https://github.com/python/cpython/pull/16377

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[miss-islington]

Change by miss-islington :


--
pull_requests: +15950
pull_request: https://github.com/python/cpython/pull/16370

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[miss-islington]

Change by miss-islington :


--
pull_requests: +15949
pull_request: https://github.com/python/cpython/pull/16369

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Change by Vinay Sajip :


--
pull_requests: +15484
pull_request: https://github.com/python/cpython/pull/15839

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Cheryl Sabella]

Change by Cheryl Sabella :


--
versions: +Python 3.8 -Python 2.7, Python 3.4, Python 3.5, Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Ilya Kulakov]

Ilya Kulakov  added the comment:

Is there anything to be done for this patch to get merged?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip added the comment:

> 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 :-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Eryk Sun]

Eryk Sun added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip added the comment:

> 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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Eryk Sun]

Eryk Sun added the comment:

> 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip added the comment:

> 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Eryk Sun]

Eryk Sun added the comment:

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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip added the comment:

Just a thought - the TYPEFLAG_NONARGTYPE needs to be copied from the base class 
if set there, right?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Eryk Sun]

Eryk Sun added the comment:

> Perhaps you mean 16 rather than 8?

Sorry, that was a misfire. It should be 16.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip added the comment:

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.

--
Added file: http://bugs.python.org/file46663/fix-22273-03.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Eryk Sun]

Eryk Sun added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Changes by Vinay Sajip :


--
stage: needs patch -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip added the comment:

Patch attached, including tests. If it looks halfway sensible, I can work up a 
PR.

--
Added file: http://bugs.python.org/file46660/fix-22273-02.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Eryk Sun]

Eryk Sun added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip added the comment:

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).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Eryk Sun]

Eryk Sun added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Eryk Sun]

Eryk Sun added the comment:

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=0x7fffd570, 
fn=fn@entry=0x77fee010,
rvalue=rvalue@entry=0x7fffd630, avalue=avalue@entry=0x7fffd610)
at ../src/x86/ffi64.c:424

[...snip...]

458   for (i = 0; i < avn; ++i)
(gdb) 
462   n = examine_argument (arg_types[i], classes, 0, , );
(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 = 0xa0005
(gdb) p/x $rsi
$11 = 0x706050403020100
(gdb) c
Continuing.

foo: 5
bar: 10
data: [0, 1, 2, 3, 4, 5, 6, 7]

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip added the comment:

Possibly also relevant: #16575 and

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Eryk Sun]

Eryk Sun added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Vinay Sajip added the comment:

> 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] = >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.

--
keywords: +patch
Added file: http://bugs.python.org/file46652/fix-22273-01.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Vinay Sajip]

Changes by Vinay Sajip :


--
nosy: +vinay.sajip

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Eryk Sun]

Eryk Sun added the comment:

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.

--
nosy: +eryksun

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Eryk Sun]

Changes by Eryk Sun :


--
stage:  -> needs patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Eryk Sun]

Changes by Eryk Sun :


--
priority: normal -> high
versions: +Python 3.5, Python 3.6, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Ilya Kulakov]

Ilya Kulakov added the comment:

The structure hack does not work on Windows 8, x64.

--
nosy: +Ilya.Kulakov

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22273
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Alexei Romanov]

Changes by Alexei Romanov drednout...@gmail.com:


--
nosy: +alexei.romanov
versions: +Python 2.7

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22273
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Weeble]

Weeble added the comment:

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.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22273
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Ned Deily]

Changes by Ned Deily n...@acm.org:


--
nosy: +amaury.forgeotdarc, belopolsky, meador.inge

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22273
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue22273] abort when passing certain structs by value using ctypes

[Weeble]

New submission from Weeble:

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  0x7782cf79 in __GI_raise (sig=sig@entry=6) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x77830388 in __GI_abort () at abort.c:89
#2  0x767134f5 in ffi_call (cif=0x7fffd7b0, fn=0x7650c625 
sum(ArrayStruct), rvalue=0x7fffd6f0, avalue=0x7fffd6d0) at 
../src/x86/ffi64.c:516
#3  0x7691fee3 in _ctypes_callproc () from 
/usr/lib/python3.4/lib-dynload/_ctypes.cpython-34m-x86_64-linux-gnu.so
#4  0x76920578 in ?? () from 
/usr/lib/python3.4/lib-dynload/_ctypes.cpython-34m-x86_64-linux-gnu.so
#5  0x0043810a in PyObject_Call ()
#6  0x00579f45 in PyEval_EvalFrameEx ()
#7  0x0057d3d3 in PyEval_EvalCodeEx ()
#8  0x0057bfaa in PyEval_EvalFrameEx ()
#9  0x0057d3d3 in PyEval_EvalCodeEx ()
#10 0x0060ba83 in PyRun_FileExFlags ()
#11 0x0060bc85 in PyRun_SimpleFileExFlags ()
#12 0x0060d3ac in Py_Main ()
#13 0x0041ec0d in main ()
(gdb) frame 2
#2  0x767134f5 in ffi_call (cif=0x7fffd7b0, fn=0x7650c625 
sum(ArrayStruct), rvalue=0x7fffd6f0, avalue=0x7fffd6d0) at 
../src/x86/ffi64.c:516
516   abort();
(gdb) info args
cif = 0x7fffd7b0
fn = 0x7650c625 sum(ArrayStruct)
rvalue = 0x7fffd6f0
avalue = 0x7fffd6d0
(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 = 0x7fffd4f0 
argp = 0x7fffd5a0 \001
arg_types = 0x7fffd6b0
gprcount = 1
ssecount = optimized out
ngpr = 1
nsse = 0
i = optimized out
avn = optimized out
ret_in_memory = optimized out
reg_args = 0x7fffd4f0
(gdb) print *cif
$2 = {abi = FFI_UNIX64, nargs = 1, arg_types = 0x7fffd6b0, rtype = 
0x76b5e228, 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

--
components: ctypes
messages: 225881
nosy: weeble
priority: normal
severity: normal
status: open
title: abort when passing certain structs by value using ctypes
type: crash
versions: Python 3.4

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22273
___
___
Python-bugs-list mailing list
Unsubscribe: