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
Check index in PyTuple_GET_ITEM/PyTuple_SET_ITEM in debug mode #79518
Comments
I propose to add assertions to PyTuple_GET_ITEM/PyTuple_SET_ITEM in debug mode to check the index when Python is compiled in debug mode (./configure --with-pydebug). This change is backward incompatible when PyTuple_GET_ITEM/PyTuple_SET_ITEM is used on a structseq with unnamed fields. Well, that's a bug in the user code, PyStructSequence_GET_ITEM/PyStructSequence_SET_ITEM should be used instead. |
My commit df108dc already added assert(PyTuple_Check(op)) to these two macros. |
See also bpo-14614. |
This is compatibility breaking change. Currently you can get the address of the array of tuple items by using &PyTuple_GET_ITEM(obj, 0). |
Right. The question is if you are ok with it :-)
I don't get your point. I know that you access directly obj->ob_item to use directly the C array rather than PyTuple_GET_ITEM/PyTuple_SET_ITEM, but the issue is about making PyTuple_GET_ITEM/PyTuple_SET_ITEM stricter in debug mode. |
I think we can break this only after adding public API for accessing internal storage of a tuple: PyTuple_ITEMS(). And the same for lists. |
If this is really just about debugging, then I would suggest to not break existing code at all. |
I'm using &PyTuple_GET_ITEM(args, 0), so Serhiy's concern is not theoretical. I think if people want the safe version they should use PyTuple_GetItem(). |
Since this feature mainly helps when running a test suite using a debug build: The same can be achieved by running the test suite under Valgrind, which catches invalid accesses and a lot more. So I'd prefer to keep the macro in its current form. |
If the feature is for the Python test suite itself, one solution would be to add -DPY_BOUNDS_CHECKS and use that on the buildbots. I still think making all of the test suite Valgrind-ready (most of it is, except test_multiprocessing and a few others) would catch more. |
Ok, I abandon my change. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: