Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

various: Fix gcc -fsanitize=undefined errors #7237

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion extmod/crypto-algorithms/sha256.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ static void sha256_transform(CRYAL_SHA256_CTX *ctx, const BYTE data[])
WORD a, b, c, d, e, f, g, h, i, j, t1, t2, m[64];

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

Expand Down
4 changes: 3 additions & 1 deletion extmod/moduasyncio.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ typedef struct _mp_obj_task_t {
mp_obj_t ph_key;
} mp_obj_task_t;

#define PAIRHEAP(x) ((x) ? &x->pairheap : NULL)

typedef struct _mp_obj_task_queue_t {
mp_obj_base_t base;
mp_obj_task_t *heap;
Expand Down Expand Up @@ -103,7 +105,7 @@ STATIC mp_obj_t task_queue_push_sorted(size_t n_args, const mp_obj_t *args) {
assert(mp_obj_is_small_int(args[2]));
task->ph_key = args[2];
}
self->heap = (mp_obj_task_t *)mp_pairheap_push(task_lt, &self->heap->pairheap, &task->pairheap);
self->heap = (mp_obj_task_t *)mp_pairheap_push(task_lt, PAIRHEAP(self->heap), PAIRHEAP(task));
return mp_const_none;
}
STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(task_queue_push_sorted_obj, 2, 3, task_queue_push_sorted);
Expand Down
1 change: 1 addition & 0 deletions ports/unix/variants/coverage/mpconfigvariant.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
#define MICROPY_PY_UCRYPTOLIB (1)
#define MICROPY_PY_UCRYPTOLIB_CTR (1)
#define MICROPY_PY_MICROPYTHON_HEAP_LOCKED (1)
#define MICROPY_NONNULL_COMPLIANT (1)

// use vfs's functions for import stat and builtin open
#define mp_import_stat mp_vfs_import_stat
Expand Down
2 changes: 1 addition & 1 deletion py/binary.c
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ long long mp_binary_get_int(size_t size, bool is_signed, bool big_endian, const
delta = 1;
}

long long val = 0;
unsigned long long val = 0;
if (is_signed && *src & 0x80) {
val = -1;
}
Expand Down
23 changes: 23 additions & 0 deletions py/misc.h
Original file line number Diff line number Diff line change
Expand Up @@ -319,4 +319,27 @@ typedef const char *mp_rom_error_text_t;
// For now, forward directly to MP_COMPRESSED_ROM_TEXT.
#define MP_ERROR_TEXT(x) (mp_rom_error_text_t)MP_COMPRESSED_ROM_TEXT(x)

/** NULL-protected mem* wrappers ****/
// In ISO C, memcpy(dest, NULL, 0) and similar are undefined (the source
// and destination pointers are annotated as "non-null").
// This doesn't matter much, as the implementation provided on embedded
// platforms never dereferences the source or destintaion pointer if the
// length is zero. However, on Linux and particularly under undefined
// behavior sanitizers, this is treated as an error.
//
// Use the "0" alternative where a NULL pointer and a zero length can
// occur together, and on systems where compliance with ISO C is
// important, ensure that MICROPY_NONNULL_COMPLIANT is defined to (1).
#if MICROPY_NONNULL_COMPLIANT
#define memcpy0(dest, src, n) ((n) != 0 ? memcpy((dest), (src), (n)) : (dest))
#define memmove0(dest, src, n) ((n) != 0 ? memmove((dest), (src), (n)) : (dest))
#define memset0(s, c, n) ((n) != 0 ? memset((s), (c), (n)) : (s))
#define memcmp0(s1, s2, n) ((n) != 0 ? memcmp((s1), (s2), (n)) : (0))
#else
#define memcpy0(dest, src, n) memcpy((dest), (src), (n))
#define memmove0(dest, src, n) memmove((dest), (src), (n))
#define memset0(s, c, n) memset((s), (c), (n))
#define memcmp0(s1, s2, n) memcmp((s1), (s2), (n))
#endif

#endif // MICROPY_INCLUDED_PY_MISC_H
4 changes: 4 additions & 0 deletions py/mpconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -1718,4 +1718,8 @@ typedef double mp_float_t;
#endif
#endif

#if !defined(MICROPY_NONNULL_COMPLIANT)
#define MICROPY_NONNULL_COMPLIANT (0)
#endif

#endif // MICROPY_INCLUDED_PY_MPCONFIG_H
6 changes: 3 additions & 3 deletions py/mpz.c
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ STATIC mpz_t *mpz_clone(const mpz_t *src) {
z->alloc = src->alloc;
z->len = src->len;
z->dig = m_new(mpz_dig_t, z->alloc);
memcpy(z->dig, src->dig, src->alloc * sizeof(mpz_dig_t));
memcpy0(z->dig, src->dig, src->alloc * sizeof(mpz_dig_t));
return z;
}

Expand All @@ -708,7 +708,7 @@ void mpz_set(mpz_t *dest, const mpz_t *src) {
mpz_need_dig(dest, src->len);
dest->neg = src->neg;
dest->len = src->len;
memcpy(dest->dig, src->dig, src->len * sizeof(mpz_dig_t));
memcpy0(dest->dig, src->dig, src->len * sizeof(mpz_dig_t));
}

void mpz_set_from_int(mpz_t *z, mp_int_t val) {
Expand Down Expand Up @@ -741,7 +741,7 @@ void mpz_set_from_ll(mpz_t *z, long long val, bool is_signed) {
unsigned long long uval;
if (is_signed && val < 0) {
z->neg = 1;
uval = -val;
uval = -(unsigned long long)val;
} else {
z->neg = 0;
uval = val;
Expand Down
14 changes: 7 additions & 7 deletions py/obj.h
Original file line number Diff line number Diff line change
Expand Up @@ -993,26 +993,26 @@ void mp_seq_multiply(const void *items, size_t item_sz, size_t len, size_t times
#if MICROPY_PY_BUILTINS_SLICE
bool mp_seq_get_fast_slice_indexes(mp_uint_t len, mp_obj_t slice, mp_bound_slice_t *indexes);
#endif
#define mp_seq_copy(dest, src, len, item_t) memcpy(dest, src, len * sizeof(item_t))
#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)); }
#define mp_seq_copy(dest, src, len, item_t) memcpy0(dest, src, len * sizeof(item_t))
#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)); }
bool mp_seq_cmp_bytes(mp_uint_t op, const byte *data1, size_t len1, const byte *data2, size_t len2);
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);
mp_obj_t mp_seq_index_obj(const mp_obj_t *items, size_t len, size_t n_args, const mp_obj_t *args);
mp_obj_t mp_seq_count_obj(const mp_obj_t *items, size_t len, mp_obj_t value);
mp_obj_t mp_seq_extract_slice(size_t len, const mp_obj_t *seq, mp_bound_slice_t *indexes);

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

// Note: dest and slice regions may overlap
#define mp_seq_replace_slice_no_grow(dest, dest_len, beg, end, slice, slice_len, item_sz) \
memmove(((char *)dest) + (beg) * (item_sz), slice, slice_len * (item_sz)); \
memmove(((char *)dest) + (beg + slice_len) * (item_sz), ((char *)dest) + (end) * (item_sz), (dest_len - end) * (item_sz));
(void)memmove0(((char *)dest) + (beg) * (item_sz), slice, slice_len * (item_sz)); \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the (void) suppressing some warning here, or just to inicate the return value is unused?

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To mark it as intentionally unused. Without the cast, gcc encounters these diagnostics:

../../py/objlist.c: In function ‘list_subscr’:
../../py/misc.h:335:72: error: statement with no effect [-Werror=unused-value]
 #define memmove0(dest, src, n) ((n) != 0 ? memmove((dest), (src), (n)) : (dest))
                                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~
../../py/obj.h:1009:5: note: in expansion of macro ‘memmove0’
     memmove0(((char *)dest) + (beg) * (item_sz), slice, slice_len * (item_sz)); \
     ^~~~~~~~
../../py/objlist.c:169:13: note: in expansion of macro ‘mp_seq_replace_slice_no_grow’
             mp_seq_replace_slice_no_grow(self->items, self->len, slice.start, slice.stop, self->items /*NULL*/, 0, sizeof(*self->items));
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right, because it's not a single function call anymore. Makes sense.

(void)memmove0(((char *)dest) + (beg + slice_len) * (item_sz), ((char *)dest) + (end) * (item_sz), (dest_len - end) * (item_sz));

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

// Provide translation for legacy API
#define MP_OBJ_IS_SMALL_INT mp_obj_is_small_int
Expand Down
6 changes: 3 additions & 3 deletions py/objarray.c
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ STATIC mp_obj_t array_construct(char typecode, mp_obj_t initializer) {
size_t sz = mp_binary_get_size('@', typecode, NULL);
size_t len = bufinfo.len / sz;
mp_obj_array_t *o = array_new(typecode, len);
memcpy(o->items, bufinfo.buf, len * sz);
memcpy0(o->items, bufinfo.buf, len * sz);
return MP_OBJ_FROM_PTR(o);
}

Expand Down Expand Up @@ -482,7 +482,7 @@ STATIC mp_obj_t array_subscr(mp_obj_t self_in, mp_obj_t index_in, mp_obj_t value
#endif
{
res = array_new(o->typecode, slice.stop - slice.start);
memcpy(res->items, (uint8_t *)o->items + slice.start * sz, (slice.stop - slice.start) * sz);
memcpy0(res->items, (uint8_t *)o->items + slice.start * sz, (slice.stop - slice.start) * sz);
}
return MP_OBJ_FROM_PTR(res);
} else
Expand Down Expand Up @@ -599,7 +599,7 @@ size_t mp_obj_array_len(mp_obj_t self_in) {
#if MICROPY_PY_BUILTINS_BYTEARRAY
mp_obj_t mp_obj_new_bytearray(size_t n, void *items) {
mp_obj_array_t *o = array_new(BYTEARRAY_TYPECODE, n);
memcpy(o->items, items, n);
memcpy0(o->items, items, n);
return MP_OBJ_FROM_PTR(o);
}

Expand Down
2 changes: 1 addition & 1 deletion py/objdict.c
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ mp_obj_t mp_obj_dict_copy(mp_obj_t self_in) {
other->map.all_keys_are_qstrs = self->map.all_keys_are_qstrs;
other->map.is_fixed = 0;
other->map.is_ordered = self->map.is_ordered;
memcpy(other->map.table, self->map.table, self->map.alloc * sizeof(mp_map_elem_t));
memcpy0(other->map.table, self->map.table, self->map.alloc * sizeof(mp_map_elem_t));
return other_out;
}
STATIC MP_DEFINE_CONST_FUN_OBJ_1(dict_copy_obj, mp_obj_dict_copy);
Expand Down
6 changes: 3 additions & 3 deletions py/objint.c
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,14 @@ STATIC mp_fp_as_int_class_t mp_classify_fp_as_int(mp_float_t val) {
#if MICROPY_FLOAT_IMPL == MICROPY_FLOAT_IMPL_DOUBLE
e |= u.i[MP_ENDIANNESS_BIG] != 0;
#endif
if ((e & ~(1 << MP_FLOAT_SIGN_SHIFT_I32)) == 0) {
if ((e & ~(1U << MP_FLOAT_SIGN_SHIFT_I32)) == 0) {
// handle case of -0 (when sign is set but rest of bits are zero)
e = 0;
} else {
e += ((1 << MP_FLOAT_EXP_BITS) - 1) << MP_FLOAT_EXP_SHIFT_I32;
e += ((1U << MP_FLOAT_EXP_BITS) - 1) << MP_FLOAT_EXP_SHIFT_I32;
}
} else {
e &= ~((1 << MP_FLOAT_EXP_SHIFT_I32) - 1);
e &= ~((1U << MP_FLOAT_EXP_SHIFT_I32) - 1);
}
// 8 * sizeof(uintptr_t) counts the number of bits for a small int
// TODO provide a way to configure this properly
Expand Down
4 changes: 3 additions & 1 deletion py/objset.c
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,9 @@ STATIC mp_obj_t set_copy(mp_obj_t self_in) {
other->base.type = self->base.type;
mp_set_init(&other->set, self->set.alloc);
other->set.used = self->set.used;
memcpy(other->set.table, self->set.table, self->set.alloc * sizeof(mp_obj_t));
if (self->set.alloc) {
memcpy(other->set.table, self->set.table, self->set.alloc * sizeof(mp_obj_t));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this use memcpy0?

}
return MP_OBJ_FROM_PTR(other);
}
STATIC MP_DEFINE_CONST_FUN_OBJ_1(set_copy_obj, set_copy);
Expand Down
2 changes: 1 addition & 1 deletion py/qstr.c
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ qstr qstr_find_strn(const char *str, size_t str_len) {
// search pools for the data
for (qstr_pool_t *pool = MP_STATE_VM(last_pool); pool != NULL; pool = pool->prev) {
for (const byte **q = pool->qstrs, **q_top = pool->qstrs + pool->len; q < q_top; q++) {
if (Q_GET_HASH(*q) == str_hash && Q_GET_LENGTH(*q) == str_len && memcmp(Q_GET_DATA(*q), str, str_len) == 0) {
if (Q_GET_HASH(*q) == str_hash && Q_GET_LENGTH(*q) == str_len && memcmp0(Q_GET_DATA(*q), str, str_len) == 0) {
return pool->total_prev_len + (q - pool->qstrs);
}
}
Expand Down
2 changes: 1 addition & 1 deletion py/runtime.c
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ mp_obj_t mp_binary_op(mp_binary_op_t op, mp_obj_t lhs, mp_obj_t rhs) {
goto generic_binary_op;
} else {
// use standard precision
lhs_val <<= rhs_val;
lhs_val = (mp_uint_t)lhs_val << rhs_val;
}
break;
}
Expand Down
2 changes: 1 addition & 1 deletion py/sequence.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ bool mp_seq_cmp_bytes(mp_uint_t op, const byte *data1, size_t len1, const byte *
}
}
size_t min_len = len1 < len2 ? len1 : len2;
int res = memcmp(data1, data2, min_len);
int res = memcmp0(data1, data2, min_len);
if (op == MP_BINARY_OP_EQUAL) {
// If we are checking for equality, here's the answer
return res == 0;
Expand Down
2 changes: 1 addition & 1 deletion py/showbc.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ const byte *mp_bytecode_print_str(const mp_print_t *print, const byte *ip) {
num--;
}
do {
num = (num << 7) | (*ip & 0x7f);
num = ((mp_uint_t)num << 7) | (*ip & 0x7f);
} while ((*ip++ & 0x80) != 0);
mp_printf(print, "LOAD_CONST_SMALL_INT " INT_FMT, num);
break;
Expand Down
2 changes: 1 addition & 1 deletion py/smallint.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
#if MICROPY_OBJ_REPR == MICROPY_OBJ_REPR_A || MICROPY_OBJ_REPR == MICROPY_OBJ_REPR_C

#define MP_SMALL_INT_MIN ((mp_int_t)(((mp_int_t)MP_OBJ_WORD_MSBIT_HIGH) >> 1))
#define MP_SMALL_INT_FITS(n) ((((n) ^ ((n) << 1)) & MP_OBJ_WORD_MSBIT_HIGH) == 0)
#define MP_SMALL_INT_FITS(n) ((((n) ^ ((mp_uint_t)(n) << 1)) & MP_OBJ_WORD_MSBIT_HIGH) == 0)
// Mask to truncate mp_int_t to positive value
#define MP_SMALL_INT_POSITIVE_MASK ~(MP_OBJ_WORD_MSBIT_HIGH | (MP_OBJ_WORD_MSBIT_HIGH >> 1))

Expand Down
2 changes: 1 addition & 1 deletion py/vm.c
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ FRAME_SETUP();
DISPATCH();

ENTRY(MP_BC_LOAD_CONST_SMALL_INT): {
mp_int_t num = 0;
mp_uint_t num = 0;
if ((ip[0] & 0x40) != 0) {
// Number is negative
num--;
Expand Down