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.

2 files changed, 50 insertions, 10 deletions

diff --git a/numpy/core/src/multiarray/descriptor.c b/numpy/core/src/multiarray/descriptor.c
index e071df94d..c97f8e4cc 100644
--- a/numpy/core/src/multiarray/descriptor.c
+++ b/numpy/core/src/multiarray/descriptor.c
@@ -399,7 +399,8 @@ _convert_from_array_descr(PyObject *obj, int align)
 
             _align = conv->alignment;
             if (_align > 1) {
-                totalsize = ((totalsize + _align - 1)/_align)*_align;
+                /* The alignment is always a power of 2, so this works */
+                totalsize = (totalsize + _align - 1) & (-_align);
             }
             maxalign = MAX(maxalign, _align);
         }
@@ -432,14 +433,23 @@ _convert_from_array_descr(PyObject *obj, int align)
         totalsize += conv->elsize;
         Py_DECREF(tup);
     }
+
+    if (maxalign > 1) {
+        /* The alignment is always a power of 2, so this works */
+        totalsize = (totalsize + maxalign - 1) & (-maxalign);
+    }
+
     new = PyArray_DescrNewFromType(PyArray_VOID);
+    if (new == NULL) {
+        Py_XDECREF(fields);
+        Py_XDECREF(nameslist);
+        return NULL;
+    }
     new->fields = fields;
     new->names = nameslist;
     new->elsize = totalsize;
     new->flags=dtypeflags;
-    if (maxalign > 1) {
-        totalsize = ((totalsize + maxalign - 1)/maxalign)*maxalign;
-    }
+
     /* Structured arrays get a sticky aligned bit */
     if (align) {
         new->flags |= NPY_ALIGNED_STRUCT;
@@ -508,7 +518,8 @@ _convert_from_list(PyObject *obj, int align)
 
             _align = conv->alignment;
             if (_align > 1) {
-                totalsize = ((totalsize + _align - 1)/_align)*_align;
+                /* The alignment is always a power of 2, so this works */
+                totalsize = (totalsize + _align - 1) & (-_align);
             }
             maxalign = MAX(maxalign, _align);
         }
@@ -523,7 +534,8 @@ _convert_from_list(PyObject *obj, int align)
     new->names = nameslist;
     new->flags = dtypeflags;
     if (maxalign > 1) {
-        totalsize = ((totalsize+maxalign-1)/maxalign)*maxalign;
+        /* The alignment is always a power of 2, so this works */
+        totalsize = (totalsize + maxalign - 1) & (-maxalign);
     }
     /* Structured arrays get a sticky aligned bit */
     if (align) {
@@ -791,18 +803,28 @@ _convert_from_dict(PyObject *obj, int align)
             }
             offset = PyInt_AsLong(off);
             PyTuple_SET_ITEM(tup, 1, off);
-            if (offset < totalsize) {
+            /* If align=True, enforce field alignment */
+            if (align && offset % newdescr->alignment != 0) {
+                PyErr_Format(PyExc_ValueError,
+                        "offset %d for NumPy dtype with fields is "
+                        "not divisible by the field alignment %d "
+                        "with align=True",
+                        (int)offset, (int)newdescr->alignment);
+                ret = PY_FAIL;
+            }
+            else if (offset < totalsize) {
                 PyErr_SetString(PyExc_ValueError,
                         "invalid offset (must be ordered)");
                 ret = PY_FAIL;
             }
-            if (offset > totalsize) {
+            else if (offset > totalsize) {
                 totalsize = offset;
             }
         }
         else {
             if (align && _align > 1) {
-                totalsize = ((totalsize + _align - 1)/_align)*_align;
+                /* The alignment is always a power of 2, so this works */
+                totalsize = (totalsize + _align - 1) & (-_align);
             }
             PyTuple_SET_ITEM(tup, 1, PyInt_FromLong(totalsize));
         }
@@ -859,7 +881,8 @@ _convert_from_dict(PyObject *obj, int align)
         goto fail;
     }
     if (maxalign > 1) {
-        totalsize = ((totalsize + maxalign - 1)/maxalign)*maxalign;
+        /* The alignment is always a power of 2, so this works */
+        totalsize = (totalsize + maxalign - 1) & (-maxalign);
     }
     if (align) {
         new->alignment = maxalign;
diff --git a/numpy/core/tests/test_dtype.py b/numpy/core/tests/test_dtype.py
index 83df4b7ed..740100d22 100644
--- a/numpy/core/tests/test_dtype.py
+++ b/numpy/core/tests/test_dtype.py
@@ -50,6 +50,11 @@ class TestBuiltin(TestCase):
         assert_raises(ValueError, np.dtype, 'f4, i4', itemsize=7)
         # If alignment is enabled, the alignment (4) must divide the itemsize
         assert_raises(ValueError, np.dtype, 'f4, i1', align=True, itemsize=9)
+        # If alignment is enabled, the individual fields must be aligned
+        assert_raises(ValueError, np.dtype,
+                        {'names':['f0','f1'],
+                         'formats':['i1','f4'],
+                         'offsets':[0,2]}, align=True)
 
 class TestRecord(TestCase):
     def test_equivalent_record(self):
@@ -81,6 +86,18 @@ class TestRecord(TestCase):
         self.assertRaises(TypeError, np.dtype,
             dict(names=['A', 'B'], formats=set(['f8', 'i4'])))
 
+    def test_aligned_size(self):
+        # Check that structured dtypes get padded to an aligned size
+        dt = np.dtype('i4, i1', align=True)
+        assert_equal(dt.itemsize, 8)
+        dt = np.dtype([('f0', 'i4'), ('f1', 'i1')], align=True)
+        assert_equal(dt.itemsize, 8)
+        dt = np.dtype({'names':['f0','f1'], 'formats':['i4', 'u1'],
+                        'offsets':[0,4]}, align=True)
+        assert_equal(dt.itemsize, 8)
+        dt = np.dtype({'f0': ('i4', 0), 'f1':('u1', 4)}, align=True)
+        assert_equal(dt.itemsize, 8)
+
 class TestSubarray(TestCase):
     def test_single_subarray(self):
         a = np.dtype((np.int, (2)))