Skip to content

Exposing more types to native modules #5641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
jeremyherbert opened this issue Feb 13, 2020 · 6 comments
Closed

Exposing more types to native modules #5641

jeremyherbert opened this issue Feb 13, 2020 · 6 comments
Labels
py-core Relates to py/ directory in source

Comments

@jeremyherbert
Copy link
Contributor

I'm trying to expose the mp_type_int, mp_type_float and mp_type_bytes to native modules. I have done the following things:

  1. modified mp_fun_table_t in nativeglue.h to add the types:
...
    const mp_obj_type_t *type_type;
    const mp_obj_type_t *type_int;
    const mp_obj_type_t *type_float;
    const mp_obj_type_t *type_str;
    const mp_obj_type_t *type_bytes;
    const mp_obj_type_t *type_list;
    const mp_obj_type_t *type_dict;
    const mp_obj_type_t *type_fun_builtin_0;
    const mp_obj_type_t *type_fun_builtin_1;
    const mp_obj_type_t *type_fun_builtin_2;
    const mp_obj_type_t *type_fun_builtin_3;
    const mp_obj_type_t *type_fun_builtin_var;
...
  1. modified mp_fun_table in nativeglue.c to set the pointer values:
...
    &mp_type_type,
    &mp_type_int,
    &mp_type_float,
    &mp_type_str,
    &mp_type_bytes,
    &mp_type_list,
    &mp_type_dict,
    &mp_type_fun_builtin_0,
    &mp_type_fun_builtin_1,
    &mp_type_fun_builtin_2,
    &mp_type_fun_builtin_3,
    &mp_type_fun_builtin_var,
...
  1. recompiled the micropython unix port.

Unfortunately I seem to be getting an odd error where the functions are no longer hooked up correctly, I suspect that this is because there is an offset in the exported function table somewhere which has not been updated, so mp_type_fun_builtin_1 is now mapped to mp_type_list.

==26922== Memcheck, a memory error detector
==26922== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==26922== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==26922== Command: ../../../ports/unix/micropython
==26922== 
MicroPython v1.12-88-gae3fadbc3-dirty on 2020-02-13; linux version
Use Ctrl-D to exit, Ctrl-E for paste mode
>>> import ucbor
>>> ucbor.loads(b'\x01')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'list' object isn't callable
>>>

I also noticed that in mpy_ld.py there is a section which specifically resolves some other type symbols. Adjusting this to also add my new types did not solve the problem unfortunately (and I'm not getting a linker error anyway...)

Does anyone know where the table is that I am not updating properly?
@tve tve mentioned this issue Feb 13, 2020
Open
@dpgeorge
Copy link
Member

You need to make sure the changes are reflected in this table: https://github.com/micropython/micropython/blob/master/tools/mpy_ld.py#L666-L682

@dpgeorge dpgeorge added the py-core Relates to py/ directory in source label Feb 14, 2020
@jeremyherbert
Copy link
Contributor Author

Yes, I've added the symbols to that (unfortunately it doesn't fix the issue, so I think I have a problem elsewhere).

I would have thought that without them I should hit the LinkError on line 704 anyway?

@jeremyherbert
Copy link
Contributor Author

jeremyherbert commented Feb 14, 2020 •
edited
Loading

Just some more notes for me to refer to later:

after building micropython with DEBUG=1, pahole shows that the exported symbol table has the correct structure:

pahole --reorganize -C _mp_fun_table_t micropython gives

...
	mp_obj_t                   (*obj_new_str)(const char  *, size_t); /*   456     8 */
	mp_obj_t                   (*obj_new_bytes)(const byte  *, size_t); /*   464     8 */
	mp_obj_t                   (*obj_new_bytearray_by_ref)(size_t, void *); /*   472     8 */
	mp_obj_t                   (*obj_new_float_from_f)(float); /*   480     8 */
	mp_obj_t                   (*obj_new_float_from_d)(double); /*   488     8 */
	float                      (*obj_get_float_to_f)(mp_obj_t); /*   496     8 */
	double                     (*obj_get_float_to_d)(mp_obj_t); /*   504     8 */
	/* --- cacheline 8 boundary (512 bytes) --- */
	void                       (*get_buffer_raise)(mp_obj_t, mp_buffer_info_t *, mp_uint_t); /*   512     8 */
	const mp_stream_p_t  *     (*get_stream_raise)(mp_obj_t, int); /*   520     8 */
	const mp_print_t  *        plat_print;           /*   528     8 */
	const mp_obj_type_t  *     type_type;            /*   536     8 */
	const mp_obj_type_t  *     type_int;             /*   544     8 */
	const mp_obj_type_t  *     type_float;           /*   552     8 */
	const mp_obj_type_t  *     type_str;             /*   560     8 */
	const mp_obj_type_t  *     type_bytes;           /*   568     8 */
	/* --- cacheline 9 boundary (576 bytes) --- */
	const mp_obj_type_t  *     type_list;            /*   576     8 */
	const mp_obj_type_t  *     type_dict;            /*   584     8 */
	const mp_obj_type_t  *     type_fun_builtin_0;   /*   592     8 */
	const mp_obj_type_t  *     type_fun_builtin_1;   /*   600     8 */
	const mp_obj_type_t  *     type_fun_builtin_2;   /*   608     8 */
	const mp_obj_type_t  *     type_fun_builtin_3;   /*   616     8 */
	const mp_obj_type_t  *     type_fun_builtin_var; /*   624     8 */
	const mp_obj_fun_builtin_var_t  * stream_read_obj; /*   632     8 */
...

And indeed the table is still lined up with the mp_ld.py offset of 67 * 8 = mp_type_type.

@jeremyherbert
Copy link
Contributor Author

jeremyherbert commented Feb 14, 2020 •
edited
Loading

modifying mp_ld.py to print out symbol offsets shows that mp_type_fun_builtin_1 is indeed being mapped to offset 576 in the native module, rather than the correct offset of 600.

But in this process I actually just found my dumb mistake, see if you can spot it:

    mp_fun_table_sec = Section('.external.mp_fun_table', b'', 0)
    fun_table = {key: 67 + idx
        for idx, key in enumerate([
            'mp_type_type',
            'mp_type_int',
            'mp_type_float'
            'mp_type_str',
            'mp_type_bytes'
            'mp_type_list',
            'mp_type_dict',
            'mp_type_fun_builtin_0',
            'mp_type_fun_builtin_1',
            'mp_type_fun_builtin_2',
            'mp_type_fun_builtin_3',
            'mp_type_fun_builtin_var',
            'mp_stream_read_obj',
            'mp_stream_readinto_obj',
            'mp_stream_unbuffered_readline_obj',
            'mp_stream_write_obj',
        ])
    }

@dpgeorge
Copy link
Member

But in this process I actually just found my dumb mistake, see if you can spot it:

Ok, so you missed commas!

It might be a good idea to generate this table in mpy_ld.py automatically (when that script runs) by parsing py/nativeglue.h.

@jeremyherbert
Copy link
Contributor Author

Ah, didn't see this in time. I just created a PR with all of the types from obj.h.

I'll need to think a bit about parsing, because from memory pycparsing doesn't like macros and micropython obviously uses them a lot.

@tve tve mentioned this issue Feb 17, 2020
Closed
tannewt added a commit to tannewt/circuitpython that referenced this issue Dec 2, 2021
@tannewt
Merge pull request micropython#5641 from jepler/bitbangio-spi-write-s…
39cc38e 
…lice

bitbangio.SPI: Handle kwargs like busio.SPI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
py-core Relates to py/ directory in source
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jeremyherbert @dpgeorge