py/dynruntime.h: Implement MP_OBJ_NEW_QSTR.

Because mpy_ld.py doesn't know the target object representation, it emits
instances of `MP_OBJ_NEW_QSTR(MP_QSTR_Foo)` as const string objects, rather
than qstrs. However this doesn't work for map keys (e.g. for a locals dict)
because the map has all_keys_are_qstrs flag is set (and also auto-complete
requires the map keys to be qstrs).

Instead, emit them as regular qstrs, and make a functioning MP_OBJ_NEW_QSTR
function available (via `native_to_obj`, also used for e.g. making
integers).

Remove the code from mpy_ld.py to emit qstrs as constant strings, but leave
behind the scaffold to emit constant objects in case we want to do use this
in the future.

Strictly this should be a .mpy sub-version bump, even though the function
table isn't changing, it does lead to a change in behavior for a new .mpy
running against old MicroPython. `mp_native_to_obj` will incorrectly return
the qstr value directly as an `mp_obj_t`, leading to unexpected results.
But given that it's broken at the moment, it seems unlikely that anyone is
relying on this, so it's not work the other downsides of a sub-version bump
(i.e. breaking pure-Python modules that use @native). The opposite case of
running an old .mpy on new MicroPython is unchanged, and remains broken in
exactly the same way.

This work was funded through GitHub Sponsors.

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>

py/dynruntime.h

@@ -79,7 +79,7 @@ static inline void *m_realloc_dyn(void *ptr, size_t new_num_bytes) {
 /******************************************************************************/
 // Types and objects
 
-#define MP_OBJ_NEW_QSTR(x) MP_OBJ_NEW_QSTR_##x
+#define MP_OBJ_NEW_QSTR(x)                  (mp_fun_table.native_to_obj(x, MP_NATIVE_TYPE_QSTR))
 
 #define mp_type_type                        (*mp_fun_table.type_type)
 #define mp_type_NoneType                    (*mp_obj_get_type(mp_const_none))

py/nativeglue.c

@@ -104,6 +104,8 @@ mp_obj_t mp_native_to_obj(mp_uint_t val, mp_uint_t type) {
             return mp_obj_new_int(val);
         case MP_NATIVE_TYPE_UINT:
             return mp_obj_new_int_from_uint(val);
+        case MP_NATIVE_TYPE_QSTR:
+            return MP_OBJ_NEW_QSTR(val);
         default: // a pointer
             // we return just the value of the pointer as an integer
             return mp_obj_new_int_from_uint(val);

py/runtime0.h

@@ -50,6 +50,9 @@
 #define MP_NATIVE_TYPE_PTR16 (0x06)
 #define MP_NATIVE_TYPE_PTR32 (0x07)
 
+// Not use for viper, but for dynamic native modules
+#define MP_NATIVE_TYPE_QSTR (0x08)
+
 // Bytecode and runtime boundaries for unary ops
 #define MP_UNARY_OP_NUM_BYTECODE    (MP_UNARY_OP_NOT + 1)
 #define MP_UNARY_OP_NUM_RUNTIME     (MP_UNARY_OP_SIZEOF + 1)

tests/micropython/import_mpy_native_gc.py

@@ -47,6 +47,10 @@ class UserFS:
 
 # Pre-compiled examples/natmod/features0 example for various architectures, keyed
 # by the required value of sys.implementation._mpy (without sub-version).
+# cd examples/natmod/features0
+# make clean
+# make ARCH=x64 # or ARCH=armv6m
+# cat features0.mpy | python -c 'import sys; print(sys.stdin.buffer.read())'
 features0_file_contents = {
     # -march=x64
     0x806: b'M\x06\x09\x1f\x02\x004build/features0.native.mpy\x00\x12factorial\x00\x8a\x02\xe9/\x00\x00\x00SH\x8b\x1d\x83\x00\x00\x00\xbe\x02\x00\x00\x00\xffS\x18\xbf\x01\x00\x00\x00H\x85\xc0u\x0cH\x8bC \xbe\x02\x00\x00\x00[\xff\xe0H\x0f\xaf\xf8H\xff\xc8\xeb\xe6ATUSH\x8b\x1dQ\x00\x00\x00H\x8bG\x08L\x8bc(H\x8bx\x08A\xff\xd4H\x8d5+\x00\x00\x00H\x89\xc5H\x8b\x059\x00\x00\x00\x0f\xb7x\x02\xffShH\x89\xefA\xff\xd4H\x8b\x03[]A\\\xc3\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x05\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x10\x11$\r&\xa3 \x01"\xff',

tools/mpy_ld.py

@@ -232,34 +232,20 @@ def extract_qstrs(source_files):
     def read_qstrs(f):
         with open(f) as f:
             vals = set()
-            objs = set()
             for line in f:
-                while line:
+                for m in re.finditer(r"MP_QSTR_[A-Za-z0-9_]*", line):
-                    m = re.search(r"MP_OBJ_NEW_QSTR\((MP_QSTR_[A-Za-z0-9_]*)\)", line)
+                    vals.add(m.group())
-                    if m:
+            return vals
-                        objs.add(m.group(1))
-                    else:
-                        m = re.search(r"MP_QSTR_[A-Za-z0-9_]*", line)
-                        if m:
-                            vals.add(m.group())
-                    if m:
-                        s = m.span()
-                        line = line[: s[0]] + line[s[1] :]
-                    else:
-                        line = ""
-            return vals, objs
 
     static_qstrs = ["MP_QSTR_" + qstrutil.qstr_escape(q) for q in qstrutil.static_qstr_list]
 
     qstr_vals = set()
-    qstr_objs = set()
     for f in source_files:
-        vals, objs = read_qstrs(f)
+        vals = read_qstrs(f)
         qstr_vals.update(vals)
-        qstr_objs.update(objs)
     qstr_vals.difference_update(static_qstrs)
 
-    return static_qstrs, qstr_vals, qstr_objs
+    return static_qstrs, qstr_vals
 
 
 ################################################################################
@@ -730,7 +716,7 @@ def load_object_file(env, felf):
                 env.unresolved_syms.append(sym)
 
 
-def link_objects(env, native_qstr_vals_len, native_qstr_objs_len):
+def link_objects(env, native_qstr_vals_len):
     # Build GOT information
     if env.arch.name == "EM_XTENSA":
         build_got_xtensa(env)
@@ -761,7 +747,7 @@ def link_objects(env, native_qstr_vals_len, native_qstr_objs_len):
     # Create section to contain mp_native_obj_table
     env.obj_table_section = Section(
         ".external.obj_table",
-        bytearray(native_qstr_objs_len * env.arch.word_size),
+        bytearray(0 * env.arch.word_size),  # currently empty
         env.arch.word_size,
     )
 
@@ -899,7 +885,7 @@ class MPYOutput:
             self.write_uint(n)
 
 
-def build_mpy(env, entry_offset, fmpy, native_qstr_vals, native_qstr_objs):
+def build_mpy(env, entry_offset, fmpy, native_qstr_vals):
     # Write jump instruction to start of text
     jump = env.arch.asm_jump(entry_offset)
     env.full_text[: len(jump)] = jump
@@ -927,7 +913,7 @@ def build_mpy(env, entry_offset, fmpy, native_qstr_vals, native_qstr_objs):
     out.write_uint(1 + len(native_qstr_vals))
 
     # MPY: n_obj
-    out.write_uint(len(native_qstr_objs))
+    out.write_uint(0)
 
     # MPY: qstr table
     out.write_qstr(fmpy)  # filename
@@ -935,10 +921,7 @@ def build_mpy(env, entry_offset, fmpy, native_qstr_vals, native_qstr_objs):
         out.write_qstr(q)
 
     # MPY: object table
-    for q in native_qstr_objs:
+    # <empty>
-        out.write_bytes(bytearray([MP_PERSISTENT_OBJ_STR]))
-        out.write_uint(len(q))
-        out.write_bytes(bytes(q, "utf8") + b"\x00")
 
     # MPY: kind/len
     out.write_uint(len(env.full_text) << 3 | (MP_CODE_NATIVE_VIPER - MP_CODE_BYTECODE))
@@ -965,6 +948,7 @@ def build_mpy(env, entry_offset, fmpy, native_qstr_vals, native_qstr_objs):
         out.write_bytes(env.full_rodata)
 
     # MPY: relocation information
+    # See py/persistentcode.c:mp_native_relocate for meaning of the `kind` integer values.
     prev_kind = None
     prev_base = None
     prev_offset = None
@@ -1016,7 +1000,7 @@ def do_preprocess(args):
     if args.output is None:
         assert args.files[0].endswith(".c")
         args.output = args.files[0][:-1] + "config.h"
-    static_qstrs, qstr_vals, qstr_objs = extract_qstrs(args.files)
+    static_qstrs, qstr_vals = extract_qstrs(args.files)
     with open(args.output, "w") as f:
         print(
             "#include <stdint.h>\n"
@@ -1029,11 +1013,6 @@ def do_preprocess(args):
             print("#define %s (%u)" % (q, i + 1), file=f)
         for i, q in enumerate(sorted(qstr_vals)):
             print("#define %s (mp_native_qstr_table[%d])" % (q, i + 1), file=f)
-        for i, q in enumerate(sorted(qstr_objs)):
-            print(
-                "#define MP_OBJ_NEW_QSTR_%s ((mp_obj_t)mp_native_obj_table[%d])" % (q, i),
-                file=f,
-            )
         print("extern const uint16_t mp_native_qstr_table[];", file=f)
         print("extern const mp_uint_t mp_native_obj_table[];", file=f)
 
@@ -1043,25 +1022,19 @@ def do_link(args):
         assert args.files[0].endswith(".o")
         args.output = args.files[0][:-1] + "mpy"
     native_qstr_vals = []
-    native_qstr_objs = []
     if args.qstrs is not None:
         with open(args.qstrs) as f:
             for l in f:
                 m = re.match(r"#define MP_QSTR_([A-Za-z0-9_]*) \(mp_native_", l)
                 if m:
                     native_qstr_vals.append(m.group(1))
-                else:
-                    m = re.match(r"#define MP_OBJ_NEW_QSTR_MP_QSTR_([A-Za-z0-9_]*)", l)
-                    if m:
-                        native_qstr_objs.append(m.group(1))
     log(LOG_LEVEL_2, "qstr vals: " + ", ".join(native_qstr_vals))
-    log(LOG_LEVEL_2, "qstr objs: " + ", ".join(native_qstr_objs))
     env = LinkEnv(args.arch)
     try:
         for file in args.files:
             load_object_file(env, file)
-        link_objects(env, len(native_qstr_vals), len(native_qstr_objs))
+        link_objects(env, len(native_qstr_vals))
-        build_mpy(env, env.find_addr("mpy_init"), args.output, native_qstr_vals, native_qstr_objs)
+        build_mpy(env, env.find_addr("mpy_init"), args.output, native_qstr_vals)
     except LinkError as er:
         print("LinkError:", er.args[0])
         sys.exit(1)