pygame is sharing code with you

Bitbucket is a code hosting site. Unlimited public and private repositories. Free for small teams.

Don't show this again

pygame / pygame http://pygame.org/

Clone this repository (size: 15.4 MB): HTTPS / SSH
hg clone https://bitbucket.org/pygame/pygame
hg clone ssh://hg@bitbucket.org/pygame/pygame

Issues

#109 BufferProxy: IndexError exception thrown sporadically from _bufferproxy_write

Reported by lifning (last edited )

The call to PyArg_ParseTuple in _bufferproxy_write uses the wrong formatting string to fill &offset. It uses "i" which is for int, rather than "n" which is for Py_ssize_t. http://docs.python.org/c-api/arg.html

sizeof(Py_ssize_t) on x86_64 is 8, whereas sizeof(int) is 4. If PyArg_ParseTuple is given an "i" it will fill the least-significant (on little-endian) four bytes of the variable, leaving the most-significant untouched. This bug probably wasn't visible on 32-bit x86, where they are both 4 bytes wide.

The end result of this is that _bufferproxy_write ends up throwing a Python IndexError exception sporadically (whenever the uninitialized bytes are not zero) when the arguments passed are otherwise valid.

To reproduce on an x86_64 machine, call it repeatedly until some non-zero garbage value happens to occupy the more significant bytes of offset when _bufferproxy_write is called.

s = some_surface_or_sound
data = some_string_that_should_fit_in_the_buffer
encountered = False
while not encountered:
  try:
    s.get_buffer.write(data, 0)
  except IndexError:
    encountered = True

A fix is attached.

Status: open Responsible: Lenard Lindstrom Type: bug Priority: minor
Milestone: none Component: other Version: Development

Attachments

Comments and changes

  1. #1 lifning

    written

    • Changed content.
  2. #2 Lenard Lindstrom

    written

    If we are no longer longer concerned with Python 2.4 support then the patch will be applied as is.

  3. #3 lifning

    written

    If 2.4 support ends up being an issue, a slightly more hackish fix that should be compatible would be to initialize that variable to 0 where it is declared.

  4. #4 cgohlke

    written , last edited

    Since length is also a Py_ssize_t, PY_SSIZE_T_CLEAN needs to be defined (see http://docs.python.org/c-api/arg.html).

    There are two more PyArg_ParseTuple format mismatches: A int/unsigned in _freetype.c, and a long/Py_ssize_t in color.c

    A patch for Python >=2.5 is attached.

  5. #5 Lenard Lindstrom

    written

    • Changed responsible from nobody to llindstrom.
    • Changed status from new to open.

    The changes have been applied and tested on an x86_32 machine under linux and Windows. Now it needs testing on an x86_64 machine.

  6. #6 cgohlke

    written

    How about the proposed change to src/color.c? I have not looked at it in detail, but Py_ssize_t might be unnecessary for a value in range(1, 5). In any case, the current code needs attention for x64.

  7. #7 Lenard Lindstrom

    written

    I missed src/color.c. Changes applied in changeset b83d8cc291ff.

  8. #8 cgohlke

    written

    Thank you! That builds and tests OK on win32 and win-amd64.

Add comment / attachment

Verification: Please write the text from the image in the box (letters only)

captcha

Is that you, Humanoid? Is this me?