[Patches] fix array module bug introduced by overflow checking, take #2 (was:[Python-Dev] hey, who broke the array module?)
Trent Mick
trentm@activestate.com
Mon, 15 May 2000 14:09:44 -0700
Oops. The last patch had a small floating-point addition limitation in the
test suite on 64-bit platforms. Please ignore the last patch submission and
use this one instead.
Discussion:
Recent patches to getargs.c added overflow checking to the PyArg_Parse*()
integral formatters thereby restricting 'b' to unsigned char value and
'h','i', and 'l' to signed integral values. The unmatching signed and
unsigned format specifier for array.array() had to be fixed to allow setting
values in their full range.
test_array.py was also extended to check that one can set the full range of
values for each of the integral signed and unsigned array types.
Legal:
I confirm that, to the best of my knowledge and belief, this
contribution is free of any claims of third parties under
copyright, patent or other rights or interests ("claims"). To
the extent that I have any such claims, I hereby grant to CNRI a
nonexclusive, irrevocable, royalty-free, worldwide license to
reproduce, distribute, perform and/or display publicly, prepare
derivative versions, and otherwise use this contribution as part
of the Python software and its related documentation, or any
derivative versions thereof, at no cost to CNRI or its licensed
users, and to authorize others to do so.
I acknowledge that CNRI may, at its sole discretion, decide
whether or not to incorporate this contribution in the Python
software and its related documentation. I further grant CNRI
permission to use my name and other identifying information
provided to CNRI by me for use in connection with the Python
software and its related documentation.
Patch:
diff -c3 /home/trentm/main/contrib/python/dist/src/Modules/arraymodule.c /home/trentm/main/Apps/Perlium/Python/dist/src/Modules/arraymodule.c
*** /home/trentm/main/contrib/python/dist/src/Modules/arraymodule.c Wed May 3 16:44:31 2000
--- /home/trentm/main/Apps/Perlium/Python/dist/src/Modules/arraymodule.c Mon May 15 13:34:01 2000
***************
*** 114,124 ****
int i;
PyObject *v;
{
! char x;
! if (!PyArg_Parse(v, "b;array item must be integer", &x))
return -1;
if (i >= 0)
! ((char *)ap->ob_item)[i] = x;
return 0;
}
--- 114,137 ----
int i;
PyObject *v;
{
! short x;
! /* PyArg_Parse's 'b' formatter is for an unsigned char, therefore
! must use the next size up that is signed ('h') and manually do
! the overflow checking */
! if (!PyArg_Parse(v, "h;array item must be integer", &x))
! return -1;
! else if (x < CHAR_MIN) {
! PyErr_SetString(PyExc_OverflowError,
! "signed char is less than minimum");
! return -1;
! }
! else if (x > CHAR_MAX) {
! PyErr_SetString(PyExc_OverflowError,
! "signed char is greater than maximum");
return -1;
+ }
if (i >= 0)
! ((char *)ap->ob_item)[i] = (char)x;
return 0;
}
***************
*** 131,137 ****
return PyInt_FromLong(x);
}
! #define BB_setitem b_setitem
static PyObject *
h_getitem(ap, i)
--- 144,163 ----
return PyInt_FromLong(x);
}
! static int
! BB_setitem(ap, i, v)
! arrayobject *ap;
! int i;
! PyObject *v;
! {
! unsigned char x;
! /* 'B' == unsigned char, maps to PyArg_Parse's 'b' formatter */
! if (!PyArg_Parse(v, "b;array item must be integer", &x))
! return -1;
! if (i >= 0)
! ((char *)ap->ob_item)[i] = x;
! return 0;
! }
static PyObject *
h_getitem(ap, i)
***************
*** 148,153 ****
--- 174,180 ----
PyObject *v;
{
short x;
+ /* 'h' == signed short, maps to PyArg_Parse's 'h' formatter */
if (!PyArg_Parse(v, "h;array item must be integer", &x))
return -1;
if (i >= 0)
***************
*** 163,169 ****
return PyInt_FromLong((long) ((unsigned short *)ap->ob_item)[i]);
}
! #define HH_setitem h_setitem
static PyObject *
i_getitem(ap, i)
--- 190,220 ----
return PyInt_FromLong((long) ((unsigned short *)ap->ob_item)[i]);
}
! static int
! HH_setitem(ap, i, v)
! arrayobject *ap;
! int i;
! PyObject *v;
! {
! int x;
! /* PyArg_Parse's 'h' formatter is for a signed short, therefore
! must use the next size up and manually do the overflow checking */
! if (!PyArg_Parse(v, "i;array item must be integer", &x))
! return -1;
! else if (x < 0) {
! PyErr_SetString(PyExc_OverflowError,
! "unsigned short is less than minimum");
! return -1;
! }
! else if (x > USHRT_MAX) {
! PyErr_SetString(PyExc_OverflowError,
! "unsigned short is greater than maximum");
! return -1;
! }
! if (i >= 0)
! ((short *)ap->ob_item)[i] = (short)x;
! return 0;
! }
static PyObject *
i_getitem(ap, i)
***************
*** 180,185 ****
--- 231,237 ----
PyObject *v;
{
int x;
+ /* 'i' == signed int, maps to PyArg_Parse's 'i' formatter */
if (!PyArg_Parse(v, "i;array item must be integer", &x))
return -1;
if (i >= 0)
***************
*** 209,219 ****
return -1;
}
else {
! if (!PyArg_Parse(v, "l;array item must be integer", &x))
return -1;
}
if (i >= 0)
! ((unsigned int *)ap->ob_item)[i] = x;
return 0;
}
--- 261,285 ----
return -1;
}
else {
! long y;
! if (!PyArg_Parse(v, "l;array item must be integer", &y))
return -1;
+ if (y < 0) {
+ PyErr_SetString(PyExc_OverflowError,
+ "unsigned int is less than minimum");
+ return -1;
+ }
+ x = (unsigned long)y;
+
+ }
+ if (x > UINT_MAX) {
+ PyErr_SetString(PyExc_OverflowError,
+ "unsigned int is greater than maximum");
+ return -1;
}
+
if (i >= 0)
! ((unsigned int *)ap->ob_item)[i] = (unsigned int)x;
return 0;
}
***************
*** 260,268 ****
return -1;
}
else {
! if (!PyArg_Parse(v, "l;array item must be integer", &x))
return -1;
}
if (i >= 0)
((unsigned long *)ap->ob_item)[i] = x;
return 0;
--- 326,348 ----
return -1;
}
else {
! long y;
! if (!PyArg_Parse(v, "l;array item must be integer", &y))
! return -1;
! if (y < 0) {
! PyErr_SetString(PyExc_OverflowError,
! "unsigned long is less than minimum");
return -1;
+ }
+ x = (unsigned long)y;
+
+ }
+ if (x > ULONG_MAX) {
+ PyErr_SetString(PyExc_OverflowError,
+ "unsigned long is greater than maximum");
+ return -1;
}
+
if (i >= 0)
((unsigned long *)ap->ob_item)[i] = x;
return 0;
***************
*** 721,728 ****
arrayobject *self;
PyObject *args;
{
! return Py_BuildValue("ll",
! (long)(self->ob_item), (long)(self->ob_size));
}
static char buffer_info_doc [] =
--- 801,813 ----
arrayobject *self;
PyObject *args;
{
! PyObject* retval = PyTuple_New(2);
! if (!retval) return NULL;
!
! PyTuple_SET_ITEM(retval, 0, PyLong_FromVoidPtr(self->ob_item));
! PyTuple_SET_ITEM(retval, 1, PyInt_FromLong((long)(self->ob_size)));
!
! return retval;
}
static char buffer_info_doc [] =
***************
*** 937,943 ****
if (n > 0) {
char *item = self->ob_item;
int itemsize = self->ob_descr->itemsize;
! int nread;
int newlength;
size_t newbytes;
/* Be careful here about overflow */
--- 1022,1028 ----
if (n > 0) {
char *item = self->ob_item;
int itemsize = self->ob_descr->itemsize;
! size_t nread;
int newlength;
size_t newbytes;
/* Be careful here about overflow */
***************
*** 955,961 ****
self->ob_size += n;
nread = fread(item + (self->ob_size - n) * itemsize,
itemsize, n, fp);
! if (nread < n) {
self->ob_size -= (n - nread);
PyMem_RESIZE(item, char, self->ob_size*itemsize);
self->ob_item = item;
--- 1040,1046 ----
self->ob_size += n;
nread = fread(item + (self->ob_size - n) * itemsize,
itemsize, n, fp);
! if (nread < (size_t)n) {
self->ob_size -= (n - nread);
PyMem_RESIZE(item, char, self->ob_size*itemsize);
self->ob_item = item;
***************
*** 990,997 ****
return NULL;
}
if (self->ob_size > 0) {
! if ((int)fwrite(self->ob_item, self->ob_descr->itemsize,
! self->ob_size, fp) != self->ob_size) {
PyErr_SetFromErrno(PyExc_IOError);
clearerr(fp);
return NULL;
--- 1075,1082 ----
return NULL;
}
if (self->ob_size > 0) {
! if (fwrite(self->ob_item, self->ob_descr->itemsize,
! self->ob_size, fp) != (size_t)self->ob_size) {
PyErr_SetFromErrno(PyExc_IOError);
clearerr(fp);
return NULL;
diff -c3 /home/trentm/main/contrib/python/dist/src/Lib/test/test_array.py /home/trentm/main/Apps/Perlium/Python/dist/src/Lib/test/test_array.py
*** /home/trentm/main/contrib/python/dist/src/Lib/test/test_array.py Thu Jul 16 08:31:43 1998
--- /home/trentm/main/Apps/Perlium/Python/dist/src/Lib/test/test_array.py Mon May 15 15:06:00 2000
***************
*** 15,20 ****
--- 15,58 ----
unlink(TESTFN)
+ def testoverflow(type, lowerLimit, upperLimit):
+ # should not overflow assigning lower limit
+ if verbose:
+ print "overflow test: array(%s, [%s])" % (`type`, `lowerLimit`)
+ try:
+ a = array.array(type, [lowerLimit])
+ except:
+ raise TestFailed, "array(%s) overflowed assigning %s" %\
+ (`type`, `lowerLimit`)
+ # should overflow assigning less than lower limit
+ if verbose:
+ print "overflow test: array(%s, [%s])" % (`type`, `lowerLimit-1`)
+ try:
+ a = array.array(type, [lowerLimit-1])
+ raise TestFailed, "array(%s) did not overflow assigning %s" %\
+ (`type`, `lowerLimit-1`)
+ except OverflowError:
+ pass
+ # should not overflow assigning upper limit
+ if verbose:
+ print "overflow test: array(%s, [%s])" % (`type`, `upperLimit`)
+ try:
+ a = array.array(type, [upperLimit])
+ except:
+ raise TestFailed, "array(%s) overflowed assigning %s" %\
+ (`type`, `upperLimit`)
+ # should overflow assigning more than upper limit
+ if verbose:
+ print "overflow test: array(%s, [%s])" % (`type`, `upperLimit+1`)
+ try:
+ a = array.array(type, [upperLimit+1])
+ raise TestFailed, "array(%s) did not overflow assigning %s" %\
+ (`type`, `upperLimit+1`)
+ except OverflowError:
+ pass
+
+
+
def testtype(type, example):
a = array.array(type)
***************
*** 81,86 ****
if a != array.array(type, [1, 1, 2, 3, 4, 5, 5]):
raise TestFailed, "array(%s) self-slice-assign (cntr)" % `type`
!
main()
!
--- 119,138 ----
if a != array.array(type, [1, 1, 2, 3, 4, 5, 5]):
raise TestFailed, "array(%s) self-slice-assign (cntr)" % `type`
! # test that overflow exceptions are raised as expected for assignment
! # to array of specific integral types
! from math import pow
! if type in ('b', 'h', 'i', 'l'):
! # check signed and unsigned versions
! a = array.array(type)
! signedLowerLimit = -1 * long(pow(2, a.itemsize * 8 - 1))
! signedUpperLimit = long(pow(2, a.itemsize * 8 - 1)) - 1L
! unsignedLowerLimit = 0
! unsignedUpperLimit = long(pow(2, a.itemsize * 8)) - 1L
! testoverflow(type, signedLowerLimit, signedUpperLimit)
! testoverflow(type.upper(), unsignedLowerLimit, unsignedUpperLimit)
!
!
!
main()
!
--
Trent Mick
trentm@activestate.com