Re: [Python-Dev] [Python-checkins] commit of r41906 - python/branches/ssize_t/Objects/obmalloc.c
Author: martin.v.loewis Date: Tue Jan 3 14:16:53 2006 New Revision: 41906
Modified: python/branches/ssize_t/Objects/obmalloc.c Log: Disable 32-bit size limitation for 64-bit mode.
Modified: python/branches/ssize_t/Objects/obmalloc.c ============================================================================== --- python/branches/ssize_t/Objects/obmalloc.c (original) +++ python/branches/ssize_t/Objects/obmalloc.c Tue Jan 3 14:16:53 2006 @@ -1005,6 +1005,8 @@
bumpserialno(); total = nbytes + 16; +#if SIZEOF_SIZE_T < 8 + /* XXX do this check only on 32-bit machines */ if (total < nbytes || (total >> 31) > 1) { /* overflow, or we can't represent it in 4 bytes */ /* Obscure: can't do (total >> 32) != 0 instead, because @@ -1013,6 +1015,7 @@ size_t is an unsigned type. */ return NULL; } +#endif
This checkin should be reverted for now. It's in _PyObject_DebugMalloc, and at present the layout of the extra debug info in a PYMALLOC_DEBUG build is hard-coded to use 4-byte fields, no matter what sizeof(size_t) may be. One of the extra fields recorded in a PYMALLOC_DEBUG build is the number of bytes requested, and at present it's simply not capable of recording a value that doesn't fit in 4 bytes. Even after (if ever ;-)) this is changed to support recording 8-byte values on a box where sizeof(size_t) == 8, the "total < nbytes" part of the test would still be appropriate: PyObject_DebugMalloc requests more memory (`total`) than the user asked for (`nbytes`), and the computation of `total` may have overflowed. That's what "total < nbytes" is checking, and that's the right way to spell the overflow check on any box.
Tim Peters wrote:
Modified: python/branches/ssize_t/Objects/obmalloc.c [...] This checkin should be reverted for now.
Not sure whether you've noticed this is "just" on the ssize_t branch. Without this patch, it is not possible to allocate 4GiB or more for a string object in debug mode, which kind of defeated my attempts to test that. I certainly plan to remove all XXX marks I have introduced in that branch before suggesting to integrate it back into the trunk. So "for now", I would prefer to keep it, and only revert it if I have a complete fix.
It's in _PyObject_DebugMalloc, and at present the layout of the extra debug info in a PYMALLOC_DEBUG build is hard-coded to use 4-byte fields, no matter what sizeof(size_t) may be. One of the extra fields recorded in a PYMALLOC_DEBUG build is the number of bytes requested, and at present it's simply not capable of recording a value that doesn't fit in 4 bytes.
Well, AFAICT, it "works" even if it records only records the lower 4 bytes of the requested size. Upon freeing, it just won't put enough DEADBYTEs in, which I cannot see having further unfortunate consequences (except that it won't diagnose errors as good anymore as it could).
Even after (if ever ;-)) this is changed to support recording 8-byte values on a box where sizeof(size_t) == 8, the "total < nbytes" part of the test would still be appropriate: PyObject_DebugMalloc requests more memory (`total`) than the user asked for (`nbytes`), and the computation of `total` may have overflowed. That's what "total < nbytes" is checking, and that's the right way to spell the overflow check on any box.
Certainly; I did not mean to completely disable this test. Regards, Martin
[Tim]
Modified: python/branches/ssize_t/Objects/obmalloc.c [...] This checkin should be reverted for now.
[Martin]
Not sure whether you've noticed this is "just" on the ssize_t branch.
Right, I noticed.
Without this patch, it is not possible to allocate 4GiB or more for a string object in debug mode, which kind of defeated my attempts to test that.
I certainly plan to remove all XXX marks I have introduced in that branch before suggesting to integrate it back into the trunk.
So "for now", I would prefer to keep it, and only revert it if I have a complete fix.
If it's expedient, that's fine for now. I'd like to help fix it "for real", but time is a problem for now. In a week or two I may have more to give to this. Would probably also like to change the dict object's ma_{fill,used,mask} members to Py_ssize_t (while other people don't seem to do this much, I routinely write first-stab programs where a single dict blows more than 2GB of VM ;-)).
...
Tim Peters wrote:
If it's expedient, that's fine for now. I'd like to help fix it "for real", but time is a problem for now. In a week or two I may have more to give to this. Would probably also like to change the dict object's ma_{fill,used,mask} members to Py_ssize_t (while other people don't seem to do this much, I routinely write first-stab programs where a single dict blows more than 2GB of VM ;-)).
Yes, I just noticed I mostly ignored dicts so far; that should also be part of the final patch. Regards, Martin
participants (2)
-
"Martin v. Löwis" -
Tim Peters