Add bindgen build zephyr-sys #76337

d3zd3z · 2024-07-26T15:00:17Z

With a lot of help and inspiration from @mjaun, generate a zephyr-sys crate using bindgen.

This currently exposes Zpehyr functions starting with k_ and with gpio_, although the code currently only uses k_str_out for the printk/printkln implementation.

The bindgen tool generates a wrapper C file that has wrappers for functions declared as static inline in the Zephyr headers. Although there will be a bit of an efficiency hit, this gives us a stable way of accessing the Zephyr interfaces. If there are any that would be better handled directly in Rust, they can be done by hand, and bindgen instructed to skip them.

teburd · 2024-07-26T15:21:19Z

It's nice you got this working, it's hard to see how manually wrapping every header we have with custom bindings would have worked out. I'm interested to see how this looks in practice.

include/zephyr/spinlock.h

include/zephyr/kernel/thread.h

mjaun · 2024-07-30T13:52:45Z

lib/rust/zephyr-sys/build.rs

+        // <inttypes.h> seems to come from somewhere mysterious in Zephyr.  For us, just pull in the
+        // one from the minimal libc.
+        .clang_arg("-DRUST_BINDGEN")
+        .clang_arg("-I../../../lib/libc/minimal/include")


I think this makes things much simpler compared to my solution. Just to double check: Can we consider it safe for the binding generation if the libc header files are not the ones actually used by the build?

It depends a little bit on what types are being used. I would suspect that for all of the possible libc implementations basic types will be the same. In fact, there are a bunch of size checks in kernel.h that will fail if this is not the case.

mjaun · 2024-07-30T14:03:14Z

lib/rust/zephyr-sys/build.rs

+        // <inttypes.h> seems to come from somewhere mysterious in Zephyr.  For us, just pull in the
+        // one from the minimal libc.


The default configuration in Zephyr has CONFIG_ENFORCE_ZEPHYR_STDINT active which adds -imacros${ZEPHYR_BASE}/include/zephyr/toolchain/zephyr_stdint.h. Do we need to consider this as well?

I can look at pulling in this file instead of one from libc.

mjaun · 2024-07-30T14:08:03Z

cmake/modules/rust.cmake

+  get_include_dirs(zephyr_interface include_dirs)
+
+  get_target_property(include_dirs, zephyr_interface INTERFACE_INCLUDE_DIRECTORIES)
+  get_property(include_defines TARGET zephyr_interface PROPERTY INTERFACE_COMPILE_DEFINITIONS)


Any reason not to use the Zephyr functions here?

zephyr_get_system_include_directories_for_lang(C system_includes) zephyr_get_include_directories_for_lang(C includes) zephyr_get_compile_definitions_for_lang(C definitions)

I looked at them, but they were formatted for C. I might still be able to parse them, so I will look at it.

cmake/modules/rust.cmake

Change the header passed into bindgen to be an absolute path. This will cause the generated wrapper to refer to this file also using an absolute path. As such, remove the explicit include path added as part of the build. Signed-off-by: David Brown <david.brown@linaro.org>

Add documentation on the bindings between Rust and C, and the bindgen tool used to generate them. Signed-off-by: David Brown <david.brown@linaro.org>

Re-export all of the bindgen generated bindings in the zephyr-sys crate into `zephyr::raw`. This keeps things easier, as users of `zephyr` only need to worry about the single `zephyr` crate. Signed-off-by: David Brown <david.brown@linaro.org>

Fix `WRAPPER_FiLE` to `WRAPPER_FILE`. Signed-off-by: David Brown <david.brown@linaro.org>

Zephyr takes advantage of a gcc/clang extension that allows structs that have no elements. Rust is perfectly happy with this (it is quite common in Rust code), but generates a warning when a struct containing no elements is passed to C code. For now, suppress this warning on the generated bindings. This has the disadvantage of suppressing it entirely, which might possibly detect other cases of invalid structs. However, the bindings are auto-generated from C structs so should always be valid. Signed-off-by: David Brown <david.brown@linaro.org>

This reverts commit 2046760. Put these back so we get the zero element structures when using just rust and not CPP. A subsequent patch will suppress the warning. Signed-off-by: David Brown <david.brown@linaro.org>

GCC automatically defines a `__SOFTFP__` define on targets that are using software floating point. The clang compiler does not do this by default, so check this, and define it. Signed-off-by: David Brown <david.brown@linaro.org>

Rustc for RISCV encodes optional features on the CPU available as part of the target tuple. Clang, on the other hand does not. In order to be able to use libclang with bindgen on RISCV, we need to simplify the target tuples a bit. Do this by just matching 'riscv32' or 'riscv64' and then passing in a generic tuple. We aren't generating any code, and the structs should always match between the targets. Signed-off-by: David Brown <david.brown@linaro.org>

Although the Cmake rules to build Rust applications keeps the target directory inside of the build directory, some IDE tools may generate a target directory while editing the code. Ignore these so they never get checked in. Signed-off-by: David Brown <david.brown@linaro.org>

cfriedt · 2024-08-29T00:19:49Z

I would guess the mps2/an521 errors might be due to stack size.

With bindgen needing to read the headers, make sure CMake knows about this. Signed-off-by: David Brown <david.brown@linaro.org>

d3zd3z · 2024-08-29T23:15:05Z

cmake/modules/rust.cmake

  add_custom_target(librustapp ALL
    DEPENDS ${DUMMY_FILE}
+        # The variables, defined at the top level, don't seem to be accessible here.


Does anyone know how to do this right? I'd like to use the ${SYSCALL_LIST_H_TARGET} instead of its expansion, but that variable doesn't seem to be defined at this point in this module.

rust.cmake is executed first, defining the rust_cargo_application() function. At this point SYSCALL_LIST_H_TARGET is not defined yet:

Then comes the Zephyr CMakeLists.txt defining SYSCALL_LIST_H_TARGET:

Finally the rust_cargo_application() function is executed, but the variables from Zephyr CMakeLists.txt are alreadyout of scope as can be seen with the stack trace:

I don't see how this could be done without modifying something elsewhere. Would it maybe be reasonable to just add zephyr_interface as a dependency?

I'll try adding zephyr_interface as a dependency, and see if it works. It's a little frustrating because the failure are somewhat non-deterministic, although it does seem to fail pretty consistently in CI.

So zephyr_interface doesn't really work as a dependency, as it seems to just hold the flags needed to set include paths for the zephyr interfaces. It doesn't actually have any header files in it. It seems that the generated targest are what is needed to have the dependencies work.

Have you tried with add_dependencies(librustapp zephyr_interface)? I think the DEPENDS parameter only works for files, not for targets.

kartben

some really minor comments -- note that my review concerns mostly the doc aspects of the PR

kartben · 2024-09-05T19:27:38Z

doc/develop/languages/rust/bindings.rst

+Bindings to Zephyr for Rust
+###########################
+
+Zephyr is written in C, and it's primary API is also made available to C.  It is written as a


Suggested change

Zephyr is written in C, and it's primary API is also made available to C. It is written as a

Zephyr is written in C, and its primary API is also made available to C. It is written as a

kartben · 2024-09-05T19:28:40Z

cmake/modules/rust.cmake

@@ -142,14 +168,19 @@ ${config_paths}
    WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
  )

+  # Be sure we don't try building this until al of the generated headers have been generated.


Suggested change

# Be sure we don't try building this until al of the generated headers have been generated.

# Be sure we don't try building this until all of the generated headers have been generated.

kartben · 2024-09-05T19:29:06Z

doc/develop/languages/rust/bindings.rst

+Using the Bindings
+******************
+
+In general, using direct bindings to C function from Rust is a bit more difficult than calling them


Suggested change

In general, using direct bindings to C function from Rust is a bit more difficult than calling them

In general, using direct bindings to C functions from Rust is a bit more difficult than calling them

kartben · 2024-09-05T19:29:25Z

doc/develop/languages/rust/bindings.rst

+
+In general, using direct bindings to C function from Rust is a bit more difficult than calling them
+from C.  Although all of these calls are considered "unsafe", and must be placed in an ``unsafe``
+block, the rust language has stricter constraints on what is allowed, even by unsafe code.  Although


Suggested change

block, the rust language has stricter constraints on what is allowed, even by unsafe code. Although

block, the Rust language has stricter constraints on what is allowed, even by unsafe code. Although

kartben · 2024-09-05T19:31:47Z

doc/develop/languages/rust/bindings.rst

+block, the rust language has stricter constraints on what is allowed, even by unsafe code.  Although
+the intent of the Rust on Zephyr project is to allow full use of Zephyr, without needing to resort


I don't necessarily dislike the idea of calling this "Rust on Zephyr", but probably better to say something like "... supporting the Rust language in Zephyr ..." or something.

Also, you have two consecutive sentences starting with "Although" :)

kartben · 2024-09-05T19:35:12Z

cmake/modules/rust.cmake

 function(rust_cargo_application)
  # For now, hard-code the Zephyr crate directly here.  Once we have
  # more than one crate, these should be added by the modules
  # themselves.
  set(LIB_RUST_CRATES zephyr zephyr-build)

+  get_include_dirs(zephyr_interface include_dirs)
+
+  get_target_property(include_dirs, zephyr_interface INTERFACE_INCLUDE_DIRECTORIES)


this can be dropped, I think? That's what get_include_dirs already does (L53)?

kartben · 2024-09-05T19:35:59Z

doc/develop/languages/rust/bindings.rst

+
+Because the Zephyr headers use numerous conditional compilation macros, the bindings needed will be
+very specialized to a given board, and even a given configuration.  To do this, the file
+:file:`lib/rust/zephyr-sys/build.rs`, which ``cargo`` knows to run at build time will


Suggested change

:file:`lib/rust/zephyr-sys/build.rs`, which ``cargo`` knows to run at build time will

:file:`lib/rust/zephyr-sys/build.rs`, which ``cargo`` knows to run at build time, will

Minor fixes to the documentation from review feedback. Signed-off-by: David Brown <david.brown@linaro.org>

Remove a redundant call that does exactly what the previous function call accomplishes. Signed-off-by: David Brown <david.brown@linaro.org>

Fix a mispelled word "al" -> "all". Signed-off-by: David Brown <david.brown@linaro.org>

d3zd3z · 2024-09-06T15:15:43Z

I'm going to go ahead an merge this, and we can focus on reviews of the cmake changes as a whole before merging the branch.

d3zd3z added DNM This PR should not be merged (Do Not Merge) area: Rust Issues affecting Rust support in Zephyr labels Jul 26, 2024

d3zd3z requested review from tejlmand, stephanosio and nordicjm July 26, 2024 15:00

zephyrbot added area: Documentation area: Kernel area: Samples Samples area: Process area: Build System labels Jul 26, 2024

zephyrbot requested review from 57300, andyross, carlescufi, ceolin, cfriedt, dcpleung, jeremybettis, kartben, nashif, npitre, peter-mitsis and TaiJuWu July 26, 2024 15:01

zephyrbot assigned kartben Jul 26, 2024

d3zd3z force-pushed the r/bindgen branch from 24ff3a6 to d0e08f9 Compare July 26, 2024 15:05

andyross reviewed Jul 26, 2024

View reviewed changes

include/zephyr/spinlock.h Outdated Show resolved Hide resolved

andyross reviewed Jul 26, 2024

View reviewed changes

include/zephyr/kernel/thread.h Outdated Show resolved Hide resolved

d3zd3z mentioned this pull request Jul 26, 2024

Add devicetree support to Rust on Zephyr #76074

Open

mjaun reviewed Jul 30, 2024

View reviewed changes

lupyuen mentioned this pull request Aug 19, 2024

[HELP] FFI compatiblity issues apache/nuttx#12960

Open

1 task

zephyrbot added area: Build System area: Kernel labels Aug 27, 2024

d3zd3z added 2 commits August 27, 2024 13:59

doc: rust: Add docs on bindings

2c3432a

Add documentation on the bindings between Rust and C, and the bindgen tool used to generate them. Signed-off-by: David Brown <david.brown@linaro.org>

rust: Export zephyr-sys as zephyr::raw

f26c50e

Re-export all of the bindgen generated bindings in the zephyr-sys crate into `zephyr::raw`. This keeps things easier, as users of `zephyr` only need to worry about the single `zephyr` crate. Signed-off-by: David Brown <david.brown@linaro.org>

zephyrbot added the area: Documentation label Aug 27, 2024

d3zd3z added 3 commits August 28, 2024 09:15

cmake: rust: Fix typo on variable name

fd2061b

Fix `WRAPPER_FiLE` to `WRAPPER_FILE`. Signed-off-by: David Brown <david.brown@linaro.org>

Revert "include: Add dummy field for Rust as well as CPP"

90b90c2

This reverts commit 2046760. Put these back so we get the zero element structures when using just rust and not CPP. A subsequent patch will suppress the warning. Signed-off-by: David Brown <david.brown@linaro.org>

d3zd3z force-pushed the r/bindgen branch from bedffce to 90b90c2 Compare August 28, 2024 15:15

andyross previously approved these changes Aug 28, 2024

View reviewed changes

d3zd3z dismissed andyross’s stale review via 7468f20 August 28, 2024 21:17

d3zd3z added 3 commits August 28, 2024 15:33

rust: Workaround gcc/clang differences with soft fp

23dad4d

GCC automatically defines a `__SOFTFP__` define on targets that are using software floating point. The clang compiler does not do this by default, so check this, and define it. Signed-off-by: David Brown <david.brown@linaro.org>

d3zd3z force-pushed the r/bindgen branch from 7468f20 to 7a95c12 Compare August 28, 2024 21:34

cmake: rust: Add header dependencies to cargo

eb68555

With bindgen needing to read the headers, make sure CMake knows about this. Signed-off-by: David Brown <david.brown@linaro.org>

d3zd3z force-pushed the r/bindgen branch from f60080e to eb68555 Compare August 29, 2024 21:11

d3zd3z commented Aug 29, 2024

View reviewed changes

teburd previously approved these changes Sep 5, 2024

View reviewed changes

kartben requested changes Sep 5, 2024

View reviewed changes

d3zd3z added 3 commits September 5, 2024 14:20

doc: develop: rust: Various cleanups to docs

6cd9a68

Minor fixes to the documentation from review feedback. Signed-off-by: David Brown <david.brown@linaro.org>

cmake: rust: Remove redundant call

2966c8f

Remove a redundant call that does exactly what the previous function call accomplishes. Signed-off-by: David Brown <david.brown@linaro.org>

cmake: rust: Comment typo fix

7a8dbb1

Fix a mispelled word "al" -> "all". Signed-off-by: David Brown <david.brown@linaro.org>

d3zd3z dismissed teburd’s stale review via 7a8dbb1 September 5, 2024 20:22

kartben approved these changes Sep 6, 2024

View reviewed changes

teburd approved these changes Sep 6, 2024

View reviewed changes

d3zd3z merged commit f67ff24 into zephyrproject-rtos:collab-rust Sep 6, 2024
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add bindgen build zephyr-sys #76337

Add bindgen build zephyr-sys #76337

d3zd3z commented Jul 26, 2024

teburd commented Jul 26, 2024

mjaun Jul 30, 2024

d3zd3z Aug 27, 2024

mjaun Jul 30, 2024

d3zd3z Aug 27, 2024

mjaun Jul 30, 2024

d3zd3z Aug 27, 2024

cfriedt commented Aug 29, 2024

d3zd3z Aug 29, 2024

mjaun Aug 30, 2024

d3zd3z Sep 4, 2024

d3zd3z Sep 5, 2024

mjaun Sep 6, 2024

kartben left a comment

kartben Sep 5, 2024

kartben Sep 5, 2024

kartben Sep 5, 2024

kartben Sep 5, 2024

kartben Sep 5, 2024

kartben Sep 5, 2024

kartben Sep 5, 2024

d3zd3z commented Sep 6, 2024

		// <inttypes.h> seems to come from somewhere mysterious in Zephyr. For us, just pull in the
		// one from the minimal libc.

	Zephyr is written in C, and it's primary API is also made available to C. It is written as a
	Zephyr is written in C, and its primary API is also made available to C. It is written as a

	# Be sure we don't try building this until al of the generated headers have been generated.
	# Be sure we don't try building this until all of the generated headers have been generated.

	In general, using direct bindings to C function from Rust is a bit more difficult than calling them
	In general, using direct bindings to C functions from Rust is a bit more difficult than calling them

	block, the rust language has stricter constraints on what is allowed, even by unsafe code. Although
	block, the Rust language has stricter constraints on what is allowed, even by unsafe code. Although

		block, the rust language has stricter constraints on what is allowed, even by unsafe code. Although
		the intent of the Rust on Zephyr project is to allow full use of Zephyr, without needing to resort

	:file:`lib/rust/zephyr-sys/build.rs`, which ``cargo`` knows to run at build time will
	:file:`lib/rust/zephyr-sys/build.rs`, which ``cargo`` knows to run at build time, will

Add bindgen build zephyr-sys #76337

Add bindgen build zephyr-sys #76337

Conversation

d3zd3z commented Jul 26, 2024

teburd commented Jul 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfriedt commented Aug 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kartben left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d3zd3z commented Sep 6, 2024