[PATCH] rust: bindgen: upgrade to 0.65.1

* [PATCH] rust: bindgen: upgrade to 0.65.1
@ 2023-06-12 19:43 Aakash Sen Sharma
  2023-06-14 18:49 ` Gary Guo
  2023-08-14 22:43 ` Miguel Ojeda
  0 siblings, 2 replies; 4+ messages in thread
From: Aakash Sen Sharma @ 2023-06-12 19:43 UTC (permalink / raw)
  To: corbet, ojeda, alex.gaynor, wedsonaf, boqun.feng, gary,
	bjorn3_gh, nathan, ndesaulniers, trix, masahiroy, me,
	aakashsensharma, aliceryhl, benno.lossin, dev, lina, hca,
	rust-for-linux
  Cc: linux-doc, linux-kernel, llvm

* 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: https://github.com/llvm/llvm-project/commit/19e984ef8f49bc3ccced15621989fa9703b2cd5b [1]
Link: https://github.com/rust-lang/rust-bindgen/pull/2319 [2]
Link: https://github.com/rust-lang/rust-bindgen/pull/1990 [3]
Link: https://github.com/rust-lang/rust-bindgen/issues/2094 [4]
Link: https://github.com/rust-lang/rust-bindgen/commit/cc78b6fdb6e829e5fb8fa1639f2182cb49333569 [5]
Link: https://github.com/rust-lang/rust-bindgen/pull/2408 [6]
Closes: https://github.com/Rust-for-Linux/linux/issues/1013 [7]
Signed-off-by: Aakash Sen Sharma <aakashsensharma@gmail.com>
---
 Documentation/process/changes.rst |  2 +-
 rust/Makefile                     |  6 +++---
 rust/helpers.c                    | 13 ++++++-------
 rust/kernel/lib.rs                |  3 ---
 scripts/min-tool-version.sh       |  2 +-
 5 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst
index ef540865ad22..5f21c4c6cf5c 100644
--- a/Documentation/process/changes.rst
+++ b/Documentation/process/changes.rst
@@ -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.62.0           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
diff --git a/rust/Makefile b/rust/Makefile
index f88d108fbef0..c187c6f3a384 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -279,7 +279,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 = \
@@ -293,8 +293,8 @@ $(obj)/bindings/bindings_generated.rs: $(src)/bindings/bindings_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 = ; \
diff --git a/rust/helpers.c b/rust/helpers.c
index 121583282976..98d9ef47225b 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -122,19 +122,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`).
  */
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index ee27e10da479..1b0dcf03b9c2 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -95,7 +95,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 {}
 }
diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh
index 20d483ec6f5f..5b80c5d3a9f8 100755
--- a/scripts/min-tool-version.sh
+++ b/scripts/min-tool-version.sh
@@ -30,7 +30,7 @@ rustc)
 	echo 1.62.0
 	;;
 bindgen)
-	echo 0.56.0
+	echo 0.65.1
 	;;
 *)
 	echo "$1: unknown tool" >&2
--
2.40.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread