msg330597 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-11-28 13:17 |
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. |
|
|
msg330598 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-11-28 13:30 |
My commit df108dc6610e41c54ed064a854e3903c143f0d77 already added assert(PyTuple_Check(op)) to these two macros. |
|
|
msg330600 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-11-28 14:19 |
New changeset 1cdfcfc9843d35ab2cb87387d3a79b2c8a585a38 by Victor Stinner in branch 'master': bpo-35337: Fix gettmarg(): use PyStructSequence_GET_ITEM() (GH-10765) https://github.com/python/cpython/commit/1cdfcfc9843d35ab2cb87387d3a79b2c8a585a38 |
|
|
msg330638 - (view) |
Author: Zackery Spytz (ZackerySpytz) *  |
Date: 2018-11-28 23:42 |
See also #14614. |
|
|
msg330675 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2018-11-29 10:24 |
This is compatibility breaking change. Currently you can get the address of the array of tuple items by using &PyTuple_GET_ITEM(obj, 0). |
|
|
msg330676 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-11-29 10:57 |
> This is compatibility breaking change. Right. The question is if you are ok with it :-) > Currently you can get the address of the array of tuple items by using &PyTuple_GET_ITEM(obj, 0). 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. |
|
|
msg331408 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2018-12-09 07:07 |
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. |
|
|
msg331410 - (view) |
Author: Stefan Behnel (scoder) *  |
Date: 2018-12-09 07:31 |
If this is really just about debugging, then I would suggest to not break existing code at all. |
|
|
msg331425 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2018-12-09 12:33 |
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(). |
|
|
msg331429 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2018-12-09 12:43 |
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. |
|
|
msg331431 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2018-12-09 13:02 |
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. |
|
|
msg331986 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-12-17 11:22 |
Ok, I abandon my change. |
|
|