Skip to content

Commit 29cd97e

Browse files
jeplerjepler
committed
various: Fix gcc -fsanitize=undefined errors.
[some line numbers below are from circuitpython and may not match; messages truncated to pass ci_commit_formatting_run] ``` binary.c:220:13: runtime error: signed integer overflow: 72057594037927... mpz.c:738:5: runtime error: null pointer passed as argument 2, which is... mpz.c:771:16: runtime error: negation of -9223372036854775808 cannot be... objarray.c:141:9: runtime error: null pointer passed as argument 1, whi... objarray.c:441:5: runtime error: null pointer passed as argument 1, whi... objarray.c:585:21: runtime error: null pointer passed as argument 1, wh... objarray.c:776:5: runtime error: null pointer passed as argument 1, whi... objdict.c:241:5: runtime error: null pointer passed as argument 2, whic... objint.c:111:22: runtime error: left shift of 1 by 31 places cannot be ... objint_mpz.c:387:9: runtime error: left shift of negative value -214748... objset.c:183:5: runtime error: null pointer passed as argument 1, which... qstr.c:187:75: runtime error: null pointer passed as argument 2, which ... runtime.c:398:33: runtime error: left shift of negative value -1 sequence.c:123:15: runtime error: null pointer passed as argument 1, wh... sha256.c:50:19: runtime error: left shift of 128 by 24 places cannot be... vm.c:328:36: runtime error: left shift of negative value -1 moduasyncio.c:108:35: runtime error: member access within null pointer ... showbc.c:177:28: runtime error: left shift of negative value -1 ``` With these cumulative changes, `make VARIANT=coverage test_full` with -fsanitize=undefined passes, except for some problems in axtls. My testing was done on an amd64 debian buster system using gcc-8.3 and these settings: ``` CFLAGS += -g3 -Og -fsanitize=undefined LDFLAGS += -fsanitize=undefined ``` The changes are intended/expected to produce no runtime overhead, as the behavior of the mem* functions is only actually modified for the unix coverage run (MICROPY_NONNULL_COMPLIANT). The introduced PAIRHEAP macro's conditional (x ? &x->i : NULL) assembles (under amd64 gcc 8.3 -Os) to the same as &x->i, since i is the initial field of the struct. However, for the purposes of undefined behavior analysis the conditional is needed. Signed-off-by: Jeff Epler <jepler@gmail.com>
1 parent 0e87459 commit 29cd97e

File tree

18 files changed

+59
-27
lines changed

18 files changed

+59
-27
lines changed

‎extmod/crypto-algorithms/sha256.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ static void sha256_transform(CRYAL_SHA256_CTX *ctx, const BYTE data[])
4646
WORD a, b, c, d, e, f, g, h, i, j, t1, t2, m[64];
4747

4848
for (i = 0, j = 0; i < 16; ++i, j += 4)
49-
m[i] = (data[j] << 24) | (data[j + 1] << 16) | (data[j + 2] << 8) | (data[j + 3]);
49+
m[i] = ((uint32_t)data[j] << 24) | (data[j + 1] << 16) | (data[j + 2] << 8) | (data[j + 3]);
5050
for ( ; i < 64; ++i)
5151
m[i] = SIG1(m[i - 2]) + m[i - 7] + SIG0(m[i - 15]) + m[i - 16];
5252

‎extmod/moduasyncio.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ typedef struct _mp_obj_task_t {
4040
mp_obj_t ph_key;
4141
} mp_obj_task_t;
4242

43+
#define PAIRHEAP(x) ((x) ? &x->pairheap : NULL)
44+
4345
typedef struct _mp_obj_task_queue_t {
4446
mp_obj_base_t base;
4547
mp_obj_task_t *heap;
@@ -103,7 +105,7 @@ STATIC mp_obj_t task_queue_push_sorted(size_t n_args, const mp_obj_t *args) {
103105
assert(mp_obj_is_small_int(args[2]));
104106
task->ph_key = args[2];
105107
}
106-
self->heap = (mp_obj_task_t *)mp_pairheap_push(task_lt, &self->heap->pairheap, &task->pairheap);
108+
self->heap = (mp_obj_task_t *)mp_pairheap_push(task_lt, PAIRHEAP(self->heap), PAIRHEAP(task));
107109
return mp_const_none;
108110
}
109111
STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(task_queue_push_sorted_obj, 2, 3, task_queue_push_sorted);

‎ports/unix/variants/coverage/mpconfigvariant.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
#define MICROPY_PY_UCRYPTOLIB (1)
6464
#define MICROPY_PY_UCRYPTOLIB_CTR (1)
6565
#define MICROPY_PY_MICROPYTHON_HEAP_LOCKED (1)
66+
#define MICROPY_NONNULL_COMPLIANT (1)
6667

6768
// use vfs's functions for import stat and builtin open
6869
#define mp_import_stat mp_vfs_import_stat

‎py/binary.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ long long mp_binary_get_int(size_t size, bool is_signed, bool big_endian, const
202202
delta = 1;
203203
}
204204

205-
long long val = 0;
205+
unsigned long long val = 0;
206206
if (is_signed && *src & 0x80) {
207207
val = -1;
208208
}

‎py/misc.h

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,4 +319,27 @@ typedef const char *mp_rom_error_text_t;
319319
// For now, forward directly to MP_COMPRESSED_ROM_TEXT.
320320
#define MP_ERROR_TEXT(x) (mp_rom_error_text_t)MP_COMPRESSED_ROM_TEXT(x)
321321

322+
/** NULL-protected mem* wrappers ****/
323+
// In ISO C, memcpy(dest, NULL, 0) and similar are undefined (the source
324+
// and destination pointers are annotated as "non-null").
325+
// This doesn't matter much, as the implementation provided on embedded
326+
// platforms never dereferences the source or destintaion pointer if the
327+
// length is zero. However, on Linux and particularly under undefined
328+
// behavior sanitizers, this is treated as an error.
329+
//
330+
// Use the "0" alternative where a NULL pointer and a zero length can
331+
// occur together, and on systems where compliance with ISO C is
332+
// important, ensure that MICROPY_NONNULL_COMPLIANT is defined to (1).
333+
#if MICROPY_NONNULL_COMPLIANT
334+
#define memcpy0(dest, src, n) ((n) != 0 ? memcpy((dest), (src), (n)) : (dest))
335+
#define memmove0(dest, src, n) ((n) != 0 ? memmove((dest), (src), (n)) : (dest))
336+
#define memset0(s, c, n) ((n) != 0 ? memset((s), (c), (n)) : (s))
337+
#define memcmp0(s1, s2, n) ((n) != 0 ? memcmp((s1), (s2), (n)) : (0))
338+
#else
339+
#define memcpy0(dest, src, n) memcpy((dest), (src), (n))
340+
#define memmove0(dest, src, n) memmove((dest), (src), (n))
341+
#define memset0(s, c, n) memset((s), (c), (n))
342+
#define memcmp0(s1, s2, n) memcmp((s1), (s2), (n))
343+
#endif
344+
322345
#endif // MICROPY_INCLUDED_PY_MISC_H

‎py/mpconfig.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,4 +1718,8 @@ typedef double mp_float_t;
17181718
#endif
17191719
#endif
17201720

1721+
#if !defined(MICROPY_NONNULL_COMPLIANT)
1722+
#define MICROPY_NONNULL_COMPLIANT (0)
1723+
#endif
1724+
17211725
#endif // MICROPY_INCLUDED_PY_MPCONFIG_H

‎py/mpz.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -697,7 +697,7 @@ STATIC mpz_t *mpz_clone(const mpz_t *src) {
697697
z->alloc = src->alloc;
698698
z->len = src->len;
699699
z->dig = m_new(mpz_dig_t, z->alloc);
700-
memcpy(z->dig, src->dig, src->alloc * sizeof(mpz_dig_t));
700+
memcpy0(z->dig, src->dig, src->alloc * sizeof(mpz_dig_t));
701701
return z;
702702
}
703703

@@ -708,7 +708,7 @@ void mpz_set(mpz_t *dest, const mpz_t *src) {
708708
mpz_need_dig(dest, src->len);
709709
dest->neg = src->neg;
710710
dest->len = src->len;
711-
memcpy(dest->dig, src->dig, src->len * sizeof(mpz_dig_t));
711+
memcpy0(dest->dig, src->dig, src->len * sizeof(mpz_dig_t));
712712
}
713713

714714
void mpz_set_from_int(mpz_t *z, mp_int_t val) {
@@ -741,7 +741,7 @@ void mpz_set_from_ll(mpz_t *z, long long val, bool is_signed) {
741741
unsigned long long uval;
742742
if (is_signed && val < 0) {
743743
z->neg = 1;
744-
uval = -val;
744+
uval = -(unsigned long long)val;
745745
} else {
746746
z->neg = 0;
747747
uval = val;

‎py/obj.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -993,26 +993,26 @@ void mp_seq_multiply(const void *items, size_t item_sz, size_t len, size_t times
993993
#if MICROPY_PY_BUILTINS_SLICE
994994
bool mp_seq_get_fast_slice_indexes(mp_uint_t len, mp_obj_t slice, mp_bound_slice_t *indexes);
995995
#endif
996-
#define mp_seq_copy(dest, src, len, item_t) memcpy(dest, src, len * sizeof(item_t))
997-
#define mp_seq_cat(dest, src1, len1, src2, len2, item_t) { memcpy(dest, src1, (len1) * sizeof(item_t)); memcpy(dest + (len1), src2, (len2) * sizeof(item_t)); }
996+
#define mp_seq_copy(dest, src, len, item_t) memcpy0(dest, src, len * sizeof(item_t))
997+
#define mp_seq_cat(dest, src1, len1, src2, len2, item_t) { memcpy0(dest, src1, (len1) * sizeof(item_t)); memcpy0(dest + (len1), src2, (len2) * sizeof(item_t)); }
998998
bool mp_seq_cmp_bytes(mp_uint_t op, const byte *data1, size_t len1, const byte *data2, size_t len2);
999999
bool mp_seq_cmp_objs(mp_uint_t op, const mp_obj_t *items1, size_t len1, const mp_obj_t *items2, size_t len2);
10001000
mp_obj_t mp_seq_index_obj(const mp_obj_t *items, size_t len, size_t n_args, const mp_obj_t *args);
10011001
mp_obj_t mp_seq_count_obj(const mp_obj_t *items, size_t len, mp_obj_t value);
10021002
mp_obj_t mp_seq_extract_slice(size_t len, const mp_obj_t *seq, mp_bound_slice_t *indexes);
10031003

10041004
// Helper to clear stale pointers from allocated, but unused memory, to preclude GC problems
1005-
#define mp_seq_clear(start, len, alloc_len, item_sz) memset((byte *)(start) + (len) * (item_sz), 0, ((alloc_len) - (len)) * (item_sz))
1005+
#define mp_seq_clear(start, len, alloc_len, item_sz) memset0((byte *)(start) + (len) * (item_sz), 0, ((alloc_len) - (len)) * (item_sz))
10061006

10071007
// Note: dest and slice regions may overlap
10081008
#define mp_seq_replace_slice_no_grow(dest, dest_len, beg, end, slice, slice_len, item_sz) \
1009-
memmove(((char *)dest) + (beg) * (item_sz), slice, slice_len * (item_sz)); \
1010-
memmove(((char *)dest) + (beg + slice_len) * (item_sz), ((char *)dest) + (end) * (item_sz), (dest_len - end) * (item_sz));
1009+
(void)memmove0(((char *)dest) + (beg) * (item_sz), slice, slice_len * (item_sz)); \
1010+
(void)memmove0(((char *)dest) + (beg + slice_len) * (item_sz), ((char *)dest) + (end) * (item_sz), (dest_len - end) * (item_sz));
10111011

10121012
// Note: dest and slice regions may overlap
10131013
#define mp_seq_replace_slice_grow_inplace(dest, dest_len, beg, end, slice, slice_len, len_adj, item_sz) \
1014-
memmove(((char *)dest) + (beg + slice_len) * (item_sz), ((char *)dest) + (end) * (item_sz), ((dest_len) + (len_adj) - ((beg) + (slice_len))) * (item_sz)); \
1015-
memmove(((char *)dest) + (beg) * (item_sz), slice, slice_len * (item_sz));
1014+
(void)memmove0(((char *)dest) + (beg + slice_len) * (item_sz), ((char *)dest) + (end) * (item_sz), ((dest_len) + (len_adj) - ((beg) + (slice_len))) * (item_sz)); \
1015+
(void)memmove0(((char *)dest) + (beg) * (item_sz), slice, slice_len * (item_sz));
10161016

10171017
// Provide translation for legacy API
10181018
#define MP_OBJ_IS_SMALL_INT mp_obj_is_small_int

‎py/objarray.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ STATIC mp_obj_t array_construct(char typecode, mp_obj_t initializer) {
127127
size_t sz = mp_binary_get_size('@', typecode, NULL);
128128
size_t len = bufinfo.len / sz;
129129
mp_obj_array_t *o = array_new(typecode, len);
130-
memcpy(o->items, bufinfo.buf, len * sz);
130+
memcpy0(o->items, bufinfo.buf, len * sz);
131131
return MP_OBJ_FROM_PTR(o);
132132
}
133133

@@ -482,7 +482,7 @@ STATIC mp_obj_t array_subscr(mp_obj_t self_in, mp_obj_t index_in, mp_obj_t value
482482
#endif
483483
{
484484
res = array_new(o->typecode, slice.stop - slice.start);
485-
memcpy(res->items, (uint8_t *)o->items + slice.start * sz, (slice.stop - slice.start) * sz);
485+
memcpy0(res->items, (uint8_t *)o->items + slice.start * sz, (slice.stop - slice.start) * sz);
486486
}
487487
return MP_OBJ_FROM_PTR(res);
488488
} else
@@ -599,7 +599,7 @@ size_t mp_obj_array_len(mp_obj_t self_in) {
599599
#if MICROPY_PY_BUILTINS_BYTEARRAY
600600
mp_obj_t mp_obj_new_bytearray(size_t n, void *items) {
601601
mp_obj_array_t *o = array_new(BYTEARRAY_TYPECODE, n);
602-
memcpy(o->items, items, n);
602+
memcpy0(o->items, items, n);
603603
return MP_OBJ_FROM_PTR(o);
604604
}
605605

‎py/objdict.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ mp_obj_t mp_obj_dict_copy(mp_obj_t self_in) {
247247
other->map.all_keys_are_qstrs = self->map.all_keys_are_qstrs;
248248
other->map.is_fixed = 0;
249249
other->map.is_ordered = self->map.is_ordered;
250-
memcpy(other->map.table, self->map.table, self->map.alloc * sizeof(mp_map_elem_t));
250+
memcpy0(other->map.table, self->map.table, self->map.alloc * sizeof(mp_map_elem_t));
251251
return other_out;
252252
}
253253
STATIC MP_DEFINE_CONST_FUN_OBJ_1(dict_copy_obj, mp_obj_dict_copy);

0 commit comments

Comments
 (0)