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

Add _PySys_GetSizeOf() #66389

Closed
serhiy-storchaka opened this issue Aug 14, 2014 · 13 comments
Closed

Add _PySys_GetSizeOf() #66389

serhiy-storchaka opened this issue Aug 14, 2014 · 13 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 22193
Nosy @loewis, @tiran, @serhiy-storchaka
Files
  • _PySys_GetSizeOf.patch
  • _PySys_GetSizeOf_overflow.patch
  • _PySys_GetSizeOf_overflow_2.patch
  • 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 = 'https://github.com/serhiy-storchaka'
    closed_at = <Date 2014-11-15.12:13:53.733>
    created_at = <Date 2014-08-14.10:24:53.636>
    labels = ['interpreter-core', 'type-feature']
    title = 'Add _PySys_GetSizeOf()'
    updated_at = <Date 2014-11-15.12:13:53.732>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2014-11-15.12:13:53.732>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2014-11-15.12:13:53.733>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2014-08-14.10:24:53.636>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = ['36368', '36376', '36391']
    hgrepos = []
    issue_num = 22193
    keywords = ['patch']
    message_count = 13.0
    messages = ['225294', '225301', '225310', '225312', '225319', '225341', '225346', '225352', '225422', '225432', '225442', '231205', '231207']
    nosy_count = 5.0
    nosy_names = ['loewis', 'christian.heimes', 'Arfrever', 'python-dev', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue22193'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

    @serhiy-storchaka
    Copy link
    Member Author

    Proposed patch adds private function _PySys_GetSizeOf(). It is needed for correct implementing of __sizeof__() methods: bpo-15490, bpo-15513, bpo-15381 (BytesIO.__sizeof__ is broken in 3.5). See discussion about it in bpo-15490. I extracted this patch in separate issue for its simplicity and because it is needed in other issues.

    @serhiy-storchaka serhiy-storchaka self-assigned this Aug 14, 2014
    @serhiy-storchaka serhiy-storchaka added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Aug 14, 2014
    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Aug 14, 2014

    LGTM

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 14, 2014

    New changeset b5114747d3ed by Serhiy Storchaka in branch '2.7':
    Issue bpo-22193: Added private function _PySys_GetSizeOf() needed to implement
    http://hg.python.org/cpython/rev/b5114747d3ed

    New changeset 46504edf7594 by Serhiy Storchaka in branch '3.4':
    Issue bpo-22193: Added private function _PySys_GetSizeOf() needed to implement
    http://hg.python.org/cpython/rev/46504edf7594

    New changeset e23999892e58 by Serhiy Storchaka in branch 'default':
    Issue bpo-22193: Added private function _PySys_GetSizeOf() needed to implement
    http://hg.python.org/cpython/rev/e23999892e58

    @serhiy-storchaka
    Copy link
    Member Author

    Thanks Martin.

    @tiran
    Copy link
    Member

    tiran commented Aug 14, 2014

    The code is missing an overflow check. There should be some verification that __sizeof__ returns a value smaller than SIZE_T_MAX - 24. Also 24 for GC overhead looks strange. IMHO it should be sizeof(PyGC_Head) which can be smaller than 24 bytes.

    Python 3.5.0a0 (default:601045ceff94, Aug 14 2014, 22:59:49) 
    [GCC 4.8.2] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> class Overflow:
    ...     def __sizeof__(self):
    ...         return (1<<64)-1
    ... 
    >>> sys.getsizeof(Overflow())
    23

    @tiran tiran reopened this Aug 14, 2014
    @serhiy-storchaka
    Copy link
    Member Author

    This was discussed in bpo-15490. Such implementation is just incorrect, because we cannot allocate such much memory.

    @serhiy-storchaka
    Copy link
    Member Author

    Here is a patch which adds overflow check. The question is: should sys.getsizeof with the default argument catch OverflowError in additional to TypeError and replace it by default value?

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Aug 15, 2014

    Christian: I don't see why 24 bytes overhead sounds strange. GC_Head is

    typedef union _gc_head {
        struct {
            union _gc_head *gc_next;
            union _gc_head *gc_prev;
            Py_ssize_t gc_refs;
        } gc;
        double dummy;  /* force worst-case alignment */
    } PyGC_Head;

    which happens to be 3*8=24 bytes on a 64-bit system.

    @Arfrever
    Copy link
    Mannequin

    Arfrever mannequin commented Aug 17, 2014

    Revision b5114747d3ed added useless '#ifndef Py_LIMITED_API' check in 2.7 branch.
    Support for Py_LIMITED_API was introduced in Python 3.2.

    @serhiy-storchaka
    Copy link
    Member Author

    Fixed in changeset 2e417d9a2b1c. Thank you Arfrever.

    @serhiy-storchaka
    Copy link
    Member Author

    Here is alternative patch. May be it looks more clear for you.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 15, 2014

    New changeset 3537994fa43b by Serhiy Storchaka in branch '2.7':
    Issue bpo-22193: Fixed integer overflow error in sys.getsizeof().
    https://hg.python.org/cpython/rev/3537994fa43b

    New changeset df5c6b05238e by Serhiy Storchaka in branch '3.4':
    Issue bpo-22193: Fixed integer overflow error in sys.getsizeof().
    https://hg.python.org/cpython/rev/df5c6b05238e

    New changeset b7651f9be4a1 by Serhiy Storchaka in branch 'default':
    Issue bpo-22193: Fixed integer overflow error in sys.getsizeof().
    https://hg.python.org/cpython/rev/b7651f9be4a1

    @serhiy-storchaka
    Copy link
    Member Author

    Also fixed an error in _PySys_GetSizeOf declaration (thanks yomgui1). Thanks all for reviews and found bugs.

    @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-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants