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 micropython#820.
xavierlesa · Aug 24, 2014 · 3c658a4 · 3c658a4
1 parent 25fc41d
commit 3c658a4
Show file tree

Hide file tree

Showing 22 changed files with 208 additions and 121 deletions.
diff --git a/bare-arm/mpconfigport.h b/bare-arm/mpconfigport.h
@@ -35,6 +35,8 @@
 
 #define BYTES_PER_WORD (4)
 
+#define MICROPY_MAKE_POINTER_CALLABLE(p) ((void*)((mp_uint_t)(p) | 1))
+
 #define UINT_FMT "%lu"
 #define INT_FMT "%ld"
 
@@ -44,7 +46,7 @@ typedef void *machine_ptr_t; // must be of pointer size
 typedef const void *machine_const_ptr_t; // must be of pointer size
 
 // extra built in names to add to the global namespace
-extern const struct _mp_obj_fun_native_t mp_builtin_open_obj;
+extern const struct _mp_obj_fun_builtin_t mp_builtin_open_obj;
 #define MICROPY_PORT_BUILTINS \
     { MP_OBJ_NEW_QSTR(MP_QSTR_open), (mp_obj_t)&mp_builtin_open_obj },
 

diff --git a/py/asmthumb.c b/py/asmthumb.c
@@ -132,8 +132,7 @@ uint asm_thumb_get_code_size(asm_thumb_t *as) {
 }
 
 void *asm_thumb_get_code(asm_thumb_t *as) {
-    // need to set low bit to indicate that it's thumb code
-    return (void *)(((mp_uint_t)as->code_base) | 1);
+    return as->code_base;
 }
 
 /*

diff --git a/py/bc.h b/py/bc.h
@@ -50,7 +50,7 @@ typedef struct _mp_code_state {
 } mp_code_state;
 
 mp_vm_return_kind_t mp_execute_bytecode(mp_code_state *code_state, volatile mp_obj_t inject_exc);
-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);
+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);
 void mp_bytecode_print(const void *descr, const byte *code, int len);
 void mp_bytecode_print2(const byte *code, int len);
 

diff --git a/py/emitglue.c b/py/emitglue.c
@@ -55,7 +55,7 @@ mp_raw_code_t *mp_emit_glue_new_raw_code(void) {
     return rc;
 }
 
-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) {
+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) {
     rc->kind = MP_CODE_BYTECODE;
     rc->scope_flags = scope_flags;
     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
     rc->u_byte.len = len;
 
 #ifdef DEBUG_PRINT
-    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);
+    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);
     DEBUG_printf("  arg names:");
     for (int i = 0; i < n_pos_args + n_kwonly_args; i++) {
         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
 #endif
 #if MICROPY_DEBUG_PRINTERS
     if (mp_verbose_flag > 0) {
-        for (int i = 0; i < 128 && i < len; i++) {
+        for (mp_uint_t i = 0; i < len; i++) {
             if (i > 0 && i % 16 == 0) {
                 printf("\n");
             }
@@ -87,22 +87,21 @@ void mp_emit_glue_assign_bytecode(mp_raw_code_t *rc, byte *code, uint len, uint
 }
 
 #if MICROPY_EMIT_NATIVE || MICROPY_EMIT_INLINE_THUMB
-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) {
+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) {
     assert(kind == MP_CODE_NATIVE_PY || kind == MP_CODE_NATIVE_VIPER || kind == MP_CODE_NATIVE_ASM);
     rc->kind = kind;
     rc->scope_flags = 0;
     rc->n_pos_args = n_args;
-    rc->u_native.fun = fun;
+    rc->u_native.fun_data = fun_data;
     rc->u_native.type_sig = type_sig;
 
 #ifdef DEBUG_PRINT
-    DEBUG_printf("assign native: kind=%d fun=%p len=%u n_args=%d\n", kind, fun, len, n_args);
-    byte *fun_data = (byte*)(((mp_uint_t)fun) & (~1)); // need to clear lower bit in case it's thumb code
-    for (int i = 0; i < 128 && i < len; i++) {
+    DEBUG_printf("assign native: kind=%d fun=%p len=" UINT_FMT " n_args=" UINT_FMT "\n", kind, fun_data, fun_len, n_args);
+    for (mp_uint_t i = 0; i < fun_len; i++) {
         if (i > 0 && i % 16 == 0) {
             DEBUG_printf("\n");
         }
-        DEBUG_printf(" %02x", fun_data[i]);
+        DEBUG_printf(" %02x", ((byte*)fun_data)[i]);
     }
     DEBUG_printf("\n");
 
@@ -133,15 +132,15 @@ mp_obj_t mp_make_function_from_raw_code(mp_raw_code_t *rc, mp_obj_t def_args, mp
             break;
         #if MICROPY_EMIT_NATIVE
         case MP_CODE_NATIVE_PY:
-            fun = mp_make_function_n(rc->n_pos_args, rc->u_native.fun);
+            fun = mp_obj_new_fun_native(rc->n_pos_args, rc->u_native.fun_data);
             break;
         case MP_CODE_NATIVE_VIPER:
-            fun = mp_obj_new_fun_viper(rc->n_pos_args, rc->u_native.fun, rc->u_native.type_sig);
+            fun = mp_obj_new_fun_viper(rc->n_pos_args, rc->u_native.fun_data, rc->u_native.type_sig);
             break;
         #endif
         #if MICROPY_EMIT_INLINE_THUMB
         case MP_CODE_NATIVE_ASM:
-            fun = mp_obj_new_fun_asm(rc->n_pos_args, rc->u_native.fun);
+            fun = mp_obj_new_fun_asm(rc->n_pos_args, rc->u_native.fun_data);
             break;
         #endif
         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
     return fun;
 }
 
-mp_obj_t mp_make_closure_from_raw_code(mp_raw_code_t *rc, uint n_closed_over, const mp_obj_t *args) {
-    DEBUG_OP_printf("make_closure_from_raw_code %p %u %p\n", rc, n_closed_over, args);
+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) {
+    DEBUG_OP_printf("make_closure_from_raw_code %p " UINT_FMT " %p\n", rc, n_closed_over, args);
     // make function object
     mp_obj_t ffun;
     if (n_closed_over & 0x100) {

diff --git a/py/emitglue.h b/py/emitglue.h
@@ -37,26 +37,26 @@ typedef enum {
 
 typedef struct _mp_code_t {
     mp_raw_code_kind_t kind : 3;
-    uint scope_flags : 7;
-    uint n_pos_args : 11;
-    uint n_kwonly_args : 11;
+    mp_uint_t scope_flags : 7;
+    mp_uint_t n_pos_args : 11;
+    mp_uint_t n_kwonly_args : 11;
     qstr *arg_names;
     union {
         struct {
             byte *code;
-            uint len;
+            mp_uint_t len;
         } u_byte;
         struct {
-            void *fun;
+            void *fun_data;
             mp_uint_t type_sig; // for viper, compressed as 2-bit types; ret is MSB, then arg0, arg1, etc
         } u_native;
     };
 } mp_raw_code_t;
 
 mp_raw_code_t *mp_emit_glue_new_raw_code(void);
 
-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);
-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);
+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);
+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);
 
 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);
-mp_obj_t mp_make_closure_from_raw_code(mp_raw_code_t *rc, uint n_closed_over, const mp_obj_t *args);
+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);
diff --git a/py/mpconfig.h b/py/mpconfig.h
@@ -429,6 +429,12 @@ typedef double mp_float_t;
 #define MP_ENDIANNESS_LITTLE (0)
 #endif
 
+// Make a pointer to RAM callable (eg set lower bit for Thumb code)
+// (This scheme won't work if we want to mix Thumb and normal ARM code.)
+#ifndef MICROPY_MAKE_POINTER_CALLABLE
+#define MICROPY_MAKE_POINTER_CALLABLE(p) (p)
+#endif
+
 // printf format spec to use for mp_int_t and friends
 #ifndef INT_FMT
 #ifdef __LP64__

diff --git a/py/obj.c b/py/obj.c
@@ -161,7 +161,7 @@ mp_int_t mp_obj_hash(mp_obj_t o_in) {
         return mp_obj_str_get_hash(o_in);
     } else if (MP_OBJ_IS_TYPE(o_in, &mp_type_NoneType)) {
         return (mp_int_t)o_in;
-    } else if (MP_OBJ_IS_TYPE(o_in, &mp_type_fun_native) || MP_OBJ_IS_TYPE(o_in, &mp_type_fun_bc)) {
+    } else if (MP_OBJ_IS_FUN(o_in)) {
         return (mp_int_t)o_in;
     } else if (MP_OBJ_IS_TYPE(o_in, &mp_type_tuple)) {
         return mp_obj_tuple_hash(o_in);

diff --git a/py/obj.h b/py/obj.h
@@ -71,9 +71,10 @@ typedef struct _mp_obj_base_t mp_obj_base_t;
 //#define MP_OBJ_IS_SMALL_INT(o) ((((mp_int_t)(o)) & 1) != 0)
 //#define MP_OBJ_IS_QSTR(o) ((((mp_int_t)(o)) & 3) == 2)
 //#define MP_OBJ_IS_OBJ(o) ((((mp_int_t)(o)) & 3) == 0)
-#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
+#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
 #define MP_OBJ_IS_INT(o) (MP_OBJ_IS_SMALL_INT(o) || MP_OBJ_IS_TYPE(o, &mp_type_int))
 #define MP_OBJ_IS_STR(o) (MP_OBJ_IS_QSTR(o) || MP_OBJ_IS_TYPE(o, &mp_type_str))
+#define MP_OBJ_IS_FUN(o) (MP_OBJ_IS_OBJ(o) && (((mp_obj_base_t*)(o))->type->binary_op == mp_obj_fun_binary_op))
 
 #define MP_OBJ_SMALL_INT_VALUE(o) (((mp_int_t)(o)) >> 1)
 #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;
 // These macros are used to declare and define constant function objects
 // You can put "static" in front of the definitions to make them local
 
-#define MP_DECLARE_CONST_FUN_OBJ(obj_name) extern const mp_obj_fun_native_t obj_name
+#define MP_DECLARE_CONST_FUN_OBJ(obj_name) extern const mp_obj_fun_builtin_t obj_name
 
-#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}
+#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}
 #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)
 #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)
 #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);
 typedef mp_obj_t (*mp_fun_1_t)(mp_obj_t);
 typedef mp_obj_t (*mp_fun_2_t)(mp_obj_t, mp_obj_t);
 typedef mp_obj_t (*mp_fun_3_t)(mp_obj_t, mp_obj_t, mp_obj_t);
-typedef mp_obj_t (*mp_fun_t)(void);
 typedef mp_obj_t (*mp_fun_var_t)(uint n, const mp_obj_t *);
 typedef mp_obj_t (*mp_fun_kw_t)(uint n, const mp_obj_t *, mp_map_t *);
 
@@ -304,7 +304,7 @@ extern const mp_obj_type_t mp_type_zip;
 extern const mp_obj_type_t mp_type_array;
 extern const mp_obj_type_t mp_type_super;
 extern const mp_obj_type_t mp_type_gen_instance;
-extern const mp_obj_type_t mp_type_fun_native;
+extern const mp_obj_type_t mp_type_fun_builtin;
 extern const mp_obj_type_t mp_type_fun_bc;
 extern const mp_obj_type_t mp_type_module;
 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);
 mp_obj_t mp_obj_new_exception_args(const mp_obj_type_t *exc_type, uint n_args, const mp_obj_t *args);
 mp_obj_t mp_obj_new_exception_msg(const mp_obj_type_t *exc_type, const char *msg);
 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!)
-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);
-mp_obj_t mp_obj_new_fun_viper(uint n_args, void *fun, mp_uint_t type_sig);
-mp_obj_t mp_obj_new_fun_asm(uint n_args, void *fun);
+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);
+mp_obj_t mp_obj_new_fun_native(mp_uint_t n_args, void *fun_data);
+mp_obj_t mp_obj_new_fun_viper(mp_uint_t n_args, void *fun_data, mp_uint_t type_sig);
+mp_obj_t mp_obj_new_fun_asm(mp_uint_t n_args, void *fun_data);
 mp_obj_t mp_obj_new_gen_wrap(mp_obj_t fun);
 mp_obj_t mp_obj_new_closure(mp_obj_t fun, uint n_closed, const mp_obj_t *closed);
 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);
 
 // functions
 #define MP_OBJ_FUN_ARGS_MAX (0xffff) // to set maximum value in n_args_max below
-typedef struct _mp_obj_fun_native_t { // need this so we can define const objects (to go in ROM)
+typedef struct _mp_obj_fun_builtin_t { // use this to make const objects that go in ROM
     mp_obj_base_t base;
     bool is_kw : 1;
-    uint n_args_min : 15; // inclusive
-    uint n_args_max : 16; // inclusive
-    void *fun;
-    // TODO add mp_map_t *globals
-    // for const function objects, make an empty, const map
-    // such functions won't be able to access the global scope, but that's probably okay
-} mp_obj_fun_native_t;
+    mp_uint_t n_args_min : 15; // inclusive
+    mp_uint_t n_args_max : 16; // inclusive
+    void *fun; // must be a pointer to a callable function in ROM
+} mp_obj_fun_builtin_t;
 
+mp_obj_t mp_obj_fun_binary_op(int op, mp_obj_t lhs_in, mp_obj_t rhs_in);
 const char *mp_obj_fun_get_name(mp_const_obj_t fun);
 const char *mp_obj_code_get_name(const byte *code_info);