Skip to content

Commit 1355cf4

Browse files
pfalconpfalcon
committed
modstruct: Fix .calcsize() to account for struct type/alignment.
1 parent 5695e07 commit 1355cf4

4 files changed

Lines changed: 65 additions & 80 deletions

File tree

‎py/binary.c‎

Lines changed: 57 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include <stdint.h>
2+
#include <stdlib.h>
23
#include <assert.h>
34

45
#include "misc.h"
@@ -9,34 +10,52 @@
910

1011
// Helpers to work with binary-encoded data
1112

12-
int mp_binary_get_size(char typecode) {
13-
// This assumes that unsigned and signed types are of the same type,
14-
// which is invariant for [u]intN_t.
15-
switch (typecode) {
16-
case BYTEARRAY_TYPECODE:
17-
case 'b':
18-
case 'B':
19-
return sizeof(int8_t);
20-
case 'h':
21-
case 'H':
22-
return sizeof(int16_t);
23-
case 'i':
24-
case 'I':
25-
return sizeof(int32_t);
26-
case 'l':
27-
case 'L':
28-
return sizeof(int32_t);
29-
case 'q':
30-
case 'Q':
31-
return sizeof(long long);
32-
#if MICROPY_ENABLE_FLOAT
33-
case 'f':
34-
return sizeof(float);
35-
case 'd':
36-
return sizeof(double);
37-
#endif
13+
int mp_binary_get_size(char struct_type, char val_type, uint *palign) {
14+
int size = 0;
15+
int align = 1;
16+
switch (struct_type) {
17+
case '<': case '>':
18+
switch (val_type) {
19+
case 'b': case 'B':
20+
size = 1; break;
21+
case 'h': case 'H':
22+
size = 2; break;
23+
case 'i': case 'I':
24+
size = 4; break;
25+
case 'l': case 'L':
26+
size = 4; break;
27+
case 'q': case 'Q':
28+
size = 8; break;
29+
}
30+
break;
31+
case '@': {
32+
// TODO:
33+
// The simplest heuristic for alignment is to align by value
34+
// size, but that doesn't work for "bigger than int" types,
35+
// for example, long long may very well have long alignment
36+
// So, we introduce separate alignment handling, but having
37+
// formal support for that is different from actually supporting
38+
// particular (or any) ABI.
39+
switch (val_type) {
40+
case BYTEARRAY_TYPECODE:
41+
case 'b': case 'B':
42+
align = size = 1; break;
43+
case 'h': case 'H':
44+
align = size = sizeof(short); break;
45+
case 'i': case 'I':
46+
align = size = sizeof(int); break;
47+
case 'l': case 'L':
48+
align = size = sizeof(long); break;
49+
case 'q': case 'Q':
50+
// TODO: This is for x86
51+
align = sizeof(int); size = sizeof(long long); break;
52+
}
53+
}
3854
}
39-
return -1;
55+
if (palign != NULL) {
56+
*palign = align;
57+
}
58+
return size;
4059
}
4160

4261
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) {
8099
#define is_signed(typecode) (typecode > 'Z')
81100
mp_obj_t mp_binary_get_val(char struct_type, char val_type, byte **ptr) {
82101
byte *p = *ptr;
83-
uint size = 0;
84-
switch (struct_type) {
85-
case '<': case '>':
86-
switch (val_type) {
87-
case 'b': case 'B':
88-
size = 1; break;
89-
case 'h': case 'H':
90-
size = 2; break;
91-
case 'i': case 'I':
92-
size = 4; break;
93-
case 'l': case 'L':
94-
size = 4; break;
95-
case 'q': case 'Q':
96-
size = 8; break;
97-
}
98-
break;
99-
case '@': {
100-
// TODO:
101-
// The simplest heuristic for alignment is to align by value
102-
// size, but that doesn't work for "bigger than int" types,
103-
// for example, long long may very well have long alignment
104-
// So, we introduce separate alignment handling, but having
105-
// formal support for that is different from actually supporting
106-
// particular (or any) ABI.
107-
uint align = 0;
108-
switch (val_type) {
109-
case 'b': case 'B':
110-
align = size = 1; break;
111-
case 'h': case 'H':
112-
align = size = sizeof(short); break;
113-
case 'i': case 'I':
114-
align = size = sizeof(int); break;
115-
case 'l': case 'L':
116-
align = size = sizeof(long); break;
117-
case 'q': case 'Q':
118-
// TODO: This is for x86
119-
align = sizeof(int); size = sizeof(long long); break;
120-
}
121-
// Make pointer aligned
122-
p = (byte*)(((machine_uint_t)p + align - 1) & ~(align - 1));
123-
#if MP_ENDIANNESS_LITTLE
124-
struct_type = '<';
125-
#else
126-
struct_type = '>';
127-
#endif
128-
break;
129-
}
102+
uint align;
103+
104+
int size = mp_binary_get_size(struct_type, val_type, &align);
105+
if (struct_type == '@') {
106+
// Make pointer aligned
107+
p = (byte*)(((machine_uint_t)p + align - 1) & ~(align - 1));
108+
#if MP_ENDIANNESS_LITTLE
109+
struct_type = '<';
110+
#else
111+
struct_type = '>';
112+
#endif
130113
}
131114

132115
int delta;

‎py/binary.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// (underlyingly they're same).
33
#define BYTEARRAY_TYPECODE 0
44

5-
int mp_binary_get_size(char typecode);
5+
int mp_binary_get_size(char struct_type, char val_type, uint *palign);
66
mp_obj_t mp_binary_get_val_array(char typecode, void *p, int index);
77
mp_obj_t mp_binary_get_val(char struct_type, char val_type, byte **ptr);
88
void mp_binary_set_val_array(char typecode, void *p, int index, mp_obj_t val_in);

‎py/modstruct.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,14 @@ STATIC uint calcsize_items(const char *fmt) {
3737
STATIC mp_obj_t struct_calcsize(mp_obj_t fmt_in) {
3838
const char *fmt = mp_obj_str_get_str(fmt_in);
3939
char fmt_type = get_fmt_type(&fmt);
40-
(void)fmt_type;
4140
machine_uint_t size;
4241
for (size = 0; *fmt; fmt++) {
43-
int sz = mp_binary_get_size(*fmt);
42+
uint align;
43+
int sz = mp_binary_get_size(fmt_type, *fmt, &align);
4444
// TODO
4545
assert(sz != -1);
46+
// Apply alignment
47+
size = (size + align - 1) & ~(align - 1);
4648
size += sz;
4749
}
4850
return MP_OBJ_NEW_SMALL_INT(size);

‎py/objarray.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ STATIC mp_obj_t array_append(mp_obj_t self_in, mp_obj_t arg) {
121121
assert(MP_OBJ_IS_TYPE(self_in, &mp_type_array) || MP_OBJ_IS_TYPE(self_in, &mp_type_bytearray));
122122
mp_obj_array_t *self = self_in;
123123
if (self->free == 0) {
124-
int item_sz = mp_binary_get_size(self->typecode);
124+
int item_sz = mp_binary_get_size('@', self->typecode, NULL);
125125
// TODO: alloc policy
126126
self->free = 8;
127127
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
154154
STATIC machine_int_t array_get_buffer(mp_obj_t o_in, mp_buffer_info_t *bufinfo, int flags) {
155155
mp_obj_array_t *o = o_in;
156156
bufinfo->buf = o->items;
157-
bufinfo->len = o->len * mp_binary_get_size(o->typecode);
157+
bufinfo->len = o->len * mp_binary_get_size('@', o->typecode, NULL);
158158
bufinfo->typecode = o->typecode;
159159
return 0;
160160
}
@@ -190,7 +190,7 @@ const mp_obj_type_t mp_type_bytearray = {
190190
};
191191

192192
STATIC mp_obj_array_t *array_new(char typecode, uint n) {
193-
int typecode_size = mp_binary_get_size(typecode);
193+
int typecode_size = mp_binary_get_size('@', typecode, NULL);
194194
if (typecode_size <= 0) {
195195
nlr_raise(mp_obj_new_exception_msg(&mp_type_ValueError, "bad typecode"));
196196
}

0 commit comments

Comments
 (0)