Skip to content

Commit f57f1ef

Browse files
author
Mark Wiebe
committed
BUG: dtype: Struct dtype size wasn't being padded for alignment (Ticket #1790)
In the case referenced in ticket #1790, the itemsize was set before the padded size was calculated, so the intent was clearly that it should have been aligned.
1 parent 6cc9613 commit f57f1ef

2 files changed

Lines changed: 50 additions & 10 deletions

File tree

‎numpy/core/src/multiarray/descriptor.c‎

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,8 @@ _convert_from_array_descr(PyObject *obj, int align)
399399

400400
_align = conv->alignment;
401401
if (_align > 1) {
402-
totalsize = ((totalsize + _align - 1)/_align)*_align;
402+
/* The alignment is always a power of 2, so this works */
403+
totalsize = (totalsize + _align - 1) & (-_align);
403404
}
404405
maxalign = MAX(maxalign, _align);
405406
}
@@ -432,14 +433,23 @@ _convert_from_array_descr(PyObject *obj, int align)
432433
totalsize += conv->elsize;
433434
Py_DECREF(tup);
434435
}
436+
437+
if (maxalign > 1) {
438+
/* The alignment is always a power of 2, so this works */
439+
totalsize = (totalsize + maxalign - 1) & (-maxalign);
440+
}
441+
435442
new = PyArray_DescrNewFromType(PyArray_VOID);
443+
if (new == NULL) {
444+
Py_XDECREF(fields);
445+
Py_XDECREF(nameslist);
446+
return NULL;
447+
}
436448
new->fields = fields;
437449
new->names = nameslist;
438450
new->elsize = totalsize;
439451
new->flags=dtypeflags;
440-
if (maxalign > 1) {
441-
totalsize = ((totalsize + maxalign - 1)/maxalign)*maxalign;
442-
}
452+
443453
/* Structured arrays get a sticky aligned bit */
444454
if (align) {
445455
new->flags |= NPY_ALIGNED_STRUCT;
@@ -508,7 +518,8 @@ _convert_from_list(PyObject *obj, int align)
508518

509519
_align = conv->alignment;
510520
if (_align > 1) {
511-
totalsize = ((totalsize + _align - 1)/_align)*_align;
521+
/* The alignment is always a power of 2, so this works */
522+
totalsize = (totalsize + _align - 1) & (-_align);
512523
}
513524
maxalign = MAX(maxalign, _align);
514525
}
@@ -523,7 +534,8 @@ _convert_from_list(PyObject *obj, int align)
523534
new->names = nameslist;
524535
new->flags = dtypeflags;
525536
if (maxalign > 1) {
526-
totalsize = ((totalsize+maxalign-1)/maxalign)*maxalign;
537+
/* The alignment is always a power of 2, so this works */
538+
totalsize = (totalsize + maxalign - 1) & (-maxalign);
527539
}
528540
/* Structured arrays get a sticky aligned bit */
529541
if (align) {
@@ -791,18 +803,28 @@ _convert_from_dict(PyObject *obj, int align)
791803
}
792804
offset = PyInt_AsLong(off);
793805
PyTuple_SET_ITEM(tup, 1, off);
794-
if (offset < totalsize) {
806+
/* If align=True, enforce field alignment */
807+
if (align && offset % newdescr->alignment != 0) {
808+
PyErr_Format(PyExc_ValueError,
809+
"offset %d for NumPy dtype with fields is "
810+
"not divisible by the field alignment %d "
811+
"with align=True",
812+
(int)offset, (int)newdescr->alignment);
813+
ret = PY_FAIL;
814+
}
815+
else if (offset < totalsize) {
795816
PyErr_SetString(PyExc_ValueError,
796817
"invalid offset (must be ordered)");
797818
ret = PY_FAIL;
798819
}
799-
if (offset > totalsize) {
820+
else if (offset > totalsize) {
800821
totalsize = offset;
801822
}
802823
}
803824
else {
804825
if (align && _align > 1) {
805-
totalsize = ((totalsize + _align - 1)/_align)*_align;
826+
/* The alignment is always a power of 2, so this works */
827+
totalsize = (totalsize + _align - 1) & (-_align);
806828
}
807829
PyTuple_SET_ITEM(tup, 1, PyInt_FromLong(totalsize));
808830
}
@@ -859,7 +881,8 @@ _convert_from_dict(PyObject *obj, int align)
859881
goto fail;
860882
}
861883
if (maxalign > 1) {
862-
totalsize = ((totalsize + maxalign - 1)/maxalign)*maxalign;
884+
/* The alignment is always a power of 2, so this works */
885+
totalsize = (totalsize + maxalign - 1) & (-maxalign);
863886
}
864887
if (align) {
865888
new->alignment = maxalign;

‎numpy/core/tests/test_dtype.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ def test_bad_param(self):
5050
assert_raises(ValueError, np.dtype, 'f4, i4', itemsize=7)
5151
# If alignment is enabled, the alignment (4) must divide the itemsize
5252
assert_raises(ValueError, np.dtype, 'f4, i1', align=True, itemsize=9)
53+
# If alignment is enabled, the individual fields must be aligned
54+
assert_raises(ValueError, np.dtype,
55+
{'names':['f0','f1'],
56+
'formats':['i1','f4'],
57+
'offsets':[0,2]}, align=True)
5358

5459
class TestRecord(TestCase):
5560
def test_equivalent_record(self):
@@ -81,6 +86,18 @@ def test_not_lists(self):
8186
self.assertRaises(TypeError, np.dtype,
8287
dict(names=['A', 'B'], formats=set(['f8', 'i4'])))
8388

89+
def test_aligned_size(self):
90+
# Check that structured dtypes get padded to an aligned size
91+
dt = np.dtype('i4, i1', align=True)
92+
assert_equal(dt.itemsize, 8)
93+
dt = np.dtype([('f0', 'i4'), ('f1', 'i1')], align=True)
94+
assert_equal(dt.itemsize, 8)
95+
dt = np.dtype({'names':['f0','f1'], 'formats':['i4', 'u1'],
96+
'offsets':[0,4]}, align=True)
97+
assert_equal(dt.itemsize, 8)
98+
dt = np.dtype({'f0': ('i4', 0), 'f1':('u1', 4)}, align=True)
99+
assert_equal(dt.itemsize, 8)
100+
84101
class TestSubarray(TestCase):
85102
def test_single_subarray(self):
86103
a = np.dtype((np.int, (2)))

0 commit comments

Comments
 (0)