Skip to content

Commit 3c658a4

Browse files
dpgeorgedpgeorge
committed
py: Fix bug where GC collected native/viper/asm function data.
Because (for Thumb) a function pointer has the LSB set, pointers to dynamic functions in RAM (eg native, viper or asm functions) were not being traced by the GC. This patch is a comprehensive fix for this. Addresses issue adafruit#820.
1 parent 25fc41d commit 3c658a4

File tree

22 files changed

+208
-121
lines changed

22 files changed

+208
-121
lines changed

‎bare-arm/mpconfigport.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535

3636
#define BYTES_PER_WORD (4)
3737

38+
#define MICROPY_MAKE_POINTER_CALLABLE(p) ((void*)((mp_uint_t)(p) | 1))
39+
3840
#define UINT_FMT "%lu"
3941
#define INT_FMT "%ld"
4042

@@ -44,7 +46,7 @@ typedef void *machine_ptr_t; // must be of pointer size
4446
typedef const void *machine_const_ptr_t; // must be of pointer size
4547

4648
// extra built in names to add to the global namespace
47-
extern const struct _mp_obj_fun_native_t mp_builtin_open_obj;
49+
extern const struct _mp_obj_fun_builtin_t mp_builtin_open_obj;
4850
#define MICROPY_PORT_BUILTINS \
4951
{ MP_OBJ_NEW_QSTR(MP_QSTR_open), (mp_obj_t)&mp_builtin_open_obj },
5052

‎py/asmthumb.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,7 @@ uint asm_thumb_get_code_size(asm_thumb_t *as) {
132132
}
133133

134134
void *asm_thumb_get_code(asm_thumb_t *as) {
135-
// need to set low bit to indicate that it's thumb code
136-
return (void *)(((mp_uint_t)as->code_base) | 1);
135+
return as->code_base;
137136
}
138137

139138
/*

‎py/bc.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ typedef struct _mp_code_state {
5050
} mp_code_state;
5151

5252
mp_vm_return_kind_t mp_execute_bytecode(mp_code_state *code_state, volatile mp_obj_t inject_exc);
53-
void mp_setup_code_state(mp_code_state *code_state, mp_obj_t self_in, uint n_args, uint n_kw, const mp_obj_t *args);
53+
void mp_setup_code_state(mp_code_state *code_state, mp_obj_t self_in, mp_uint_t n_args, mp_uint_t n_kw, const mp_obj_t *args);
5454
void mp_bytecode_print(const void *descr, const byte *code, int len);
5555
void mp_bytecode_print2(const byte *code, int len);
5656

‎py/emitglue.c‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ mp_raw_code_t *mp_emit_glue_new_raw_code(void) {
5555
return rc;
5656
}
5757

58-
void mp_emit_glue_assign_bytecode(mp_raw_code_t *rc, byte *code, uint len, uint n_pos_args, uint n_kwonly_args, qstr *arg_names, uint scope_flags) {
58+
void mp_emit_glue_assign_bytecode(mp_raw_code_t *rc, byte *code, mp_uint_t len, mp_uint_t n_pos_args, mp_uint_t n_kwonly_args, qstr *arg_names, mp_uint_t scope_flags) {
5959
rc->kind = MP_CODE_BYTECODE;
6060
rc->scope_flags = scope_flags;
6161
rc->n_pos_args = n_pos_args;
@@ -65,7 +65,7 @@ void mp_emit_glue_assign_bytecode(mp_raw_code_t *rc, byte *code, uint len, uint
6565
rc->u_byte.len = len;
6666

6767
#ifdef DEBUG_PRINT
68-
DEBUG_printf("assign byte code: code=%p len=%u n_pos_args=%d n_kwonly_args=%d flags=%x\n", code, len, n_pos_args, n_kwonly_args, scope_flags);
68+
DEBUG_printf("assign byte code: code=%p len=" UINT_FMT " n_pos_args=" UINT_FMT " n_kwonly_args=" UINT_FMT " flags=%x\n", code, len, n_pos_args, n_kwonly_args, (uint)scope_flags);
6969
DEBUG_printf(" arg names:");
7070
for (int i = 0; i < n_pos_args + n_kwonly_args; i++) {
7171
DEBUG_printf(" %s", qstr_str(arg_names[i]));
@@ -74,7 +74,7 @@ void mp_emit_glue_assign_bytecode(mp_raw_code_t *rc, byte *code, uint len, uint
7474
#endif
7575
#if MICROPY_DEBUG_PRINTERS
7676
if (mp_verbose_flag > 0) {
77-
for (int i = 0; i < 128 && i < len; i++) {
77+
for (mp_uint_t i = 0; i < len; i++) {
7878
if (i > 0 && i % 16 == 0) {
7979
printf("\n");
8080
}
@@ -87,22 +87,21 @@ void mp_emit_glue_assign_bytecode(mp_raw_code_t *rc, byte *code, uint len, uint
8787
}
8888

8989
#if MICROPY_EMIT_NATIVE || MICROPY_EMIT_INLINE_THUMB
90-
void mp_emit_glue_assign_native(mp_raw_code_t *rc, mp_raw_code_kind_t kind, void *fun, uint len, int n_args, mp_uint_t type_sig) {
90+
void mp_emit_glue_assign_native(mp_raw_code_t *rc, mp_raw_code_kind_t kind, void *fun_data, mp_uint_t fun_len, mp_uint_t n_args, mp_uint_t type_sig) {
9191
assert(kind == MP_CODE_NATIVE_PY || kind == MP_CODE_NATIVE_VIPER || kind == MP_CODE_NATIVE_ASM);
9292
rc->kind = kind;
9393
rc->scope_flags = 0;
9494
rc->n_pos_args = n_args;
95-
rc->u_native.fun = fun;
95+
rc->u_native.fun_data = fun_data;
9696
rc->u_native.type_sig = type_sig;
9797

9898
#ifdef DEBUG_PRINT
99-
DEBUG_printf("assign native: kind=%d fun=%p len=%u n_args=%d\n", kind, fun, len, n_args);
100-
byte *fun_data = (byte*)(((mp_uint_t)fun) & (~1)); // need to clear lower bit in case it's thumb code
101-
for (int i = 0; i < 128 && i < len; i++) {
99+
DEBUG_printf("assign native: kind=%d fun=%p len=" UINT_FMT " n_args=" UINT_FMT "\n", kind, fun_data, fun_len, n_args);
100+
for (mp_uint_t i = 0; i < fun_len; i++) {
102101
if (i > 0 && i % 16 == 0) {
103102
DEBUG_printf("\n");
104103
}
105-
DEBUG_printf(" %02x", fun_data[i]);
104+
DEBUG_printf(" %02x", ((byte*)fun_data)[i]);
106105
}
107106
DEBUG_printf("\n");
108107

@@ -133,15 +132,15 @@ mp_obj_t mp_make_function_from_raw_code(mp_raw_code_t *rc, mp_obj_t def_args, mp
133132
break;
134133
#if MICROPY_EMIT_NATIVE
135134
case MP_CODE_NATIVE_PY:
136-
fun = mp_make_function_n(rc->n_pos_args, rc->u_native.fun);
135+
fun = mp_obj_new_fun_native(rc->n_pos_args, rc->u_native.fun_data);
137136
break;
138137
case MP_CODE_NATIVE_VIPER:
139-
fun = mp_obj_new_fun_viper(rc->n_pos_args, rc->u_native.fun, rc->u_native.type_sig);
138+
fun = mp_obj_new_fun_viper(rc->n_pos_args, rc->u_native.fun_data, rc->u_native.type_sig);
140139
break;
141140
#endif
142141
#if MICROPY_EMIT_INLINE_THUMB
143142
case MP_CODE_NATIVE_ASM:
144-
fun = mp_obj_new_fun_asm(rc->n_pos_args, rc->u_native.fun);
143+
fun = mp_obj_new_fun_asm(rc->n_pos_args, rc->u_native.fun_data);
145144
break;
146145
#endif
147146
default:
@@ -158,8 +157,8 @@ mp_obj_t mp_make_function_from_raw_code(mp_raw_code_t *rc, mp_obj_t def_args, mp
158157
return fun;
159158
}
160159

161-
mp_obj_t mp_make_closure_from_raw_code(mp_raw_code_t *rc, uint n_closed_over, const mp_obj_t *args) {
162-
DEBUG_OP_printf("make_closure_from_raw_code %p %u %p\n", rc, n_closed_over, args);
160+
mp_obj_t mp_make_closure_from_raw_code(mp_raw_code_t *rc, mp_uint_t n_closed_over, const mp_obj_t *args) {
161+
DEBUG_OP_printf("make_closure_from_raw_code %p " UINT_FMT " %p\n", rc, n_closed_over, args);
163162
// make function object
164163
mp_obj_t ffun;
165164
if (n_closed_over & 0x100) {

‎py/emitglue.h‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,26 +37,26 @@ typedef enum {
3737

3838
typedef struct _mp_code_t {
3939
mp_raw_code_kind_t kind : 3;
40-
uint scope_flags : 7;
41-
uint n_pos_args : 11;
42-
uint n_kwonly_args : 11;
40+
mp_uint_t scope_flags : 7;
41+
mp_uint_t n_pos_args : 11;
42+
mp_uint_t n_kwonly_args : 11;
4343
qstr *arg_names;
4444
union {
4545
struct {
4646
byte *code;
47-
uint len;
47+
mp_uint_t len;
4848
} u_byte;
4949
struct {
50-
void *fun;
50+
void *fun_data;
5151
mp_uint_t type_sig; // for viper, compressed as 2-bit types; ret is MSB, then arg0, arg1, etc
5252
} u_native;
5353
};
5454
} mp_raw_code_t;
5555

5656
mp_raw_code_t *mp_emit_glue_new_raw_code(void);
5757

58-
void mp_emit_glue_assign_bytecode(mp_raw_code_t *rc, byte *code, uint len, uint n_pos_args, uint n_kwonly_args, qstr *arg_names, uint scope_flags);
59-
void mp_emit_glue_assign_native(mp_raw_code_t *rc, mp_raw_code_kind_t kind, void *f, uint len, int n_args, mp_uint_t type_sig);
58+
void mp_emit_glue_assign_bytecode(mp_raw_code_t *rc, byte *code, mp_uint_t len, mp_uint_t n_pos_args, mp_uint_t n_kwonly_args, qstr *arg_names, mp_uint_t scope_flags);
59+
void mp_emit_glue_assign_native(mp_raw_code_t *rc, mp_raw_code_kind_t kind, void *fun_data, mp_uint_t fun_len, mp_uint_t n_args, mp_uint_t type_sig);
6060

6161
mp_obj_t mp_make_function_from_raw_code(mp_raw_code_t *rc, mp_obj_t def_args, mp_obj_t def_kw_args);
62-
mp_obj_t mp_make_closure_from_raw_code(mp_raw_code_t *rc, uint n_closed_over, const mp_obj_t *args);
62+
mp_obj_t mp_make_closure_from_raw_code(mp_raw_code_t *rc, mp_uint_t n_closed_over, const mp_obj_t *args);

‎py/mpconfig.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,12 @@ typedef double mp_float_t;
429429
#define MP_ENDIANNESS_LITTLE (0)
430430
#endif
431431

432+
// Make a pointer to RAM callable (eg set lower bit for Thumb code)
433+
// (This scheme won't work if we want to mix Thumb and normal ARM code.)
434+
#ifndef MICROPY_MAKE_POINTER_CALLABLE
435+
#define MICROPY_MAKE_POINTER_CALLABLE(p) (p)
436+
#endif
437+
432438
// printf format spec to use for mp_int_t and friends
433439
#ifndef INT_FMT
434440
#ifdef __LP64__

‎py/obj.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ mp_int_t mp_obj_hash(mp_obj_t o_in) {
161161
return mp_obj_str_get_hash(o_in);
162162
} else if (MP_OBJ_IS_TYPE(o_in, &mp_type_NoneType)) {
163163
return (mp_int_t)o_in;
164-
} else if (MP_OBJ_IS_TYPE(o_in, &mp_type_fun_native) || MP_OBJ_IS_TYPE(o_in, &mp_type_fun_bc)) {
164+
} else if (MP_OBJ_IS_FUN(o_in)) {
165165
return (mp_int_t)o_in;
166166
} else if (MP_OBJ_IS_TYPE(o_in, &mp_type_tuple)) {
167167
return mp_obj_tuple_hash(o_in);

‎py/obj.h‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,10 @@ typedef struct _mp_obj_base_t mp_obj_base_t;
7171
//#define MP_OBJ_IS_SMALL_INT(o) ((((mp_int_t)(o)) & 1) != 0)
7272
//#define MP_OBJ_IS_QSTR(o) ((((mp_int_t)(o)) & 3) == 2)
7373
//#define MP_OBJ_IS_OBJ(o) ((((mp_int_t)(o)) & 3) == 0)
74-
#define MP_OBJ_IS_TYPE(o, t) (MP_OBJ_IS_OBJ(o) && (((mp_obj_base_t*)(o))->type == (t))) // this does not work for checking a string, use below macro for that
74+
#define MP_OBJ_IS_TYPE(o, t) (MP_OBJ_IS_OBJ(o) && (((mp_obj_base_t*)(o))->type == (t))) // this does not work for checking int, str or fun; use below macros for that
7575
#define MP_OBJ_IS_INT(o) (MP_OBJ_IS_SMALL_INT(o) || MP_OBJ_IS_TYPE(o, &mp_type_int))
7676
#define MP_OBJ_IS_STR(o) (MP_OBJ_IS_QSTR(o) || MP_OBJ_IS_TYPE(o, &mp_type_str))
77+
#define MP_OBJ_IS_FUN(o) (MP_OBJ_IS_OBJ(o) && (((mp_obj_base_t*)(o))->type->binary_op == mp_obj_fun_binary_op))
7778

7879
#define MP_OBJ_SMALL_INT_VALUE(o) (((mp_int_t)(o)) >> 1)
7980
#define MP_OBJ_NEW_SMALL_INT(small_int) ((mp_obj_t)((((mp_int_t)(small_int)) << 1) | 1))
@@ -84,9 +85,9 @@ typedef struct _mp_obj_base_t mp_obj_base_t;
8485
// These macros are used to declare and define constant function objects
8586
// You can put "static" in front of the definitions to make them local
8687

87-
#define MP_DECLARE_CONST_FUN_OBJ(obj_name) extern const mp_obj_fun_native_t obj_name
88+
#define MP_DECLARE_CONST_FUN_OBJ(obj_name) extern const mp_obj_fun_builtin_t obj_name
8889

89-
#define MP_DEFINE_CONST_FUN_OBJ_VOID_PTR(obj_name, is_kw, n_args_min, n_args_max, fun_name) const mp_obj_fun_native_t obj_name = {{&mp_type_fun_native}, is_kw, n_args_min, n_args_max, (void *)fun_name}
90+
#define MP_DEFINE_CONST_FUN_OBJ_VOID_PTR(obj_name, is_kw, n_args_min, n_args_max, fun_name) const mp_obj_fun_builtin_t obj_name = {{&mp_type_fun_builtin}, is_kw, n_args_min, n_args_max, (void *)fun_name}
9091
#define MP_DEFINE_CONST_FUN_OBJ_0(obj_name, fun_name) MP_DEFINE_CONST_FUN_OBJ_VOID_PTR(obj_name, false, 0, 0, (mp_fun_0_t)fun_name)
9192
#define MP_DEFINE_CONST_FUN_OBJ_1(obj_name, fun_name) MP_DEFINE_CONST_FUN_OBJ_VOID_PTR(obj_name, false, 1, 1, (mp_fun_1_t)fun_name)
9293
#define MP_DEFINE_CONST_FUN_OBJ_2(obj_name, fun_name) MP_DEFINE_CONST_FUN_OBJ_VOID_PTR(obj_name, false, 2, 2, (mp_fun_2_t)fun_name)
@@ -178,7 +179,6 @@ typedef mp_obj_t (*mp_fun_0_t)(void);
178179
typedef mp_obj_t (*mp_fun_1_t)(mp_obj_t);
179180
typedef mp_obj_t (*mp_fun_2_t)(mp_obj_t, mp_obj_t);
180181
typedef mp_obj_t (*mp_fun_3_t)(mp_obj_t, mp_obj_t, mp_obj_t);
181-
typedef mp_obj_t (*mp_fun_t)(void);
182182
typedef mp_obj_t (*mp_fun_var_t)(uint n, const mp_obj_t *);
183183
typedef mp_obj_t (*mp_fun_kw_t)(uint n, const mp_obj_t *, mp_map_t *);
184184

@@ -304,7 +304,7 @@ extern const mp_obj_type_t mp_type_zip;
304304
extern const mp_obj_type_t mp_type_array;
305305
extern const mp_obj_type_t mp_type_super;
306306
extern const mp_obj_type_t mp_type_gen_instance;
307-
extern const mp_obj_type_t mp_type_fun_native;
307+
extern const mp_obj_type_t mp_type_fun_builtin;
308308
extern const mp_obj_type_t mp_type_fun_bc;
309309
extern const mp_obj_type_t mp_type_module;
310310
extern const mp_obj_type_t mp_type_staticmethod;
@@ -377,9 +377,10 @@ mp_obj_t mp_obj_new_exception_arg1(const mp_obj_type_t *exc_type, mp_obj_t arg);
377377
mp_obj_t mp_obj_new_exception_args(const mp_obj_type_t *exc_type, uint n_args, const mp_obj_t *args);
378378
mp_obj_t mp_obj_new_exception_msg(const mp_obj_type_t *exc_type, const char *msg);
379379
mp_obj_t mp_obj_new_exception_msg_varg(const mp_obj_type_t *exc_type, const char *fmt, ...); // counts args by number of % symbols in fmt, excluding %%; can only handle void* sizes (ie no float/double!)
380-
mp_obj_t mp_obj_new_fun_bc(uint scope_flags, qstr *args, uint n_pos_args, uint n_kwonly_args, mp_obj_t def_args, mp_obj_t def_kw_args, const byte *code);
381-
mp_obj_t mp_obj_new_fun_viper(uint n_args, void *fun, mp_uint_t type_sig);
382-
mp_obj_t mp_obj_new_fun_asm(uint n_args, void *fun);
380+
mp_obj_t mp_obj_new_fun_bc(mp_uint_t scope_flags, qstr *args, mp_uint_t n_pos_args, mp_uint_t n_kwonly_args, mp_obj_t def_args, mp_obj_t def_kw_args, const byte *code);
381+
mp_obj_t mp_obj_new_fun_native(mp_uint_t n_args, void *fun_data);
382+
mp_obj_t mp_obj_new_fun_viper(mp_uint_t n_args, void *fun_data, mp_uint_t type_sig);
383+
mp_obj_t mp_obj_new_fun_asm(mp_uint_t n_args, void *fun_data);
383384
mp_obj_t mp_obj_new_gen_wrap(mp_obj_t fun);
384385
mp_obj_t mp_obj_new_closure(mp_obj_t fun, uint n_closed, const mp_obj_t *closed);
385386
mp_obj_t mp_obj_new_tuple(uint n, const mp_obj_t *items);
@@ -525,17 +526,15 @@ mp_obj_t mp_obj_new_bytearray_by_ref(uint n, void *items);
525526

526527
// functions
527528
#define MP_OBJ_FUN_ARGS_MAX (0xffff) // to set maximum value in n_args_max below
528-
typedef struct _mp_obj_fun_native_t { // need this so we can define const objects (to go in ROM)
529+
typedef struct _mp_obj_fun_builtin_t { // use this to make const objects that go in ROM
529530
mp_obj_base_t base;
530531
bool is_kw : 1;
531-
uint n_args_min : 15; // inclusive
532-
uint n_args_max : 16; // inclusive
533-
void *fun;
534-
// TODO add mp_map_t *globals
535-
// for const function objects, make an empty, const map
536-
// such functions won't be able to access the global scope, but that's probably okay
537-
} mp_obj_fun_native_t;
532+
mp_uint_t n_args_min : 15; // inclusive
533+
mp_uint_t n_args_max : 16; // inclusive
534+
void *fun; // must be a pointer to a callable function in ROM
535+
} mp_obj_fun_builtin_t;
538536

537+
mp_obj_t mp_obj_fun_binary_op(int op, mp_obj_t lhs_in, mp_obj_t rhs_in);
539538
const char *mp_obj_fun_get_name(mp_const_obj_t fun);
540539
const char *mp_obj_code_get_name(const byte *code_info);
541540

0 commit comments

Comments
 (0)