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

Segmentation fault in bytearray tests #49185

Closed
pitrou opened this issue Jan 13, 2009 · 7 comments
Closed

Segmentation fault in bytearray tests #49185

pitrou opened this issue Jan 13, 2009 · 7 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@pitrou
Copy link
Member

pitrou commented Jan 13, 2009

BPO 4935
Nosy @mdickinson, @pitrou
Files
  • issue4935.patch
  • issue4935-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 = None
    closed_at = <Date 2009-01-13.23:26:46.053>
    created_at = <Date 2009-01-13.18:50:39.010>
    labels = ['interpreter-core', 'type-crash']
    title = 'Segmentation fault in bytearray tests'
    updated_at = <Date 2009-01-13.23:26:46.052>
    user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2009-01-13.23:26:46.052>
    actor = 'pitrou'
    assignee = 'none'
    closed = True
    closed_date = <Date 2009-01-13.23:26:46.053>
    closer = 'pitrou'
    components = ['Interpreter Core']
    creation = <Date 2009-01-13.18:50:39.010>
    creator = 'pitrou'
    dependencies = []
    files = ['12724', '12726']
    hgrepos = []
    issue_num = 4935
    keywords = ['patch']
    message_count = 7.0
    messages = ['79766', '79772', '79778', '79779', '79781', '79785', '79795']
    nosy_count = 2.0
    nosy_names = ['mark.dickinson', 'pitrou']
    pr_nums = []
    priority = 'critical'
    resolution = 'fixed'
    stage = 'patch review'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue4935'
    versions = []

    @pitrou
    Copy link
    Member Author

    pitrou commented Jan 13, 2009

    This happens on a 32-bit build on a 64-bit system, which happens to have
    some interesting properties: for example, malloc() will happily allocate
    memory larger than Py_SSIZE_T_MAX.

    The crash is exactly triggered by the following snippet:

            if sys.maxsize < (1 << 32) and struct.calcsize('P') == 4:
                self.assertRaises(OverflowError,
                                  self.marshal(b'\ta\n\tb').expandtabs,
    sys.maxsize)

    @pitrou pitrou added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump labels Jan 13, 2009
    @pitrou
    Copy link
    Member Author

    pitrou commented Jan 13, 2009

    Here is a patch. The idea is to use unsigned arithmetic instead of
    signed, and to check for overflows by comparing with PY_SSIZE_T_MAX.

    (the exact reason of the crash is unknown, it looks like either a
    compiler bug or a mis-optimization as (i < 0) returns 0 while i is
    -0x7FFFFFFF)

    @mdickinson
    Copy link
    Member

    What's the purpose of old_i? It looks like it's never used for anything.

    Other than than, the patch looks good to me.

    I'd guess that the "if (i < 0)" was simply optimized away. This isn't
    necessarily a compiler bug: if I understand correctly, a strict reading of
    the C standards says it's legitimate for a compiler to assume that code is
    written in such a way that signed-arithmetic overflow never happens, and
    gcc (for one) is known to take advantage of this.

    Also, it would be nice to cleanup the whitespace in this function while
    you're fixing it; at the moment it's showing me a mixture of tabs and
    spaces.

    @mdickinson
    Copy link
    Member

    Actually, what's the purpose of old_j? It looks like that's not needed
    any more, either!

    @pitrou
    Copy link
    Member Author

    pitrou commented Jan 13, 2009

    You are right, old_i and old_j are unused, they were part of another
    approach I tried and which failed.
    Attaching new patch with these 2 variables removed, and the function
    cleanly reindented (of course the patch is messier because of this).

    @mdickinson
    Copy link
    Member

    Looks fine; I think this should be applied.

    It seems as though your reindentation left trailing whitespace on
    the blank lines in the function: the svn commit hook might
    complain about this...

    @pitrou
    Copy link
    Member Author

    pitrou commented Jan 13, 2009

    Committed in trunk, py3k, 2.6 and 3.0. Thanks!

    @pitrou pitrou closed this as completed Jan 13, 2009
    @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-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants