Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PowerLinux getargs.c FETCH_SIZE endianness bug #62128

Closed
DavidEdelsohn mannequin opened this issue May 7, 2013 · 6 comments
Closed

PowerLinux getargs.c FETCH_SIZE endianness bug #62128

DavidEdelsohn mannequin opened this issue May 7, 2013 · 6 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@DavidEdelsohn
Copy link
Mannequin

DavidEdelsohn mannequin commented May 7, 2013

BPO 17928
Nosy @pitrou, @matejcik

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2013-05-08.00:32:38.629>
created_at = <Date 2013-05-07.18:29:22.059>
labels = ['interpreter-core', 'type-bug']
title = 'PowerLinux getargs.c FETCH_SIZE endianness bug'
updated_at = <Date 2013-05-16.18:05:04.236>
user = 'https://bugs.python.org/DavidEdelsohn'

bugs.python.org fields:

activity = <Date 2013-05-16.18:05:04.236>
actor = 'matejcik'
assignee = 'none'
closed = True
closed_date = <Date 2013-05-08.00:32:38.629>
closer = 'pitrou'
components = ['Interpreter Core']
creation = <Date 2013-05-07.18:29:22.059>
creator = 'David.Edelsohn'
dependencies = []
files = []
hgrepos = []
issue_num = 17928
keywords = []
message_count = 6.0
messages = ['188677', '188695', '188696', '188698', '189392', '189395']
nosy_count = 4.0
nosy_names = ['pitrou', 'matejcik', 'python-dev', 'David.Edelsohn']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue17928'
versions = ['Python 2.7']

@DavidEdelsohn
Copy link
Mannequin Author

DavidEdelsohn mannequin commented May 7, 2013

Another endianness bug that causes a failure in test_structmembers.py.

_testcapi reports "string too long" because getargs.c:PyArg_ParseTupleAndKeywords() incorrectly returns a huge value for string_len.

The problem is FETCH_ARGS is passing the wrong type to va_arg. It grabs an "int" for the size arg, but that is the not the argument type on 64 bit platforms. This happens to work for little endian because the low part of the 64 bit argument overlaps correctly. Big endian is not as fortuitous.

If I change "int" to "long", the testcase succeeds.

diff -r a285ce18bd55 Python/getargs.c
--- a/Python/getargs.c	Mon May 06 18:21:10 2013 -0700
+++ b/Python/getargs.c	Tue May 07 11:26:21 2013 -0700
@@ -582,9 +582,9 @@
               char *msgbuf, size_t bufsize, PyObject **freelist)
 {
     /* For # codes */
-#define FETCH_SIZE      int *q=NULL;Py_ssize_t *q2=NULL;\
+#define FETCH_SIZE      long *q=NULL;Py_ssize_t *q2=NULL;\
     if (flags & FLAG_SIZE_T) q2=va_arg(*p_va, Py_ssize_t*); \
-    else q=va_arg(*p_va, int*);
+    else q=va_arg(*p_va, long*);
 #define STORE_SIZE(s)   \
     if (flags & FLAG_SIZE_T) \
         *q2=s; \

I am not certain exactly what type it should be, but it definitely needs to be a matching 64 bit type of 64 bit platforms.

I believe that this bug exists in all versions.

@DavidEdelsohn DavidEdelsohn mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels May 7, 2013
@pitrou
Copy link
Member

pitrou commented May 7, 2013

Is it 2.7-only?

@python-dev
Copy link
Mannequin

python-dev mannequin commented May 8, 2013

New changeset a199ec80c679 by Antoine Pitrou in branch '2.7':
Issue bpo-17928: Fix test_structmembers on 64-bit big-endian machines.
http://hg.python.org/cpython/rev/a199ec80c679

@pitrou
Copy link
Member

pitrou commented May 8, 2013

Fixed. _testcapi was actually the culprit.

@pitrou pitrou closed this as completed May 8, 2013
@matejcik
Copy link
Mannequin

matejcik mannequin commented May 16, 2013

The fix causes regression on my 64bit little-endian machine. It seems that while parsing the arguments, the length value overwrites part of the string pointer.

@matejcik
Copy link
Mannequin

matejcik mannequin commented May 16, 2013

hmm, but it's caused by a private patch claiming that _testcapimodule.c is PY_SSIZE_T_CLEAN. sorry for the noise.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

1 participant