Skip to contentSkip to conversation

rust: warn on bindgen < 0.69.5 and libclang >= 19.1

PATCH Applied Miguel Ojeda 2 participants 3 messages Nov 11, 2024 – Nov 12, 2024

Subsystems (2)
lore
Share & tools
lore

Miguel Ojeda

author maintainer
#0 lore raw

PATCH v2 1/1 rust: warn on bindgen < 0.69.5 and libclang >= 19.1

When testing a clang upgrade with Rust Binder, Alice encountered [1] a build failure caused by bindgen not translating some symbols related to tracepoints. This was caused by commit 2e770edd8ce1 ("[libclang] Compute the right spelling location") changing the behavior of a function exposed by libclang. bindgen fixed the regression in commit 600f63895f73 ("Use clang_getFileLocation instead of clang_getSpellingLocation").

However, the regression fix is only available in bindgen versions 0.69.5 or later (it was backported for 0.69.x). This means that when older bindgen versions are used with new versions of libclang, bindgen may do the wrong thing, which could lead to a build failure.

Alice encountered the bug with some header files related to tracepoints, but it could also cause build failures in other circumstances. Thus, always emit a warning when using an old bindgen with a new libclang so that other people do not have to spend time chasing down the same bug.

However, testing just the version is inconvenient, since distributions do patch their packages without changing the version, so I reduced the issue into the following piece of code that can trigger the issue:

    #define F(x) int x##x
    F(foo);

In particular, an unpatched bindgen will ignore the macro expansion and thus not provide a declaration for the exported int.

Thus add a build test to rust_is_available.sh using the code above (that is only triggered if the versions appear to be affected), following what we did for the 0.66.x issue.

Moreover, I checked the status in the major distributions we have instructions for:

- Fedora 41 was affected but is now OK, since it now ships bindgen 0.69.5.

Thanks Ben for the quick reply on the updates that were ongoing.

Fedora 40 and earlier are OK (older libclang, and they also now carry bindgen 0.69.5).

- Debian Sid was affected but is now OK, since they now ship a patched bindgen binary (0.66.1-7+b3). The issue was reported to Debian by email and then as a bug report [2].

Thanks NoisyCoil and Matthias for the quick replies. NoisyCoil handled the needed updates. Debian may upgrade to bindgen 0.70.x, too.

Debian Testing is OK (older libclang so far).

- Ubuntu non-LTS (oracular) is affected. The issue was reported to Ubuntu by email and then as a bug report [3].

Ubuntu LTS is not affected (older libclang so far).

- Arch Linux, Gentoo Linux and openSUSE should be OK (newer bindgen is provided). Nix as well (older libclang so far).

This issue was also added to our "live list" that tracks issues around distributions [4].

---
We would like to put this into the merge window, or ideally very soon after as a "fix" (it is not really a fix, but it is very convenient for people wondering why their toolchain may not work, especially if tracepoints land in the merge window as expected).

v2 (based on Alice's v1):
- Fixed libclang version number (we use a different get_canonical_version that returns one more digit).
- Added build test -- now we can detect binaries like Debian's that are patched but do not change the version number.
- Added tests.
- Explained the current status of the distributions in the commit message.

 scripts/rust_is_available.sh                  | 15 ++++++++
 ...ust_is_available_bindgen_libclang_concat.h |  3 ++
 scripts/rust_is_available_test.py             | 34 ++++++++++++++++++-
 3 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 scripts/rust_is_available_bindgen_libclang_concat.h
scripts/rust_is_available.sh +15 -0
index 5262c56dd674..93c0ef7fb3fb 100755
@@ -225,6 +225,21 @@ if [ "$bindgen_libclang_cversion" -lt "$bindgen_libclang_min_cversion" ]; then
 	exit 1
 fi
 
+if [ "$bindgen_libclang_cversion" -ge 1900100 ] &&
+	[ "$rust_bindings_generator_cversion" -lt 6905 ]; then
+	# Distributions may have patched the issue (e.g. Debian did).
+	if ! "$BINDGEN" $(dirname $0)/rust_is_available_bindgen_libclang_concat.h | grep -q foofoo; then
+		echo >&2 "***"
+		echo >&2 "*** Rust bindings generator '$BINDGEN' < 0.69.5 together with libclang >= 19.1"
+		echo >&2 "*** may not work due to a bug (https://github.com/rust-lang/rust-bindgen/pull/2824),"
+		echo >&2 "*** unless patched (like Debian's)."
+		echo >&2 "***   Your bindgen version:  $rust_bindings_generator_version"
+		echo >&2 "***   Your libclang version: $bindgen_libclang_version"
+		echo >&2 "***"
+		warning=1
+	fi
+fi
+
 # If the C compiler is Clang, then we can also check whether its version
 # matches the `libclang` version used by the Rust bindings generator.
 #
scripts/rust_is_available_bindgen_libclang_concat.h new file +3 -0
new file mode 100644
index 000000000000..efc6e98d0f1d
@@ -0,0 +1,3 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#define F(x) int x##x
+F(foo);
scripts/rust_is_available_test.py +33 -1
index 413741037fb3..4fcc319dea84 100755
@@ -54,7 +54,7 @@ else:
 """)
 
     @classmethod
-    def generate_bindgen(cls, version_stdout, libclang_stderr, version_0_66_patched=False):
+    def generate_bindgen(cls, version_stdout, libclang_stderr, version_0_66_patched=False, libclang_concat_patched=False):
         if libclang_stderr is None:
             libclang_case = f"raise SystemExit({cls.bindgen_default_bindgen_libclang_failure_exit_code})"
         else:
@@ -65,12 +65,19 @@ else:
         else:
             version_0_66_case = "raise SystemExit(1)"
 
+        if libclang_concat_patched:
+            libclang_concat_case = "print('pub static mut foofoo: ::std::os::raw::c_int;')"
+        else:
+            libclang_concat_case = "pass"
+
         return cls.generate_executable(f"""#!/usr/bin/env python3
 import sys
 if "rust_is_available_bindgen_libclang.h" in " ".join(sys.argv):
     {libclang_case}
 elif "rust_is_available_bindgen_0_66.h" in " ".join(sys.argv):
     {version_0_66_case}
+elif "rust_is_available_bindgen_libclang_concat.h" in " ".join(sys.argv):
+    {libclang_concat_case}
 else:
     print({repr(version_stdout)})
 """)
@@ -268,6 +275,31 @@ else:
         result = self.run_script(self.Expected.FAILURE, { "BINDGEN": bindgen })
         self.assertIn(f"libclang (used by the Rust bindings generator '{bindgen}') is too old.", result.stderr)
 
+    def test_bindgen_bad_libclang_concat(self):
+        for (bindgen_version, libclang_version, expected_not_patched) in (
+            ("0.69.4", "18.0.0", self.Expected.SUCCESS),
+            ("0.69.4", "19.1.0", self.Expected.SUCCESS_WITH_WARNINGS),
+            ("0.69.4", "19.2.0", self.Expected.SUCCESS_WITH_WARNINGS),
+
+            ("0.69.5", "18.0.0", self.Expected.SUCCESS),
+            ("0.69.5", "19.1.0", self.Expected.SUCCESS),
+            ("0.69.5", "19.2.0", self.Expected.SUCCESS),
+
+            ("0.70.0", "18.0.0", self.Expected.SUCCESS),
+            ("0.70.0", "19.1.0", self.Expected.SUCCESS),
+            ("0.70.0", "19.2.0", self.Expected.SUCCESS),
+        ):
+            with self.subTest(bindgen_version=bindgen_version, libclang_version=libclang_version):
+                cc = self.generate_clang(f"clang version {libclang_version}")
+                libclang_stderr = f"scripts/rust_is_available_bindgen_libclang.h:2:9: warning: clang version {libclang_version} [-W#pragma-messages], err: false"
+                bindgen = self.generate_bindgen(f"bindgen {bindgen_version}", libclang_stderr)
+                result = self.run_script(expected_not_patched, { "BINDGEN": bindgen, "CC": cc })
+                if expected_not_patched == self.Expected.SUCCESS_WITH_WARNINGS:
+                    self.assertIn(f"Rust bindings generator '{bindgen}' < 0.69.5 together with libclang >= 19.1", result.stderr)
+
+                bindgen = self.generate_bindgen(f"bindgen {bindgen_version}", libclang_stderr, libclang_concat_patched=True)
+                result = self.run_script(self.Expected.SUCCESS, { "BINDGEN": bindgen, "CC": cc })
+
     def test_clang_matches_bindgen_libclang_different_bindgen(self):
         bindgen = self.generate_bindgen_libclang("scripts/rust_is_available_bindgen_libclang.h:2:9: warning: clang version 999.0.0 [-W#pragma-messages], err: false")
         result = self.run_script(self.Expected.SUCCESS_WITH_WARNINGS, { "BINDGEN": bindgen })

base-commit: ae7851c29747fa3765ecb722fe722117a346f988 --
2.47.0

#1 lore raw
Quoted 80 lines · Miguel Ojeda

When testing a clang upgrade with Rust Binder, Alice encountered [1] a build failure caused by bindgen not translating some symbols related to tracepoints. This was caused by commit 2e770edd8ce1 ("[libclang] Compute the right spelling location") changing the behavior of a function exposed by libclang. bindgen fixed the regression in commit 600f63895f73 ("Use clang_getFileLocation instead of clang_getSpellingLocation").

However, the regression fix is only available in bindgen versions 0.69.5 or later (it was backported for 0.69.x). This means that when older bindgen versions are used with new versions of libclang, bindgen may do the wrong thing, which could lead to a build failure.

Alice encountered the bug with some header files related to tracepoints, but it could also cause build failures in other circumstances. Thus, always emit a warning when using an old bindgen with a new libclang so that other people do not have to spend time chasing down the same bug.

However, testing just the version is inconvenient, since distributions do patch their packages without changing the version, so I reduced the issue into the following piece of code that can trigger the issue:

    #define F(x) int x##x
    F(foo);

In particular, an unpatched bindgen will ignore the macro expansion and thus not provide a declaration for the exported int.

Thus add a build test to rust_is_available.sh using the code above (that is only triggered if the versions appear to be affected), following what we did for the 0.66.x issue.

Moreover, I checked the status in the major distributions we have instructions for:

- Fedora 41 was affected but is now OK, since it now ships bindgen 0.69.5.

Thanks Ben for the quick reply on the updates that were ongoing.

Fedora 40 and earlier are OK (older libclang, and they also now carry bindgen 0.69.5).

- Debian Sid was affected but is now OK, since they now ship a patched bindgen binary (0.66.1-7+b3). The issue was reported to Debian by email and then as a bug report [2].

Thanks NoisyCoil and Matthias for the quick replies. NoisyCoil handled the needed updates. Debian may upgrade to bindgen 0.70.x, too.

Debian Testing is OK (older libclang so far).

- Ubuntu non-LTS (oracular) is affected. The issue was reported to Ubuntu by email and then as a bug report [3].

Ubuntu LTS is not affected (older libclang so far).

- Arch Linux, Gentoo Linux and openSUSE should be OK (newer bindgen is provided). Nix as well (older libclang so far).

This issue was also added to our "live list" that tracks issues around distributions [4].

---
We would like to put this into the merge window, or ideally very soon after as a "fix" (it is not really a fix, but it is very convenient for people wondering why their toolchain may not work, especially if tracepoints land in the merge window as expected).

Quoted 7 lines

v2 (based on Alice's v1):
- Fixed libclang version number (we use a different get_canonical_version that returns one more digit).
- Added build test -- now we can detect binaries like Debian's that are patched but do not change the version number.
- Added tests.
- Explained the current status of the distributions in the commit message.

Cc'ing Kbuild too, so that they are aware, just in case -- sorry, I should have done that in the patch itself yesterday:

    https://lore.kernel.org/rust-for-linux/[email protected]/

Cheers,
Miguel

Applied
#2 lore raw
In reply to Miguel Ojeda
Quoted 5 lines · Miguel Ojeda

We would like to put this into the merge window, or ideally very soon after as a "fix" (it is not really a fix, but it is very convenient for people wondering why their toolchain may not work, especially if tracepoints land in the merge window as expected).

Applied to rust-next (tentatively) -- thanks everyone!

I will wait a few days, and if no one objects, I will include it in the PR. Otherwise, I will remove it (it is the last commit for rust-next).

Cheers,
Miguel