Skip to content

Commit 2a6ba47

Browse files
Jongydpgeorge
authored andcommitted
py/obj: Add static safety checks to mp_obj_is_type().
Commit d96cfd1 introduced a regression by breaking existing users of mp_obj_is_type(.., &mp_obj_bool). This function (and associated helpers like mp_obj_is_int()) have some specific nuances, and mistakes like this one can happen again. This commit adds mp_obj_is_exact_type() which behaves like the the old mp_obj_is_type(). The new mp_obj_is_type() has the same prototype but it attempts to statically assert that it's not called with types which should be checked using mp_obj_is_type(). If called with any of these types: int, str, bool, NoneType - it will cause a compilation error. Additional checked types (e.g function types) can be added in the future. Existing users of mp_obj_is_type() with the now "invalid" types, were translated to use mp_obj_is_exact_type(). The use of MP_STATIC_ASSERT() is not bulletproof - usually GCC (and other compilers) can't statically check conditions that are only known during link-time (like variables' addresses comparison). However, in this case, GCC is able to statically detect these conditions, probably because it's the exact same object - `&mp_type_int == &mp_type_int` is detected. Misuses of this function with runtime-chosen types (e.g: `mp_obj_type_t *x = ...; mp_obj_is_type(..., x);` won't be detected. MSC is unable to detect this, so we use MP_STATIC_ASSERT_NOT_MSC(). Compiling with this commit and without the fix for d96cfd1 shows that it detects the problem. Signed-off-by: Yonatan Goldschmidt <[email protected]>
1 parent 6670281 commit 2a6ba47

File tree

10 files changed

+38
-24
lines changed

10 files changed

+38
-24
lines changed

py/binary.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ void mp_binary_set_val(char struct_type, char val_type, mp_obj_t val_in, byte *p
334334
#endif
335335
default:
336336
#if MICROPY_LONGINT_IMPL != MICROPY_LONGINT_IMPL_NONE
337-
if (mp_obj_is_type(val_in, &mp_type_int)) {
337+
if (mp_obj_is_exact_type(val_in, &mp_type_int)) {
338338
mp_obj_int_to_bytes_impl(val_in, struct_type == '>', size, p);
339339
return;
340340
}
@@ -371,7 +371,7 @@ void mp_binary_set_val_array(char typecode, void *p, size_t index, mp_obj_t val_
371371
break;
372372
default:
373373
#if MICROPY_LONGINT_IMPL != MICROPY_LONGINT_IMPL_NONE
374-
if (mp_obj_is_type(val_in, &mp_type_int)) {
374+
if (mp_obj_is_exact_type(val_in, &mp_type_int)) {
375375
size_t size = mp_binary_get_size('@', typecode, NULL);
376376
mp_obj_int_to_bytes_impl(val_in, MP_ENDIANNESS_BIG,
377377
size, (uint8_t *)p + index * size);

py/map.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ mp_map_elem_t *MICROPY_WRAP_MP_MAP_LOOKUP(mp_map_lookup)(mp_map_t * map, mp_obj_
174174
if (compare_only_ptrs) {
175175
if (mp_obj_is_qstr(index)) {
176176
// Index is a qstr, so can just do ptr comparison.
177-
} else if (mp_obj_is_type(index, &mp_type_str)) {
177+
} else if (mp_obj_is_exact_type(index, &mp_type_str)) {
178178
// Index is a non-interned string.
179179
// We can either intern the string, or force a full equality comparison.
180180
// We chose the latter, since interning costs time and potentially RAM,

py/obj.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ mp_int_t mp_obj_get_int(mp_const_obj_t arg) {
304304
return 1;
305305
} else if (mp_obj_is_small_int(arg)) {
306306
return MP_OBJ_SMALL_INT_VALUE(arg);
307-
} else if (mp_obj_is_type(arg, &mp_type_int)) {
307+
} else if (mp_obj_is_exact_type(arg, &mp_type_int)) {
308308
return mp_obj_int_get_checked(arg);
309309
} else {
310310
mp_obj_t res = mp_unary_op(MP_UNARY_OP_INT, (mp_obj_t)arg);
@@ -330,7 +330,7 @@ bool mp_obj_get_int_maybe(mp_const_obj_t arg, mp_int_t *value) {
330330
*value = 1;
331331
} else if (mp_obj_is_small_int(arg)) {
332332
*value = MP_OBJ_SMALL_INT_VALUE(arg);
333-
} else if (mp_obj_is_type(arg, &mp_type_int)) {
333+
} else if (mp_obj_is_exact_type(arg, &mp_type_int)) {
334334
*value = mp_obj_int_get_checked(arg);
335335
} else {
336336
return false;
@@ -349,7 +349,7 @@ bool mp_obj_get_float_maybe(mp_obj_t arg, mp_float_t *value) {
349349
} else if (mp_obj_is_small_int(arg)) {
350350
val = (mp_float_t)MP_OBJ_SMALL_INT_VALUE(arg);
351351
#if MICROPY_LONGINT_IMPL != MICROPY_LONGINT_IMPL_NONE
352-
} else if (mp_obj_is_type(arg, &mp_type_int)) {
352+
} else if (mp_obj_is_exact_type(arg, &mp_type_int)) {
353353
val = mp_obj_int_as_float_impl(arg);
354354
#endif
355355
} else if (mp_obj_is_float(arg)) {
@@ -389,7 +389,7 @@ bool mp_obj_get_complex_maybe(mp_obj_t arg, mp_float_t *real, mp_float_t *imag)
389389
*real = (mp_float_t)MP_OBJ_SMALL_INT_VALUE(arg);
390390
*imag = 0;
391391
#if MICROPY_LONGINT_IMPL != MICROPY_LONGINT_IMPL_NONE
392-
} else if (mp_obj_is_type(arg, &mp_type_int)) {
392+
} else if (mp_obj_is_exact_type(arg, &mp_type_int)) {
393393
*real = mp_obj_int_as_float_impl(arg);
394394
*imag = 0;
395395
#endif

py/obj.h

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -743,15 +743,29 @@ void *mp_obj_malloc_helper(size_t num_bytes, const mp_obj_type_t *type);
743743
// check for more specific object types.
744744
// Note: these are kept as macros because inline functions sometimes use much
745745
// more code space than the equivalent macros, depending on the compiler.
746-
#define mp_obj_is_type(o, t) (mp_obj_is_obj(o) && (((mp_obj_base_t *)MP_OBJ_TO_PTR(o))->type == (t))) // this does not work for checking int, str or fun; use below macros for that
746+
// don't use mp_obj_is_exact_type directly; use mp_obj_is_type which provides additional safety checks.
747+
// use the former only if you need to bypass these checks (because you've already checked everything else)
748+
#define mp_obj_is_exact_type(o, t) (mp_obj_is_obj(o) && (((mp_obj_base_t *)MP_OBJ_TO_PTR(o))->type == (t)))
749+
750+
// Type checks are split to a separate, constant result macro. This is so it doesn't hinder the compilers's
751+
// optimizations (other tricks like using ({ expr; exper; }) or (exp, expr, expr) in mp_obj_is_type() result
752+
// in missed optimizations)
753+
#define mp_type_assert_not_bool_int_str_nonetype(t) ( \
754+
MP_STATIC_ASSERT_NOT_MSC((t) != &mp_type_bool), \
755+
MP_STATIC_ASSERT_NOT_MSC((t) != &mp_type_int), \
756+
MP_STATIC_ASSERT_NOT_MSC((t) != &mp_type_str), \
757+
MP_STATIC_ASSERT_NOT_MSC((t) != &mp_type_NoneType), \
758+
1)
759+
760+
#define mp_obj_is_type(o, t) (mp_type_assert_not_bool_int_str_nonetype(t) && mp_obj_is_exact_type(o, t))
747761
#if MICROPY_OBJ_IMMEDIATE_OBJS
748762
// bool's are immediates, not real objects, so test for the 2 possible values.
749763
#define mp_obj_is_bool(o) ((o) == mp_const_false || (o) == mp_const_true)
750764
#else
751-
#define mp_obj_is_bool(o) mp_obj_is_type(o, &mp_type_bool)
765+
#define mp_obj_is_bool(o) mp_obj_is_exact_type(o, &mp_type_bool)
752766
#endif
753-
#define mp_obj_is_int(o) (mp_obj_is_small_int(o) || mp_obj_is_type(o, &mp_type_int))
754-
#define mp_obj_is_str(o) (mp_obj_is_qstr(o) || mp_obj_is_type(o, &mp_type_str))
767+
#define mp_obj_is_int(o) (mp_obj_is_small_int(o) || mp_obj_is_exact_type(o, &mp_type_int))
768+
#define mp_obj_is_str(o) (mp_obj_is_qstr(o) || mp_obj_is_exact_type(o, &mp_type_str))
755769
#define mp_obj_is_str_or_bytes(o) (mp_obj_is_qstr(o) || (mp_obj_is_obj(o) && ((mp_obj_base_t *)MP_OBJ_TO_PTR(o))->type->binary_op == mp_obj_str_binary_op))
756770
#define mp_obj_is_dict_or_ordereddict(o) (mp_obj_is_obj(o) && ((mp_obj_base_t *)MP_OBJ_TO_PTR(o))->type->make_new == mp_obj_dict_make_new)
757771
#define mp_obj_is_fun(o) (mp_obj_is_obj(o) && (((mp_obj_base_t *)MP_OBJ_TO_PTR(o))->type->name == MP_QSTR_function))

py/objexcept.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ STATIC mp_obj_exception_t *get_native_exception(mp_obj_t self_in) {
122122

123123
STATIC void decompress_error_text_maybe(mp_obj_exception_t *o) {
124124
#if MICROPY_ROM_TEXT_COMPRESSION
125-
if (o->args->len == 1 && mp_obj_is_type(o->args->items[0], &mp_type_str)) {
125+
if (o->args->len == 1 && mp_obj_is_exact_type(o->args->items[0], &mp_type_str)) {
126126
mp_obj_str_t *o_str = MP_OBJ_TO_PTR(o->args->items[0]);
127127
if (MP_IS_COMPRESSED_ROM_STRING(o_str->data)) {
128128
byte *buf = m_new_maybe(byte, MP_MAX_UNCOMPRESSED_TEXT_LEN + 1);

py/objfun.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ STATIC mp_uint_t convert_obj_for_inline_asm(mp_obj_t obj) {
468468
return 0;
469469
} else if (obj == mp_const_true) {
470470
return 1;
471-
} else if (mp_obj_is_type(obj, &mp_type_int)) {
471+
} else if (mp_obj_is_exact_type(obj, &mp_type_int)) {
472472
return mp_obj_int_get_truncated(obj);
473473
} else if (mp_obj_is_str(obj)) {
474474
// pointer to the string (it's probably constant though!)

py/objint.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ char *mp_obj_int_formatted(char **buf, size_t *buf_size, size_t *fmt_size, mp_co
232232
// A small int; get the integer value to format.
233233
num = MP_OBJ_SMALL_INT_VALUE(self_in);
234234
} else {
235-
assert(mp_obj_is_type(self_in, &mp_type_int));
235+
assert(mp_obj_is_exact_type(self_in, &mp_type_int));
236236
// Not a small int.
237237
#if MICROPY_LONGINT_IMPL == MICROPY_LONGINT_IMPL_LONGLONG
238238
const mp_obj_int_t *self = self_in;

py/objint_longlong.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ mp_obj_t mp_obj_int_from_bytes_impl(bool big_endian, size_t len, const byte *buf
5858
}
5959

6060
void mp_obj_int_to_bytes_impl(mp_obj_t self_in, bool big_endian, size_t len, byte *buf) {
61-
assert(mp_obj_is_type(self_in, &mp_type_int));
61+
assert(mp_obj_is_exact_type(self_in, &mp_type_int));
6262
mp_obj_int_t *self = self_in;
6363
long long val = self->val;
6464
if (big_endian) {
@@ -131,13 +131,13 @@ mp_obj_t mp_obj_int_binary_op(mp_binary_op_t op, mp_obj_t lhs_in, mp_obj_t rhs_i
131131
if (mp_obj_is_small_int(lhs_in)) {
132132
lhs_val = MP_OBJ_SMALL_INT_VALUE(lhs_in);
133133
} else {
134-
assert(mp_obj_is_type(lhs_in, &mp_type_int));
134+
assert(mp_obj_is_exact_type(lhs_in, &mp_type_int));
135135
lhs_val = ((mp_obj_int_t *)lhs_in)->val;
136136
}
137137

138138
if (mp_obj_is_small_int(rhs_in)) {
139139
rhs_val = MP_OBJ_SMALL_INT_VALUE(rhs_in);
140-
} else if (mp_obj_is_type(rhs_in, &mp_type_int)) {
140+
} else if (mp_obj_is_exact_type(rhs_in, &mp_type_int)) {
141141
rhs_val = ((mp_obj_int_t *)rhs_in)->val;
142142
} else {
143143
// delegate to generic function to check for extra cases
@@ -284,7 +284,7 @@ mp_int_t mp_obj_int_get_checked(mp_const_obj_t self_in) {
284284

285285
#if MICROPY_PY_BUILTINS_FLOAT
286286
mp_float_t mp_obj_int_as_float_impl(mp_obj_t self_in) {
287-
assert(mp_obj_is_type(self_in, &mp_type_int));
287+
assert(mp_obj_is_exact_type(self_in, &mp_type_int));
288288
mp_obj_int_t *self = self_in;
289289
return self->val;
290290
}

py/objint_mpz.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ mp_obj_int_t *mp_obj_int_new_mpz(void) {
9191
// This particular routine should only be called for the mpz representation of the int.
9292
char *mp_obj_int_formatted_impl(char **buf, size_t *buf_size, size_t *fmt_size, mp_const_obj_t self_in,
9393
int base, const char *prefix, char base_char, char comma) {
94-
assert(mp_obj_is_type(self_in, &mp_type_int));
94+
assert(mp_obj_is_exact_type(self_in, &mp_type_int));
9595
const mp_obj_int_t *self = MP_OBJ_TO_PTR(self_in);
9696

9797
size_t needed_size = mp_int_format_size(mpz_max_num_bits(&self->mpz), base, prefix, comma);
@@ -113,7 +113,7 @@ mp_obj_t mp_obj_int_from_bytes_impl(bool big_endian, size_t len, const byte *buf
113113
}
114114

115115
void mp_obj_int_to_bytes_impl(mp_obj_t self_in, bool big_endian, size_t len, byte *buf) {
116-
assert(mp_obj_is_type(self_in, &mp_type_int));
116+
assert(mp_obj_is_exact_type(self_in, &mp_type_int));
117117
mp_obj_int_t *self = MP_OBJ_TO_PTR(self_in);
118118
mpz_as_bytes(&self->mpz, big_endian, len, buf);
119119
}
@@ -181,15 +181,15 @@ mp_obj_t mp_obj_int_binary_op(mp_binary_op_t op, mp_obj_t lhs_in, mp_obj_t rhs_i
181181
mpz_init_fixed_from_int(&z_int, z_int_dig, MPZ_NUM_DIG_FOR_INT, MP_OBJ_SMALL_INT_VALUE(lhs_in));
182182
zlhs = &z_int;
183183
} else {
184-
assert(mp_obj_is_type(lhs_in, &mp_type_int));
184+
assert(mp_obj_is_exact_type(lhs_in, &mp_type_int));
185185
zlhs = &((mp_obj_int_t *)MP_OBJ_TO_PTR(lhs_in))->mpz;
186186
}
187187

188188
// if rhs is small int, then lhs was not (otherwise mp_binary_op handles it)
189189
if (mp_obj_is_small_int(rhs_in)) {
190190
mpz_init_fixed_from_int(&z_int, z_int_dig, MPZ_NUM_DIG_FOR_INT, MP_OBJ_SMALL_INT_VALUE(rhs_in));
191191
zrhs = &z_int;
192-
} else if (mp_obj_is_type(rhs_in, &mp_type_int)) {
192+
} else if (mp_obj_is_exact_type(rhs_in, &mp_type_int)) {
193193
zrhs = &((mp_obj_int_t *)MP_OBJ_TO_PTR(rhs_in))->mpz;
194194
#if MICROPY_PY_BUILTINS_FLOAT
195195
} else if (mp_obj_is_float(rhs_in)) {
@@ -447,7 +447,7 @@ mp_uint_t mp_obj_int_get_uint_checked(mp_const_obj_t self_in) {
447447

448448
#if MICROPY_PY_BUILTINS_FLOAT
449449
mp_float_t mp_obj_int_as_float_impl(mp_obj_t self_in) {
450-
assert(mp_obj_is_type(self_in, &mp_type_int));
450+
assert(mp_obj_is_exact_type(self_in, &mp_type_int));
451451
mp_obj_int_t *self = MP_OBJ_TO_PTR(self_in);
452452
return mpz_as_float(&self->mpz);
453453
}

py/objstr.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2144,7 +2144,7 @@ STATIC NORETURN void bad_implicit_conversion(mp_obj_t self_in) {
21442144
qstr mp_obj_str_get_qstr(mp_obj_t self_in) {
21452145
if (mp_obj_is_qstr(self_in)) {
21462146
return MP_OBJ_QSTR_VALUE(self_in);
2147-
} else if (mp_obj_is_type(self_in, &mp_type_str)) {
2147+
} else if (mp_obj_is_exact_type(self_in, &mp_type_str)) {
21482148
mp_obj_str_t *self = MP_OBJ_TO_PTR(self_in);
21492149
return qstr_from_strn((char *)self->data, self->len);
21502150
} else {

0 commit comments

Comments
 (0)