classification
Title: test_structmembers fails on s390x (bigendian 64-bit): int/Py_ssize_t issue
Type: behavior Stage: resolved
Components: Tests Versions: Python 2.7
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: Nosy List: BreamoreBoy, dmalcolm, mark.dickinson, pitrou, serhiy.storchaka
Priority: normal Keywords: easy, patch

Created on 2010-09-27 15:11 by dmalcolm, last changed 2018-12-12 09:39 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
python-test_structmembers.patch dmalcolm, 2010-09-27 15:11
fix-test_structmember-on-64bit-bigendian.patch dmalcolm, 2012-04-12 21:53
Messages (6)
msg117448 - (view) Author: Dave Malcolm (dmalcolm) (Python committer) Date: 2010-09-27 15:11
test test_structmembers crashed -- <type 'exceptions.ValueError'>:
string too long
Traceback (most recent call last):
  File "/builddir/build/BUILD/Python-2.7/Lib/test/regrtest.py", line 863, in runtest_inner
    the_package = __import__(abstest, globals(), locals(), [])
  File "/builddir/build/BUILD/Python-2.7/Lib/test/test_structmembers.py", line 12, in <module>
    9.99999, 10.1010101010, "hi")
ValueError: string too long

_testcapimodule.c: test_structmembers_new's fmt has:
  "s#"
and these args:
  &s, &string_len

for grabbing this data:
    const char *s = NULL;
    Py_ssize_t string_len = 0;

However, the module doesn't define PY_SSIZE_T_CLEAN, which leads to &string_len being treated as an (int*) rather than a (Py_ssize_t*) and thus written to with just the first 32-bits of the size, rather than the full size.

The PyArgs_ call without PY_SSIZE_T_CLEAN writes the size of the string (2) through (int*)&string_len, writing 0x00000002 to the first 4 bytes, setting the 64-bit "string_len" value to 0x0000000200000000 i.e. 2^34, wildly too large for the iirc 5 byte buffer.

Confirmed in gdb:
  (gdb) p string_len
  $2 = 8589934592

  (gdb) p /x string_len
  $3 = 0x200000000

Am attaching a patch (against 2.7 maintenance branch) which defines PY_SSIZE_T_CLEAN; doing so requires updating another int to be a Py_ssize_t in that module, within test_u_code.


http://docs.python.org/c-api/arg.html lists "u# (Unicode) [Py_UNICODE *, int]" and has no reference to the effect of PY_SSIZE_T_CLEAN on the "u#" format specifier.  My reading of Python/getargs.c is that this macro does affect "u#" in a manner analogous to "s#"; does this documentation need clarifying?
msg117951 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-10-04 14:37
Is this 2.7-specific? Otherwise, it would be better to provide a patch for 3.2 first, and then svnmerge to other branches.

> My reading of Python/getargs.c is that this macro does affect "u#" in a > manner analogous to "s#"; does this documentation need clarifying?

Probably. Same for "es#", "et#" and "t#" perhaps?
msg128576 - (view) Author: Dave Malcolm (dmalcolm) (Python committer) Date: 2011-02-14 21:23
FWIW, this regressed in 2.6.6 relative to 2.6.5, due to r79646 adding the T_STRING_INPLACE test code to the 2.6 branch.

Note to self: https://bugzilla.redhat.com/show_bug.cgi?id=677392
msg158177 - (view) Author: Dave Malcolm (dmalcolm) (Python committer) Date: 2012-04-12 21:53
The originally attached patch is no good for the the 2.* branch, as it appears that _testcapimodule.c will not become "ssize_t" safe in Python 2.*; see e.g.:
  http://hg.python.org/cpython/rev/3ecddf168f1f

Am attaching a revised patch that I'm applying downstream in Fedora's builds of 2.7.3
msg221826 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-06-29 00:39
I believe that the change in the later patch has already been applied to the 2.7 branch.  If I'm correct can we close this please.
msg331685 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-12-12 09:39
This was fixed in issue17928.
History
Date User Action Args
2018-12-12 09:39:24serhiy.storchakasetstatus: open -> closed

nosy: + serhiy.storchaka
messages: + msg331685

resolution: out of date
stage: patch review -> resolved
2014-06-29 00:39:55BreamoreBoysetnosy: + BreamoreBoy
messages: + msg221826
2012-04-12 21:53:54dmalcolmsetfiles: + fix-test_structmember-on-64bit-bigendian.patch

messages: + msg158177
2011-02-14 21:23:05dmalcolmsetnosy: mark.dickinson, pitrou, dmalcolm
messages: + msg128576
2010-10-04 14:37:38pitrousetnosy: + pitrou
messages: + msg117951
2010-09-27 15:20:14mark.dickinsonsetnosy: + mark.dickinson
2010-09-27 15:11:33dmalcolmcreate