Ticket #715: trac_715-size_t-take2.patch

File trac_715-size_t-take2.patch, 8.4 KB (added by jpflori, 8 years ago)

Use size_t instead of Py_ssize_t for indices used by PyList_GET_ITEM

  • sage/structure/coerce_dict.pyx

    # HG changeset patch
    # User Jean-Pierre Flori <jean-pierre.flor@ssi.gouv.fr>
    # Date 1334152948 -7200
    # Node ID 3d6d0c5e1a8c69f177ec44771c5e01af356afcff
    # Parent  cbfa7f143f6d702264ca30e64b08bb4f728c53df
    #715: the C % operator can return negative results.
    
    diff --git a/sage/structure/coerce_dict.pyx b/sage/structure/coerce_dict.pyx
    a b  
    9393            sage: len(T)    # indirect doctest
    9494            0
    9595        """
    96         # r is a (weak) reference (typically to a parent),
    97         # and it knows the stored key of the unique triple r() had been part of.
    98         #
     96        # r is a (weak) reference (typically to a parent), and it knows the
     97        # stored key of the unique triple r() had been part of.
    9998        # We remove that unique triple from self.D
    100         cdef Py_ssize_t k1,k2,k3
     99        cdef size_t k1,k2,k3
    101100        k1,k2,k3 = r.key
    102         cdef int h = (k1 + 13*k2 ^ 503*k3) % PyList_GET_SIZE(self.D.buckets)
    103         cdef list bucket = <object>PyList_GET_ITEM(self.D.buckets,h)
     101        cdef size_t h = (k1 + 13*k2 ^ 503*k3)
     102        cdef list bucket = <object>PyList_GET_ITEM(self.D.buckets, h % PyList_GET_SIZE(self.D.buckets))
    104103        cdef int i
    105104        for i from 0 <= i < PyList_GET_SIZE(bucket) by 4:
    106             if <Py_ssize_t><object>PyList_GET_ITEM(bucket,i)==k1 and \
    107                <Py_ssize_t><object>PyList_GET_ITEM(bucket,i+1)==k2 and \
    108                <Py_ssize_t><object>PyList_GET_ITEM(bucket,i+2)==k3:
     105            if <size_t><object>PyList_GET_ITEM(bucket, i)==k1 and \
     106               <size_t><object>PyList_GET_ITEM(bucket, i+1)==k2 and \
     107               <size_t><object>PyList_GET_ITEM(bucket, i+2)==k3:
    109108                del bucket[i:i+4]
    110109                self.D._size -= 1
    111110                break
     
    211210        sage: len(LE)    # indirect doctest
    212211        1
    213212
     213    ..NOTE::
     214
     215        The index `h` corresponding to the key [k1, k2, k3] is computed as a
     216        value of unsigned type size_t as follows:
     217
     218        ..MATH::
     219
     220            h = id(k1) + 13*id(k2) \oplus 503 id(k3)
     221
     222        Indeed, although the PyList_GetItem function and corresponding
     223        PyList_GET_ITEM macro take a value of signed type Py_ssize_t as input
     224        for the index, they do not accept negative inputs as the higher level
     225        Python functions. Moreover, the above formula can overflow so that `h`
     226        might be considered as negative. Even though this value is taken
     227        modulo the size of the buckets' list before accessing the corresponding
     228        item, the Cython "%" operator behaves for values of type size_t and
     229        Py_ssize_t like the C "%" operator, rather than like the Python "%"
     230        operator as it does for values of type int. That is, it returns a
     231        result of the same sign as its input. Therefore, if `h` was defined as
     232        a signed value, we might access the list at a negative index and raise
     233        a segfault (and this has been observed on 32 bits systems, see
     234        :trac:`715` for details).
     235
    214236    AUTHORS:
    215237
    216238    - Robert Bradshaw, 2007-08
     
    353375        return self.get(k1, k2, k3)
    354376
    355377    cdef get(self, object k1, object k2, object k3):
    356         cdef Py_ssize_t h = (<Py_ssize_t><void *>k1 + 13*<Py_ssize_t><void *>k2 ^ 503*<Py_ssize_t><void *>k3)
    357         #if h < 0: h = -h  # shouldn't "%" result in a positive number anyway?
     378        cdef size_t h = (<size_t><void *>k1 + 13*<size_t><void *>k2 ^ 503*<size_t><void *>k3)
    358379        cdef Py_ssize_t i
    359380        cdef list all_buckets = self.buckets
    360381        cdef list bucket = <object>PyList_GET_ITEM(all_buckets, h % PyList_GET_SIZE(all_buckets))
    361382        cdef object tmp
    362383        for i from 0 <= i < PyList_GET_SIZE(bucket) by 4:
    363384            tmp = <object>PyList_GET_ITEM(bucket, i)
    364             if <Py_ssize_t>tmp == <Py_ssize_t><void *>k1:
     385            if <size_t>tmp == <size_t><void *>k1:
    365386                tmp = <object>PyList_GET_ITEM(bucket, i+1)
    366                 if <Py_ssize_t>tmp == <Py_ssize_t><void *>k2:
     387                if <size_t>tmp == <size_t><void *>k2:
    367388                    tmp = <object>PyList_GET_ITEM(bucket, i+2)
    368                     if <Py_ssize_t>tmp == <Py_ssize_t><void *>k3:
     389                    if <size_t>tmp == <size_t><void *>k3:
    369390                        return <object>PyList_GET_ITEM(bucket, i+3)
    370391        raise KeyError, (k1, k2, k3)
    371392
     
    390411    cdef set(self, object k1, object k2, object k3, value):
    391412        if self.threshold and self._size > len(self.buckets) * self.threshold:
    392413            self.resize()
    393         cdef Py_ssize_t h1 = <Py_ssize_t><void *>k1
    394         cdef Py_ssize_t h2 = <Py_ssize_t><void *>k2
    395         cdef Py_ssize_t h3 = <Py_ssize_t><void *>k3
    396 
    397         cdef Py_ssize_t h = (h1 + 13*h2 ^ 503*h3)
    398         #if h < 0: h = -h   # shouldn't "%" return a positive number anyway?
     414        cdef size_t h1 = <size_t><void *>k1
     415        cdef size_t h2 = <size_t><void *>k2
     416        cdef size_t h3 = <size_t><void *>k3
     417        cdef size_t h = (h1 + 13*h2 ^ 503*h3)
    399418        cdef Py_ssize_t i
    400419        cdef list bucket = <object>PyList_GET_ITEM(self.buckets, h % PyList_GET_SIZE(self.buckets))
    401420        cdef object tmp
    402421        for i from 0 <= i < PyList_GET_SIZE(bucket) by 4:
    403422            tmp = <object>PyList_GET_ITEM(bucket, i)
    404             if <Py_ssize_t>tmp == h1:
     423            if <size_t>tmp == h1:
    405424                tmp = <object>PyList_GET_ITEM(bucket, i+1)
    406                 if <Py_ssize_t>tmp == h2:
     425                if <size_t>tmp == h2:
    407426                    tmp = <object>PyList_GET_ITEM(bucket, i+2)
    408                     if <Py_ssize_t>tmp == h3:
     427                    if <size_t>tmp == h3:
    409428                        bucket[i+3] = value
    410429                        return
    411430        PyList_Append(bucket, h1)
     
    448467            k1, k2, k3 = k
    449468        except (TypeError,ValueError):
    450469            raise KeyError, k
    451         cdef Py_ssize_t h = (<Py_ssize_t><void *>k1 + 13*<Py_ssize_t><void *>k2 ^ 503*<Py_ssize_t><void *>k3)
    452         #if h < 0: h = -h
     470        cdef size_t h = (<size_t><void *>k1 + 13*<size_t><void *>k2 ^ 503*<size_t><void *>k3)
    453471        cdef Py_ssize_t i
    454472        cdef list bucket = <object>PyList_GET_ITEM(self.buckets, h % PyList_GET_SIZE(self.buckets))
    455473        for i from 0 <= i < PyList_GET_SIZE(bucket) by 4:
    456             if <Py_ssize_t><object>PyList_GET_ITEM(bucket, i) == <Py_ssize_t><void *>k1 and \
    457                <Py_ssize_t><object>PyList_GET_ITEM(bucket, i+1) == <Py_ssize_t><void *>k2 and \
    458                <Py_ssize_t><object>PyList_GET_ITEM(bucket, i+2) == <Py_ssize_t><void *>k3:
     474            if <size_t><object>PyList_GET_ITEM(bucket, i) == <size_t><void *>k1 and \
     475               <size_t><object>PyList_GET_ITEM(bucket, i+1) == <size_t><void *>k2 and \
     476               <size_t><object>PyList_GET_ITEM(bucket, i+2) == <size_t><void *>k3:
    459477                del bucket[i:i+4]
    460478                self._size -= 1
    461479                return
     
    485503            buckets = next_odd_prime(2*len(self.buckets))
    486504        cdef list old_buckets = self.buckets
    487505        cdef list bucket
    488         cdef Py_ssize_t i, h
     506        cdef Py_ssize_t i
     507        cdef size_t h
    489508        self.buckets = [[] for i from 0 <= i <  buckets]
    490         cdef Py_ssize_t k1,k2,k3
     509        cdef size_t k1,k2,k3
    491510        cdef object v
    492511        for bucket in old_buckets:
    493512            for i from 0 <= i < PyList_GET_SIZE(bucket) by 4:
    494                 k1 = <Py_ssize_t><object>PyList_GET_ITEM(bucket,i)
    495                 k2 = <Py_ssize_t><object>PyList_GET_ITEM(bucket,i+1)
    496                 k3 = <Py_ssize_t><object>PyList_GET_ITEM(bucket,i+2)
    497                 v  = <object>PyList_GET_ITEM(bucket,i+3)
    498                 h = (k1 + 13*k2 ^ 503*k3) % buckets # the new hash
    499                 self.buckets[h] += [k1,k2,k3,v]
     513                k1 = <size_t><object>PyList_GET_ITEM(bucket, i)
     514                k2 = <size_t><object>PyList_GET_ITEM(bucket, i+1)
     515                k3 = <size_t><object>PyList_GET_ITEM(bucket, i+2)
     516                v  = <object>PyList_GET_ITEM(bucket, i+3)
     517                h = (k1 + 13*k2 ^ 503*k3)
     518                self.buckets[h % buckets] += [k1,k2,k3,v]
    500519
    501520    def iteritems(self):
    502521        """
     
    567586        while self.bucket_iter is None:
    568587            self.bucket_iter = self.buckets.next()
    569588        self.bucket_iter = iter(self.bucket_iter)
    570         cdef Py_ssize_t k1,k2,k3
     589        cdef size_t k1,k2,k3
    571590        try:
    572591            k1 = self.bucket_iter.next()
    573592            k2 = self.bucket_iter.next()