libs: Add static compatiblity check for Rust

[no1wudi]

Summary

This patch adds a static compatiblity check for most of structures from NuttX, which is used by Rust.

Currently, for most of them have a reserved field, which is for possible future use, you can refer to https://github.com/no1wudi/libc/blob/39277b5357cb2aa7af1bd8344956ec292c35abed/src/unix/nuttx/mod.rs#L71 for more details.

I think we should define the initial version carefully, to avoid compatiblity problems in future as much as possible:

1. Struct reserved size is 2 pointer size, is it sufficient?
2. Which option should we preferred to use? Such as SYSTEM_TIME64 and FS_LARGEFILE?
3. Should we enable it by default to cover more cases in build CI? It can prevent introduce compatiblity issues.

Please feel free to comment

[anchao]

Why not generate structure code dynamically, but bind statically?

[no1wudi]

Why not generate structure code dynamically, but bind statically?

Please see https://github.com/apache/nuttx/issues/12960, generate structure code dynamically is OK on technical, but binary compatibility cannot be guaranteed.

Thus the libc/libstd of Rust can not be built without NuttX develop enviroment, if you want to use them, you must use the nighlty toolchain of Rust instead of stable toolchain.

Consider in Rust, the direct use of structs from libc is not very common, so there is an opportunity to implement binary compatibility for these structs.

[anchao]

Why not generate structure code dynamically, but bind statically?

Please see #12960, generate structure code dynamically is OK on technical, but binary compatibility cannot be guaranteed.

Thus the libc/libstd of Rust can not be built without NuttX develop enviroment, if you want to use them, you must use the nighlty toolchain of Rust instead of stable toolchain.

Consider in Rust, the direct use of structs from libc is not very common, so there is an opportunity to implement binary compatibility for these structs.

libc/libstd definitely needs the participation of nuttx to be compiled, similar to the WAMR glue code we did before, which allows rust to be bound to a certain configuration to avoid doing these meaningless work. It is a common practice for different nuttx header files to generate different lib/libstd.

[xiaoxiang781216]

I'm voting no on this PR unless there are other RTOS that use similar alternative practices.

Why not support both approach?

• NuttX is compliant with POSIX, so it could take the integration approach used by Linux and *BSD as https://github.com/apache/nuttx/pull/13245 and https://github.com/apache/nuttx/issues/12960.
• Or RTOS approach as https://github.com/apache/nuttx/pull/12862 and https://github.com/apache/nuttx-apps/pull/2462

[lupyuen]

I'm voting no on this PR unless there are other RTOS that use similar alternative practices.

Hi @anchao are we thinking of the bindgen approach? As discussed, it's potentially possible, but it will require special tools because we don't want to recompile libstd for every NuttX Platform. Unlike other OSes (like Linux), I don't think we can afford the effort to create these special tools.

[no1wudi]

libc/libstd definitely needs the participation of nuttx to be compiled, similar to the WAMR glue code we did before, which allows rust to be bound to a certain configuration to avoid doing these meaningless work. It is a common practice for different nuttx header files to generate different lib/libstd.

@anchao You can refer to the libc crate for other supported platforms, for example TEEOS: https://github.com/rust-lang/libc/blob/0e6afd534ed7a0406e9dfc9242c2c9370a5b8d05/src/teeos/mod.rs#L106-L109 and newlib based ROTS: https://github.com/rust-lang/libc/blob/0e6afd534ed7a0406e9dfc9242c2c9370a5b8d05/src/unix/newlib/mod.rs#L240-L272

For NuttX, reuse the unix port is better.

Only a few data structures need to be defined on the Rust side, and they should be kept in sync with the native definitions on the NuttX side, which are usually related to libc and POSIX/pthread. For function definitions, there should be no doubt; follow the standard definitions to proceed.

For other NuttX private interfaces, such as various peripheral drivers and tasks, the bindgen method can be used for support. These additional packages are not within the scope of libstd and should be provided through additional crates.

BTW：In fact, to make bindgen work out of the box requires support from libstd. If we want to use bindgen for libstd/libc itself, we need to hack it like: https://github.com/lvgl/lv_binding_rust mentions.

[lupyuen]

Hi @acassis do you think we should proceed with this approach, to verify the NuttX Struct Sizes? This is for creating the initial version of Rust Standard Library for NuttX. Thanks!

[xiaoxiang781216]

@no1wudi could we check the struct layout by parsing symbol from image's elf file, which is less intrusive and more accurate?

[no1wudi]

@no1wudi could we check the struct layout by parsing symbol from image's elf file, which is less intrusive and more accurate?

That need NuttX to build with debug symbol enabled and add a post action after linking. But, why parsing symbol from elf file is more accurate than this approach?

[acassis]

Hi @acassis do you think we should proceed with this approach, to verify the NuttX Struct Sizes? This is for creating the initial version of Rust Standard Library for NuttX. Thanks!

@no1wudi @lupyuen maybe we should have an option to let the user test it even when we don't have the guarantees of Struct Sizes case he/she enables EXPERIMENTAL, what to you think?

Something like:

	depends on (SYSTEM_TIME64 && FS_LARGEFILE && !SMALL_MEMORY) || EXPERIMENTAL

[no1wudi]

@no1wudi @lupyuen maybe we should have an option to let the user test it even when we don't have the guarantees of Struct Sizes case he/she enables EXPERIMENTAL, what to you think?

Something like:

	depends on (SYSTEM_TIME64 && FS_LARGEFILE && !SMALL_MEMORY) || EXPERIMENTAL

@acassis Do the test for more case is good idea but if the essential kconfig is missing, the test will fail and affects normal usage with EXPERIMENTAL enabled.

Or we use a python script to do the check with elf file after build.

[lupyuen]

@no1wudi I agree with Alan:

1. Rust Standard Library on NuttX will need lots of testing by the NuttX Community. So it's good to release the Initial Version early, get feedback, improve and iterate.
2. EXPERIMENTAL is good to have because it encourages more folks to try out. And to propose fixes if it breaks.
3. About the Python Script checking with ELF File: Maybe we can do this in the next release? I think it's better to release the first version early, without too many features. Thanks!

[no1wudi]

@lupyuen @acassis OK, indeed, a significant amount of work is still required before it stabilizes. I would like to clarify that the issue regarding the compatibility of struct definitions will take a longer period to address. Firstly, if NuttX introduces any breaking changes, these changes will first be integrated into libc.

Then, we will wait for the libc crate to release a new version, followed by upgrading the Rust libstd's dependency on libc to the latest version. The entire cycle could potentially take several months.

Currently, I would like to draw everyone's attention to the fact that, in order to reduce the likelihood of such occurrences, my current approach is to reserve space equivalent to two pointer sizes for each structure definition on the Rust side to accommodate updates on the NuttX side:

1. Once this PR is merged, it means that any modifications to these structures will be restricted, especially when adding new structure members.
2. Is the reservation of space equivalent to two pointer sizes appropriate?

Regarding question 1, I personally believe that the structures currently under review are all included in the POSIX definition. POSIX defines the minimum set of members within these structures and allows OS implementations to extend them. Therefore, if modifications to these definitions are necessary, especially when adding internal data required by NuttX's implementation, special handling for them should be acceptable. For example, with struct tm: https://github.com/apache/nuttx/blob/32801d3047174ef30cea759d0ec0dcd31a077bca/include/time.h#L139

tm_gmtoff is not in POSIX: https://pubs.opengroup.org/onlinepubs/9699919799.orig/basedefs/time.h.html

If in the future we need to add other similar non-standard members that cause the reserved space to be insufficient, then we can wrap these members into an internal NuttX structure and point to this internal structure from struct tm using a pointer.

If it is OK, I'll add EXPERIMENTAL to Kconfig, and enable the static check for QEMU based ARM32/ARM64/RISCV32/RISCV64 in some defconfig to catch breaking changes for the struct.

[no1wudi]

The CI report error if EXPERIMENTAL enabled:

====================================================================================
Cmake in present: qemu-armv8a/sotest
Configuration/Tool: qemu-armv8a/sotest
2024-09-02 14:37:07
------------------------------------------------------------------------------------
  Cleaning...
  Configuring...
 Select HOST_LINUX=y
 Select HOST_X86_64=y
  Building NuttX...
[1/334] cd /github/workspace/sources/nuttx/build/libs/libc/misc && /usr/local/bin/cmake -E touch /github/workspace/sources/nuttx/libs/libc/misc/lib_utsname.c
[2/334] Building C object libs/librust/CMakeFiles/rust.dir/lib_rust.c.obj
FAILED: libs/librust/CMakeFiles/rust.dir/lib_rust.c.obj 
/tools/ccache/bin/aarch64-none-elf-gcc -D__NuttX__ -isystem /github/workspace/sources/nuttx/include -isystem /github/workspace/sources/nuttx/build/include -isystem /github/workspace/sources/nuttx/build/include_arch -march=armv8-a -mcpu=cortex-a53 -Os -fno-strict-aliasing -fomit-frame-pointer -D_LDBL_EQ_DBL -fno-common -Wall -Wshadow -Wundef -Wno-attributes -Wno-unknown-pragmas -Werror -Wstrict-prototypes -Wno-psabi -ffunction-sections -fdata-sections -g -fdiagnostics-color=always -fmacro-prefix-map=/github/workspace/sources/nuttx= -fmacro-prefix-map=/github/workspace/sources/apps= -fmacro-prefix-map=/github/workspace/sources/nuttx/boards/arm64/qemu/qemu-armv8a= -fmacro-prefix-map=/github/workspace/sources/nuttx/arch/arm64/src/qemu= -MD -MT libs/librust/CMakeFiles/rust.dir/lib_rust.c.obj -MF libs/librust/CMakeFiles/rust.dir/lib_rust.c.obj.d -o libs/librust/CMakeFiles/rust.dir/lib_rust.c.obj -c /github/workspace/sources/nuttx/libs/librust/lib_rust.c
In file included from /github/workspace/sources/nuttx/libs/librust/lib_rust.c:27:
Error: /github/workspace/sources/nuttx/libs/librust/lib_rust.c:139:1: error: static assertion failed: "blkcnt_t size mismatch with Rust"
  139 | static_assert(sizeof(blkcnt_t) == 8, "blkcnt_t size mismatch with Rust");
      | ^~~~~~~~~~~~~
Error: /github/workspace/sources/nuttx/libs/librust/lib_rust.c:144:1: error: static assertion failed: "fsblkcnt_t size mismatch with Rust"
  144 | static_assert(sizeof(fsblkcnt_t) == 8, "fsblkcnt_t size mismatch with Rust");
      | ^~~~~~~~~~~~~
Error: /github/workspace/sources/nuttx/libs/librust/lib_rust.c:149:1: error: static assertion failed: "off_t size mismatch with Rust"
  149 | static_assert(sizeof(off_t) == 8, "off_t size mismatch with Rust");
      | ^~~~~~~~~~~~~
Error: /github/workspace/sources/nuttx/libs/librust/lib_rust.c:157:1: error: static assertion failed: "rlim_t size mismatch with Rust"
  157 | static_assert(sizeof(rlim_t) == 8, "rlim_t size mismatch with Rust");
      | ^~~~~~~~~~~~~

Should we enable the essential options for them in defconfig?

1. TIME64 should be OK for QEMU, especially for the coming 2038 issue
2. FS_LARGEFILE should be OK for QEMU, it's useful for complex product with many on device files from our experience
3. !SMALL_MEMORY, QEMU has nearly unlimit memory

[lupyuen]

@no1wudi Sorry I didn't realise the implications. Could we remove EXPERIMENTAL for now, and add it to the next version? So we don't hold up the release of librust. Thanks!

[no1wudi]

Let's wait feedback from https://github.com/rust-lang/libc/pull/3909 before merge this PR.

[github-actions[bot]]

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

[yamt]

does this mean to require everyone here, who might not care rust at all, to maintain certain abi compatibility for rust? i'm not sure if it's a good idea. if rust doesn't work well w/o os-level abi stability, it's a problem in rust, not nuttx.

[no1wudi]

does this mean to require everyone here, who might not care rust at all, to maintain certain abi compatibility for rust? i'm not sure if it's a good idea. if rust doesn't work well w/o os-level abi stability, it's a problem in rust, not nuttx.

Not just for Rust, it's a common issue for NuttX now, image that you have a prebuilt .a library that referenced to NuttX header, and once the structure or config changed in NuttX side, you must re-compile the static library from source.

Maybe by this approach, not only Rust but also can provide relative stable binary interface for this usage.

[yamt]

does this mean to require everyone here, who might not care rust at all, to maintain certain abi compatibility for rust? i'm not sure if it's a good idea. if rust doesn't work well w/o os-level abi stability, it's a problem in rust, not nuttx.

Not just for Rust, it's a common issue for NuttX now, image that you have a prebuilt .a library that referenced to NuttX header, and once the structure or config changed in NuttX side, you must re-compile the static library from source.

Maybe by this approach, not only Rust but also can provide relative stable binary interface for this usage.

my impression is that "recompile everything on configuration changes" isn't a big problem for many of users of nuttx. users tend to prefer better optimizations (smaller structures, lto, etc) over abi stability.

as it's a big policy change with pros and cons, i suspect we should discuss this topic with a wider audience.

[github-actions[bot]]

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

[no1wudi]

as it's a big policy change with pros and cons, i suspect we should discuss this topic with a wider audience.

Agree, so I open an issue to collect the idea from NuttX community: https://github.com/apache/nuttx/issues/12960

[lupyuen]

Hi @no1wudi could you share on the Mailing List the current approach for Rust Standard Library? And explain the tradeoff between Better Optimizations (smaller structures, lto, etc) vs ABI Stability? I think we need the community to agree on this. Thanks!

[no1wudi]

Hi @no1wudi could you share on the Mailing List the current approach for Rust Standard Library? And explain the tradeoff between Better Optimizations (smaller structures, lto, etc) vs ABI Stability? I think we need the community to agree on this. Thanks!

OK

[no1wudi]

@yamt @lupyuen The current approach is not to enforce a mandatory requirement that we cannot modify these structures or the values of macro definitions, but rather to strengthen the review of their modifications. If possible, we should try not to modify them. However, if these modifications are indeed necessary, we should make the changes in conjunction with the Rust side.

[no1wudi]

Make it ready to review since https://github.com/rust-lang/libc/pull/3909 merged

[github-actions[bot]]

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

[xiaoxiang781216]

should we move the code to apps(e.g. rustcheck)? since all of them is checking the size and offset of posix type.