gh-81381: Reduce allocated size of PyType_GenericAlloc if possible

[corona10]

• Issue: PyType_GenericAlloc might over-allocate memory #81381

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @corona10 for commit 3946154 🤖

If you want to schedule another build, you need to add the :hammer: test-with-refleak-buildbots label again.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @corona10 for commit a9952ac 🤖

If you want to schedule another build, you need to add the :hammer: test-with-refleak-buildbots label again.

[corona10]

READ of size 4 at 0x6060002495f8 thread T0
    #0 0x55742c0fec6f in medium_value Objects/longobject.c:33
    #1 0x55742c0fec6f in _PyLong_Copy Objects/longobject.c:184
    #2 0x55742c0fec6f in long_long Objects/longobject.c:5345
    #3 0x55742c02c8a9 in PyNumber_Long Objects/abstract.c:1523
    #4 0x55742c119124 in long_new Objects/clinic/longobject.c.h:65
    #5 0x55742c1af0e9 in type_call Objects/typeobject.c:1264
    #6 0x55742c07574f in _PyObject_MakeTpCall Objects/call.c:216
    #7 0x557[42](https://github.com/python/cpython/actions/runs/3867841805/jobs/6592895892#step:13:43)bf31c8b in _PyEval_EvalFrameDefault Python/generated_cases.c.h:2967
    #8 0x55742c078c5e in PyObject_Call (/home/runner/work/cpython/cpython/python+0x3f1c5e)
    #9 0x55742bf[44](https://github.com/python/cpython/actions/runs/3867841805/jobs/6592895892#step:13:45)66c in do_call_core Python/ceval.c:2990
    #10 0x55742bf4466c in _PyEval_EvalFrameDefault Python/generated_cases.c.h:3499
    #11 0x55742c080848 in _PyObject_VectorcallTstate Include/internal/pycore_call.h:92
    #12 0x55742c080848 in method_vectorcall Objects/classobject.c:89
    #13 0x55742c078c5e in PyObject_Call (/home/runner/work/cpython/cpython/python+0x3f1c5e)
    #14 0x55742bf4[46](https://github.com/python/cpython/actions/runs/3867841805/jobs/6592895892#step:13:47)6c in do_call_core Python/ceval.c:2990

[nascheme]

What's the conclusion here? Based on the comment above, medium_value is reading beyond the allocated memory? AFAICT, gh-81381 is still unfixed. I'm thinking now that looking at existing type flags to determine if the +1 is needed might not work. There are likely 3rd party extensions that expect the extra space and removing the +1 will result in out-of-bounds memory access. Maybe we need a new type flag?

[corona10]

@nascheme

I may need more investigation but...

I'm thinking now that looking at existing type flags to determine if the +1 is needed might not work. There are likely 3rd party extensions that expect the extra space and removing the +1 will result in out-of-bounds memory access.

I think so too, we can not expect which type of flag will be safe.

Maybe we need a new type flag?

It's up to the situation that objects will be created a lot, but I am not sure it's worth to adding the flag to reduce it.

[mdickinson]

Based on the comment above, medium_value is reading beyond the allocated memory?

I'd like to understand this better. For things of exact type int, we always allocate at least one digit, even when ob_size = 0. The code is here:

cpython/Objects/longobject.c

Line 157 in ef633e5

Py_ssize_t ndigits = size ? size : 1;

For subclasses of int, we go through long_subtype_new and there's no correction for the zero case. That may be a bug, that we only get away with because of the +1 in PyType_GenericAlloc.

@corona10 Could you give some more context for the trace you show?

[mdickinson]

READ of size 4 at 0x6060002495f8 thread T0
    #0 0x55742c0fec6f in medium_value Objects/longobject.c:33
    #1 0x55742c0fec6f in _PyLong_Copy Objects/longobject.c:184
    #2 0x55742c0fec6f in long_long Objects/longobject.c:5345
    #3 0x55742c02c8a9 in PyNumber_Long Objects/abstract.c:1523
    #4 0x55742c119124 in long_new Objects/clinic/longobject.c.h:65
    #5 0x55742c1af0e9 in type_call Objects/typeobject.c:1264
    [...]

So if I'm reading this right, this looks like what would happen if one did int(myint), where myint is an instance of a subclass of int, with value 0 (e.g., class MyInt(int): pass, then myint=MyInt()). Looking at long_subtype_new, that zero is allocated with zero digits, but then converting to an int invokes medium_value, which checks ob_digit[0].

But I'm not really sure what I'm looking at above. @corona10: does this sound plausible? If so, I think there is indeed a bug in long_subtype_new, and it looks like an easily-fixed one.

[mdickinson]

I've opened an issue and a PR (#101038). If #101038 goes in, I think that fixes the issue that @corona10 was reporting in #100855 (comment).

@nascheme

There are likely 3rd party extensions that expect the extra space

Agreed; I think that means that #101038 doesn't unblock this PR, unfortunately.

Could there be other options, like adding a (private, for now) variant of PyType_GenericAlloc that doesn't have the +1 and using it for those types where we can establish that it's safe to do so?

For the root problem, the 3-element namedtuple case is about the worst vaguely plausible case that I can think of: for that, we end up allocating 80 bytes instead of 64 for the base object.