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

Crash when setting attribute with string subclass as the name (--with-pydebug) #91059

Closed
ronaldoussoren opened this issue Mar 2, 2022 · 7 comments
Labels
3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@ronaldoussoren
Copy link
Contributor

BPO 46903
Nosy @ronaldoussoren, @methane, @markshannon, @tirkarthi, @sweeneyde
PRs
  • bpo-46903: Handle str-subclasses in virtual instance dictionaries. #31658
  • bpo-46903: Use assertEqual, not assertEquals in test_unicode #31718
  • 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 2022-03-06.20:19:52.404>
    created_at = <Date 2022-03-02.21:30:36.583>
    labels = ['interpreter-core', 'type-crash', '3.11']
    title = 'Crash when setting attribute with string subclass as the name (--with-pydebug)'
    updated_at = <Date 2022-03-07.07:06:38.693>
    user = 'https://github.com/ronaldoussoren'

    bugs.python.org fields:

    activity = <Date 2022-03-07.07:06:38.693>
    actor = 'Dennis Sweeney'
    assignee = 'none'
    closed = True
    closed_date = <Date 2022-03-06.20:19:52.404>
    closer = 'ronaldoussoren'
    components = ['Interpreter Core']
    creation = <Date 2022-03-02.21:30:36.583>
    creator = 'ronaldoussoren'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 46903
    keywords = ['patch']
    message_count = 7.0
    messages = ['414386', '414387', '414400', '414515', '414616', '414624', '414640']
    nosy_count = 5.0
    nosy_names = ['ronaldoussoren', 'methane', 'Mark.Shannon', 'xtreak', 'Dennis Sweeney']
    pr_nums = ['31658', '31718']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue46903'
    versions = ['Python 3.11']

    @ronaldoussoren
    Copy link
    Contributor Author

    The script below fails with an assertion error with python 3.11a5+ (current trunk) when build with --with-pydebug:

    # BEGIN OF FILE
    class String(str):
        __slots__ = ()
    
    name = String("hello")
    
    class Bag:
        pass
    
    o = Bag()
    setattr(o, name, 42)

    # END OF FILE

    Error output:

    % /tmp/py311/bin/python3 -Xdev str.py (master)pyobjc-8
    Assertion failed: (PyUnicode_CheckExact(name)), function _PyObject_StoreInstanceAttribute, file dictobject.c, line 5426.
    Fatal Python error: Aborted

    Current thread 0x0000000100a98580 (most recent call first):
    File "/Users/ronald/Projects/pyobjc-8/pyobjc-core/str.py", line 10 in <module>
    zsh: abort /tmp/py311/bin/python3 -Xdev str.py

    @ronaldoussoren ronaldoussoren added 3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump labels Mar 2, 2022
    @ronaldoussoren
    Copy link
    Contributor Author

    A hard crash seems wrong to me, and I'm not convinced yet that checking that the type of the attribute name is exactly string is correct.

    Found while debugging a crash in PyObjC's testsuite, PyObjC uses a subclass of str to represent Objective-C strings. Using such instances works fine in practice, but causes a crash in a --with-pydebug build due to this assertion.

    @sweeneyde
    Copy link
    Member

    a8b9350 is the first bad commit
    commit a8b9350
    Author: Mark Shannon <mark@hotpy.org>
    Date: Wed Oct 13 14:19:34 2021 +0100

    bpo-45340: Don't create object dictionaries unless actually needed (GH-28802)
    
    * Never change types' cached keys. It could invalidate inline attribute objects.
    
    * Lazily create object dictionaries.
    
    * Update specialization of LOAD/STORE_ATTR.
    
    * Don't update shared keys version for deletion of value.
    
    * Update gdb support to handle instance values.
    
    * Rename SPLIT_KEYS opcodes to INSTANCE_VALUE.
    

    @markshannon
    Copy link
    Member

    New changeset 03c2a36 by Mark Shannon in branch 'main':
    bpo-46903: Handle str-subclasses in virtual instance dictionaries. (GH-31658)
    03c2a36

    @markshannon
    Copy link
    Member

    Ronald, does PR 31658 fix your issue?

    @ronaldoussoren
    Copy link
    Contributor Author

    Your PR fixed the issue for me.

    @tirkarthi
    Copy link
    Member

    The PR introduced some deprecation warnings in tests.

    ./python -Wall -m test test_unicode
    0:00:00 load avg: 1.54 Run tests sequentially
    0:00:00 load avg: 1.54 [1/1] test_unicode
    /home/karthikeyan/stuff/python/cpython/Lib/test/test_unicode.py:3058: DeprecationWarning: Please use assertEqual instead.
    self.assertEquals(o.name, 1)
    /home/karthikeyan/stuff/python/cpython/Lib/test/test_unicode.py:3060: DeprecationWarning: Please use assertEqual instead.
    self.assertEquals(list(o.__dict__), [name])
    /home/karthikeyan/stuff/python/cpython/Lib/test/test_unicode.py:3067: DeprecationWarning: Please use assertEqual instead.
    self.assertEquals(o.name2, 3)
    /home/karthikeyan/stuff/python/cpython/Lib/test/test_unicode.py:3069: DeprecationWarning: Please use assertEqual instead.
    self.assertEquals(list(o.__dict__), [name, name2])

    == Tests result: SUCCESS ==

    1 test OK.

    Total duration: 952 ms
    Tests result: SUCCESS

    @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
    3.11 only security fixes 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

    4 participants