From 1355cf42f23a39d3b887d2771d8bc7f3669d211c Mon Sep 17 00:00:00 2001 From: Paul Sokolovsky Date: Sat, 19 Apr 2014 01:25:49 +0300 Subject: [PATCH] modstruct: Fix .calcsize() to account for struct type/alignment. --- py/binary.c | 131 +++++++++++++++++++++---------------------------- py/binary.h | 2 +- py/modstruct.c | 6 ++- py/objarray.c | 6 +-- 4 files changed, 65 insertions(+), 80 deletions(-) diff --git a/py/binary.c b/py/binary.c index 0c9db9bd80..702a9cceb3 100644 --- a/py/binary.c +++ b/py/binary.c @@ -1,4 +1,5 @@ #include +#include #include #include "misc.h" @@ -9,34 +10,52 @@ // Helpers to work with binary-encoded data -int mp_binary_get_size(char typecode) { - // This assumes that unsigned and signed types are of the same type, - // which is invariant for [u]intN_t. - switch (typecode) { - case BYTEARRAY_TYPECODE: - case 'b': - case 'B': - return sizeof(int8_t); - case 'h': - case 'H': - return sizeof(int16_t); - case 'i': - case 'I': - return sizeof(int32_t); - case 'l': - case 'L': - return sizeof(int32_t); - case 'q': - case 'Q': - return sizeof(long long); -#if MICROPY_ENABLE_FLOAT - case 'f': - return sizeof(float); - case 'd': - return sizeof(double); -#endif +int mp_binary_get_size(char struct_type, char val_type, uint *palign) { + int size = 0; + int align = 1; + switch (struct_type) { + case '<': case '>': + switch (val_type) { + case 'b': case 'B': + size = 1; break; + case 'h': case 'H': + size = 2; break; + case 'i': case 'I': + size = 4; break; + case 'l': case 'L': + size = 4; break; + case 'q': case 'Q': + size = 8; break; + } + break; + case '@': { + // TODO: + // The simplest heuristic for alignment is to align by value + // size, but that doesn't work for "bigger than int" types, + // for example, long long may very well have long alignment + // So, we introduce separate alignment handling, but having + // formal support for that is different from actually supporting + // particular (or any) ABI. + switch (val_type) { + case BYTEARRAY_TYPECODE: + case 'b': case 'B': + align = size = 1; break; + case 'h': case 'H': + align = size = sizeof(short); break; + case 'i': case 'I': + align = size = sizeof(int); break; + case 'l': case 'L': + align = size = sizeof(long); break; + case 'q': case 'Q': + // TODO: This is for x86 + align = sizeof(int); size = sizeof(long long); break; + } + } } - return -1; + if (palign != NULL) { + *palign = align; + } + return size; } mp_obj_t mp_binary_get_val_array(char typecode, void *p, int index) { @@ -80,53 +99,17 @@ mp_obj_t mp_binary_get_val_array(char typecode, void *p, int index) { #define is_signed(typecode) (typecode > 'Z') mp_obj_t mp_binary_get_val(char struct_type, char val_type, byte **ptr) { byte *p = *ptr; - uint size = 0; - switch (struct_type) { - case '<': case '>': - switch (val_type) { - case 'b': case 'B': - size = 1; break; - case 'h': case 'H': - size = 2; break; - case 'i': case 'I': - size = 4; break; - case 'l': case 'L': - size = 4; break; - case 'q': case 'Q': - size = 8; break; - } - break; - case '@': { - // TODO: - // The simplest heuristic for alignment is to align by value - // size, but that doesn't work for "bigger than int" types, - // for example, long long may very well have long alignment - // So, we introduce separate alignment handling, but having - // formal support for that is different from actually supporting - // particular (or any) ABI. - uint align = 0; - switch (val_type) { - case 'b': case 'B': - align = size = 1; break; - case 'h': case 'H': - align = size = sizeof(short); break; - case 'i': case 'I': - align = size = sizeof(int); break; - case 'l': case 'L': - align = size = sizeof(long); break; - case 'q': case 'Q': - // TODO: This is for x86 - align = sizeof(int); size = sizeof(long long); break; - } - // Make pointer aligned - p = (byte*)(((machine_uint_t)p + align - 1) & ~(align - 1)); - #if MP_ENDIANNESS_LITTLE - struct_type = '<'; - #else - struct_type = '>'; - #endif - break; - } + uint align; + + int size = mp_binary_get_size(struct_type, val_type, &align); + if (struct_type == '@') { + // Make pointer aligned + p = (byte*)(((machine_uint_t)p + align - 1) & ~(align - 1)); + #if MP_ENDIANNESS_LITTLE + struct_type = '<'; + #else + struct_type = '>'; + #endif } int delta; diff --git a/py/binary.h b/py/binary.h index 538d6e7f29..54ff2dc56b 100644 --- a/py/binary.h +++ b/py/binary.h @@ -2,7 +2,7 @@ // (underlyingly they're same). #define BYTEARRAY_TYPECODE 0 -int mp_binary_get_size(char typecode); +int mp_binary_get_size(char struct_type, char val_type, uint *palign); mp_obj_t mp_binary_get_val_array(char typecode, void *p, int index); mp_obj_t mp_binary_get_val(char struct_type, char val_type, byte **ptr); void mp_binary_set_val_array(char typecode, void *p, int index, mp_obj_t val_in); diff --git a/py/modstruct.c b/py/modstruct.c index cd2516b240..0be194feec 100644 --- a/py/modstruct.c +++ b/py/modstruct.c @@ -37,12 +37,14 @@ STATIC uint calcsize_items(const char *fmt) { STATIC mp_obj_t struct_calcsize(mp_obj_t fmt_in) { const char *fmt = mp_obj_str_get_str(fmt_in); char fmt_type = get_fmt_type(&fmt); - (void)fmt_type; machine_uint_t size; for (size = 0; *fmt; fmt++) { - int sz = mp_binary_get_size(*fmt); + uint align; + int sz = mp_binary_get_size(fmt_type, *fmt, &align); // TODO assert(sz != -1); + // Apply alignment + size = (size + align - 1) & ~(align - 1); size += sz; } return MP_OBJ_NEW_SMALL_INT(size); diff --git a/py/objarray.c b/py/objarray.c index c6da45728a..2255e29d7b 100644 --- a/py/objarray.c +++ b/py/objarray.c @@ -121,7 +121,7 @@ STATIC mp_obj_t array_append(mp_obj_t self_in, mp_obj_t arg) { assert(MP_OBJ_IS_TYPE(self_in, &mp_type_array) || MP_OBJ_IS_TYPE(self_in, &mp_type_bytearray)); mp_obj_array_t *self = self_in; if (self->free == 0) { - int item_sz = mp_binary_get_size(self->typecode); + int item_sz = mp_binary_get_size('@', self->typecode, NULL); // TODO: alloc policy self->free = 8; self->items = m_realloc(self->items, item_sz * self->len, item_sz * (self->len + self->free)); @@ -154,7 +154,7 @@ STATIC mp_obj_t array_subscr(mp_obj_t self_in, mp_obj_t index_in, mp_obj_t value STATIC machine_int_t array_get_buffer(mp_obj_t o_in, mp_buffer_info_t *bufinfo, int flags) { mp_obj_array_t *o = o_in; bufinfo->buf = o->items; - bufinfo->len = o->len * mp_binary_get_size(o->typecode); + bufinfo->len = o->len * mp_binary_get_size('@', o->typecode, NULL); bufinfo->typecode = o->typecode; return 0; } @@ -190,7 +190,7 @@ const mp_obj_type_t mp_type_bytearray = { }; STATIC mp_obj_array_t *array_new(char typecode, uint n) { - int typecode_size = mp_binary_get_size(typecode); + int typecode_size = mp_binary_get_size('@', typecode, NULL); if (typecode_size <= 0) { nlr_raise(mp_obj_new_exception_msg(&mp_type_ValueError, "bad typecode")); }