Skip to content

Commit

Permalink
rust: bindgen: upgrade to 0.65.1
Browse files Browse the repository at this point in the history
* Rationale:

Upgrades bindgen to code-generation for anonymous unions, structs, and enums [7]
for LLVM-16 based toolchains.

The following upgrade also incorporates `noreturn` support from bindgen
allowing us to remove useless `loop` calls which was placed as a
workaround.

* Truncated build logs on using bindgen `v0.56.0` prior to LLVM-16 toolchain:

```
$ make rustdoc LLVM=1 CLIPPY=1 -j$(nproc)
  RUSTC L rust/core.o
  BINDGEN rust/bindings/bindings_generated.rs
  BINDGEN rust/bindings/bindings_helpers_generated.rs
  BINDGEN rust/uapi/uapi_generated.rs
thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at__/_/include/linux/compiler_types_h_146_2)" is not a valid Ident', .../proc-macro2-1.0.24/src/fallback.rs:693:9
...
thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at__/_/include/linux/compiler_types_h_146_2)" is not a valid Ident', .../proc-macro2-1.0.24/src/fallback.rs:693:9
...
```

* LLVM-16 Changes:

API changes [1] were introduced such that libclang would emit names like
"(unnamed union at compiler_types.h:146:2)" for unnamed unions, structs, and
enums whereas it previously returned an empty string.

* Bindgen Changes:

Bindgen `v0.56.0` on LLVM-16 based toolchains hence was unable to process the
anonymous union in `include/linux/compiler_types` `struct ftrace_branch_data`
and caused subsequent panics as the new `libclang` API emitted name was not
being handled. The following issue was fixed in Bindgen `v0.62.0` [2].

Bindgen `v0.58.0` changed the flags `--whitelist-*` and `--blacklist-*`
to `--allowlist-*` and `--blocklist-*` respectively [3].

Bindgen `v0.61.0` added support for `_Noreturn`, `[[noreturn]]`, `__attribute__((noreturn))` [4],
hence the empty `loop`s used to circumvent bindgen returning `!` in place of `()`
for noreturn attributes have been removed completely.

Bindgen `v0.61.0` also changed default functionality to bind `size_t` to `usize` [5] and
added the `--no-size_t-is-usize` [5] flag to not bind `size_t` as `usize`.

Bindgen `v0.65.0` removed `--size_t-is-usize` flag [6].

Link: llvm/llvm-project@19e984e [1]
Link: rust-lang/rust-bindgen#2319 [2]
Link: rust-lang/rust-bindgen#1990 [3]
Link: rust-lang/rust-bindgen#2094 [4]
Link: rust-lang/rust-bindgen@cc78b6f [5]
Link: rust-lang/rust-bindgen#2408 [6]
Reported-by: Josh Poimboeuf <jpoimboe@kernel.org>
Closes: https://lore.kernel.org/rust-for-linux/20230606215212.r7if2gsynajugf6j@treble/
Reported-by: Aakash Sen Sharma <aakashsensharma@gmail.com>
Closes: Rust-for-Linux#1013 [7]
Signed-off-by: Aakash Sen Sharma <aakashsensharma@gmail.com>
Link: https://lore.kernel.org/r/20230612194311.24826-1-aakashsensharma@gmail.com
[ Reword, add Josh's report and update Quick Start guide. ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
  • Loading branch information
Shinyzenith authored and ojeda committed Jun 12, 2023
1 parent d09a610 commit 698129c
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 16 deletions.
2 changes: 1 addition & 1 deletion Documentation/process/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ you probably needn't concern yourself with pcmciautils.
GNU C 5.1 gcc --version
Clang/LLVM (optional) 11.0.0 clang --version
Rust (optional) 1.68.2 rustc --version
bindgen (optional) 0.56.0 bindgen --version
bindgen (optional) 0.65.1 bindgen --version
GNU make 3.82 make --version
bash 4.2 bash --version
binutils 2.25 ld -v
Expand Down
2 changes: 1 addition & 1 deletion Documentation/rust/quick-start.rst
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ the ``bindgen`` tool. A particular version is required.

Install it via (note that this will download and build the tool from source)::

cargo install --locked --version $(scripts/min-tool-version.sh bindgen) bindgen
cargo install --locked --version $(scripts/min-tool-version.sh bindgen) bindgen-cli


Requirements: Developing
Expand Down
6 changes: 3 additions & 3 deletions rust/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ quiet_cmd_bindgen = BINDGEN $@
$(BINDGEN) $< $(bindgen_target_flags) \
--use-core --with-derive-default --ctypes-prefix core::ffi --no-layout-tests \
--no-debug '.*' \
--size_t-is-usize -o $@ -- $(bindgen_c_flags_final) -DMODULE \
-o $@ -- $(bindgen_c_flags_final) -DMODULE \
$(bindgen_target_cflags) $(bindgen_target_extra)

$(obj)/bindings/bindings_generated.rs: private bindgen_target_flags = \
Expand All @@ -320,8 +320,8 @@ $(obj)/uapi/uapi_generated.rs: $(src)/uapi/uapi_helper.h \
# given it is `libclang`; but for consistency, future Clang changes and/or
# a potential future GCC backend for `bindgen`, we disable it too.
$(obj)/bindings/bindings_helpers_generated.rs: private bindgen_target_flags = \
--blacklist-type '.*' --whitelist-var '' \
--whitelist-function 'rust_helper_.*'
--blocklist-type '.*' --allowlist-var '' \
--allowlist-function 'rust_helper_.*'
$(obj)/bindings/bindings_helpers_generated.rs: private bindgen_target_cflags = \
-I$(objtree)/$(obj) -Wno-missing-prototypes -Wno-missing-declarations
$(obj)/bindings/bindings_helpers_generated.rs: private bindgen_target_extra = ; \
Expand Down
13 changes: 6 additions & 7 deletions rust/helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,19 +129,18 @@ void rust_helper_put_task_struct(struct task_struct *t)
EXPORT_SYMBOL_GPL(rust_helper_put_task_struct);

/*
* We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
* as the Rust `usize` type, so we can use it in contexts where Rust
* expects a `usize` like slice (array) indices. `usize` is defined to be
* the same as C's `uintptr_t` type (can hold any pointer) but not
* necessarily the same as `size_t` (can hold the size of any single
* `bindgen` binds the C `size_t` type the Rust `usize` type, so we can
* use it in contexts where Rust expects a `usize` like slice (array) indices.
* `usize` is defined to be the same as C's `uintptr_t` type (can hold any pointer)
* but not necessarily the same as `size_t` (can hold the size of any single
* object). Most modern platforms use the same concrete integer type for
* both of them, but in case we find ourselves on a platform where
* that's not true, fail early instead of risking ABI or
* integer-overflow issues.
*
* If your platform fails this assertion, it means that you are in
* danger of integer-overflow bugs (even if you attempt to remove
* `--size_t-is-usize`). It may be easiest to change the kernel ABI on
* danger of integer-overflow bugs (even if you attempt to add
* `--no-size_t-is-usize`). It may be easiest to change the kernel ABI on
* your platform such that `size_t` matches `uintptr_t` (i.e., to increase
* `size_t`, because `uintptr_t` has to be at least as big as `size_t`).
*/
Expand Down
3 changes: 0 additions & 3 deletions rust/kernel/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,4 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
pr_emerg!("{}\n", info);
// SAFETY: FFI call.
unsafe { bindings::BUG() };
// Bindgen currently does not recognize `__noreturn` so `BUG` returns `()`
// instead of `!`. See <https://github.com/rust-lang/rust-bindgen/issues/2094>.
loop {}
}
2 changes: 1 addition & 1 deletion scripts/min-tool-version.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ rustc)
echo 1.68.2
;;
bindgen)
echo 0.56.0
echo 0.65.1
;;
*)
echo "$1: unknown tool" >&2
Expand Down

0 comments on commit 698129c

Please sign in to comment.