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

Using __slots__ with str derived classes can cause segfault #38693

Closed
jpe mannequin opened this issue Jun 20, 2003 · 7 comments
Closed

Using __slots__ with str derived classes can cause segfault #38693

jpe mannequin opened this issue Jun 20, 2003 · 7 comments

Comments

@jpe
Copy link
Mannequin

jpe mannequin commented Jun 20, 2003

BPO 757997
Nosy @gvanrossum
Files
  • slots.diff: fix crash w/slots 1
  • slots.diff: patch 2
  • 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 2003-07-16.17:09:12.000>
    created_at = <Date 2003-06-20.16:28:30.000>
    labels = []
    title = 'Using __slots__ with str derived classes can cause segfault'
    updated_at = <Date 2003-07-16.17:09:12.000>
    user = 'https://bugs.python.org/jpe'

    bugs.python.org fields:

    activity = <Date 2003-07-16.17:09:12.000>
    actor = 'jhylton'
    assignee = 'jhylton'
    closed = True
    closed_date = None
    closer = None
    components = ['None']
    creation = <Date 2003-06-20.16:28:30.000>
    creator = 'jpe'
    dependencies = []
    files = ['5406', '5407']
    hgrepos = []
    issue_num = 757997
    keywords = ['patch']
    message_count = 7.0
    messages = ['44085', '44086', '44087', '44088', '44089', '44090', '44091']
    nosy_count = 4.0
    nosy_names = ['gvanrossum', 'jhylton', 'nnorwitz', 'jpe']
    pr_nums = []
    priority = 'high'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue757997'
    versions = []

    @jpe
    Copy link
    Mannequin Author

    jpe mannequin commented Jun 20, 2003

    The following results in a segfault on Linux with 2.3b1:
    -------------------------

    class StringDerived(str):
      __slots__ = ['a']
      def __init__(self, string):
        str.__init__(self, string)
        self.a = 1
    
    class DerivedAgain(StringDerived):
      pass
    
    o = DerivedAgain('hello world')
    o.b = 2

    Python 2.2.2 raises a TypeError when attempting to
    derive a class from str with a non-empty __slots__,
    maybe 2.3 should do the same?

    I have no use case for deriving from str and defining
    __slots__; I encountered the bug when writing test
    cases for a debugger.

    @jpe jpe mannequin closed this as completed Jun 20, 2003
    @jpe jpe mannequin assigned jhylton Jun 20, 2003
    @jpe jpe mannequin closed this as completed Jun 20, 2003
    @jpe jpe mannequin assigned jhylton Jun 20, 2003
    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Jun 20, 2003

    Logged In: YES
    user_id=33168

    unicode, list, and dict don't crash, only string.

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Jun 20, 2003

    Logged In: YES
    user_id=33168

    I think the problem is that strings are variable length.
    clear_slots() doesn't handle this condtion. The attached
    patch fixes the crash and works fine under valgrind, but I'm
    not entirely sure it's correct. Hopefully someone more
    familiar with the TypeObject code can review this.

    I'll add a test later.

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Jun 20, 2003

    Logged In: YES
    user_id=33168

    Ain't testing grand. Found a couple more problems. Patch 2
    is better, but I'm still not sure it's complete or correct.

    I'm thinking the slot offset should be fixed when it's set,
    rather than at each usage of the slot offset.

    @gvanrossum
    Copy link
    Member

    Logged In: YES
    user_id=6380

    Neal, I don't think your patch can work, unless you
    change the *descriptors* used for slots also:
    the slot offsets used by the slot descriptors
    will still be constant and hence overwrite the
    characters of the string (see structmember.c).
    Example (with your patch):

    >>> class C(str): __slots__ = ['a'] 
    ...      
    >>> s = C("hello world") 
    >>> s.a 
    '\x00\x00\x00\x00\x90\x86\x12\x08\x00\x00' 
    >>>  

    I'd rather see the 2.2 situation back, where
    it's simply illegal to use __slots__ in a str subclass
    (or any subclass of a class whose tp_itemsize
    != 0).

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Jul 15, 2003

    Logged In: YES
    user_id=33168

    Jeremy, I agree with Guido, I think nonempty slots shouldn't
    be allowed for strings. I think this is just a matter of
    removing && !PyType_Check(base) in Objects/typeobject.c
    around line 1656. I don't have time to test it and I'm not
    sure what the ramifications of this change are. Can you get
    this into 2.3?

    @jhylton
    Copy link
    Mannequin

    jhylton mannequin commented Jul 16, 2003

    Logged In: YES
    user_id=31392

    My checkin 2.240 had the side-effect of restoring the
    behavior we got in 2.2.x. My checkin comment wasn't very
    clear about this (nor was the 2.215 comment that added this
    feature).

    Somehow the check for whether slots were allowed added a
    condition that could never be true. So there was no way to
    get the TypeError "nonempty __slots__ not supported for
    subtype of '%s'".

    I don't think the change was intentional.  In addition to
    the string crash, it created the possiblity for mutable tuples:
    >>> class T(tuple):
    ...     __slots__ = "x", "y"
    ... 
    >>> t = T((1, 2))
    >>> t
    (1, 2)
    >>> t.x
    1
    >>> t.y
    2
    >>> t.y = -1
    >>> t
    (1, -1)

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    None yet
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant