Skip to content

Commit 413f34c

Browse files
jeplerdpgeorge
jepler
authored and
dpgeorge
committed
all: Fix signed shifts and NULL access errors from -fsanitize=undefined.
Fixes the following (the line numbers match commit 0e87459): ../../extmod/crypto-algorithms/sha256.c:49:19: runtime error: left shif... ../../extmod/moduasyncio.c:106:35: runtime error: member access within ... ../../py/binary.c:210:13: runtime error: left shift of negative value -... ../../py/mpz.c:744:16: runtime error: negation of -9223372036854775808 ... ../../py/objint.c:109:22: runtime error: left shift of 1 by 31 places c... ../../py/objint_mpz.c:374:9: runtime error: left shift of 4611686018427... ../../py/objint_mpz.c:374:9: runtime error: left shift of negative valu... ../../py/parsenum.c:106:14: runtime error: left shift of 46116860184273... ../../py/runtime.c:395:33: runtime error: left shift of negative value ... ../../py/showbc.c:177:28: runtime error: left shift of negative value -... ../../py/vm.c:321:36: runtime error: left shift of negative value -1``` 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 introduced TASK_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 0009a7d commit 413f34c

File tree

9 files changed

+14
-11
lines changed

9 files changed

+14
-11
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: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@
3131

3232
#if MICROPY_PY_UASYNCIO
3333

34+
// Used when task cannot be guaranteed to be non-NULL.
35+
#define TASK_PAIRHEAP(task) ((task) ? &(task)->pairheap : NULL)
36+
3437
#define TASK_STATE_RUNNING_NOT_WAITED_ON (mp_const_true)
3538
#define TASK_STATE_DONE_NOT_WAITED_ON (mp_const_none)
3639
#define TASK_STATE_DONE_WAS_WAITED_ON (mp_const_false)
@@ -110,7 +113,7 @@ STATIC mp_obj_t task_queue_push_sorted(size_t n_args, const mp_obj_t *args) {
110113
assert(mp_obj_is_small_int(args[2]));
111114
task->ph_key = args[2];
112115
}
113-
self->heap = (mp_obj_task_t *)mp_pairheap_push(task_lt, &self->heap->pairheap, &task->pairheap);
116+
self->heap = (mp_obj_task_t *)mp_pairheap_push(task_lt, TASK_PAIRHEAP(self->heap), TASK_PAIRHEAP(task));
114117
return mp_const_none;
115118
}
116119
STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(task_queue_push_sorted_obj, 2, 3, task_queue_push_sorted);

‎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/mpz.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -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/objint.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,14 @@ STATIC mp_fp_as_int_class_t mp_classify_fp_as_int(mp_float_t val) {
106106
#if MICROPY_FLOAT_IMPL == MICROPY_FLOAT_IMPL_DOUBLE
107107
e |= u.i[MP_ENDIANNESS_BIG] != 0;
108108
#endif
109-
if ((e & ~(1 << MP_FLOAT_SIGN_SHIFT_I32)) == 0) {
109+
if ((e & ~(1U << MP_FLOAT_SIGN_SHIFT_I32)) == 0) {
110110
// handle case of -0 (when sign is set but rest of bits are zero)
111111
e = 0;
112112
} else {
113-
e += ((1 << MP_FLOAT_EXP_BITS) - 1) << MP_FLOAT_EXP_SHIFT_I32;
113+
e += ((1U << MP_FLOAT_EXP_BITS) - 1) << MP_FLOAT_EXP_SHIFT_I32;
114114
}
115115
} else {
116-
e &= ~((1 << MP_FLOAT_EXP_SHIFT_I32) - 1);
116+
e &= ~((1U << MP_FLOAT_EXP_SHIFT_I32) - 1);
117117
}
118118
// 8 * sizeof(uintptr_t) counts the number of bits for a small int
119119
// TODO provide a way to configure this properly

‎py/runtime.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ mp_obj_t mp_binary_op(mp_binary_op_t op, mp_obj_t lhs, mp_obj_t rhs) {
392392
goto generic_binary_op;
393393
} else {
394394
// use standard precision
395-
lhs_val <<= rhs_val;
395+
lhs_val = (mp_uint_t)lhs_val << rhs_val;
396396
}
397397
break;
398398
}

‎py/showbc.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ const byte *mp_bytecode_print_str(const mp_print_t *print, const byte *ip) {
174174
num--;
175175
}
176176
do {
177-
num = (num << 7) | (*ip & 0x7f);
177+
num = ((mp_uint_t)num << 7) | (*ip & 0x7f);
178178
} while ((*ip++ & 0x80) != 0);
179179
mp_printf(print, "LOAD_CONST_SMALL_INT " INT_FMT, num);
180180
break;

‎py/smallint.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
#if MICROPY_OBJ_REPR == MICROPY_OBJ_REPR_A || MICROPY_OBJ_REPR == MICROPY_OBJ_REPR_C
3838

3939
#define MP_SMALL_INT_MIN ((mp_int_t)(((mp_int_t)MP_OBJ_WORD_MSBIT_HIGH) >> 1))
40-
#define MP_SMALL_INT_FITS(n) ((((n) ^ ((n) << 1)) & MP_OBJ_WORD_MSBIT_HIGH) == 0)
40+
#define MP_SMALL_INT_FITS(n) ((((n) ^ ((mp_uint_t)(n) << 1)) & MP_OBJ_WORD_MSBIT_HIGH) == 0)
4141
// Mask to truncate mp_int_t to positive value
4242
#define MP_SMALL_INT_POSITIVE_MASK ~(MP_OBJ_WORD_MSBIT_HIGH | (MP_OBJ_WORD_MSBIT_HIGH >> 1))
4343

‎py/vm.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ FRAME_SETUP();
312312
DISPATCH();
313313

314314
ENTRY(MP_BC_LOAD_CONST_SMALL_INT): {
315-
mp_int_t num = 0;
315+
mp_uint_t num = 0;
316316
if ((ip[0] & 0x40) != 0) {
317317
// Number is negative
318318
num--;

0 commit comments

Comments
 (0)