Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

codegen: Generate CStr only if possible. #2567

Merged
merged 1 commit into from
Jun 26, 2023

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Jun 26, 2023

And fix a crash when strings have interior nulls.

Fixes #2566

And fix a crash when strings have interior nulls.

Fixes rust-lang#2566
@emilio
Copy link
Contributor Author

emilio commented Jun 26, 2023

r? @reitermarkus @pvdrz

@emilio emilio merged commit 441bc7b into rust-lang:main Jun 26, 2023
@emilio emilio deleted the cstr-bytes-internal-nul branch June 26, 2023 16:42
ojeda added a commit to ojeda/rust-bindgen that referenced this pull request Jun 18, 2024
Rust for Linux, so far, has pinned the Rust compiler and `bindgen`
versions. The kernel is looking into expanding that support to several
versions, i.e. establishing a minimum supported version, so that the
kernel can start to be more easily built. In particular, it should be
possible to build the kernel using the tools provided directly by Linux
distributions.

In order to help achieve that goal, the Rust project has added the kernel
to its Rust pre-merge CI. This commit does the same for `bindgen`.

In particular, it adds a quick, build-only test of the Rust code in
the kernel done as an added, last step in the `test` workflow. As it is
implemented, it adds about 1-2 minutes of CI time.

This is intended to be an end-to-end test that runs what kernel
developers/users would do. In particular, it is useful to catch certain
issues that go beyond the C header comparisons. For instance, it would
have been able to catch an issue like the `--version` option unexpectedly
requiring a header in 0.69.0 (fixed in 0.69.1) [1].

It would also have detected another issue present in 0.66.0 and 0.66.1:
a panic handling certain C headers with string literals containing
an interior NUL [2]. While the kernel is not really a stable test,
and such an issue would still require that a proper test is added,
it is nevertheless a good test case of non-trivial C headers that may
trigger edge cases like that.

Of course, `bindgen` may need to disable the test for different reasons,
i.e. there is no expectation to block any urgent/important PR, and the
kernel can also call `bindgen` differently depending on the version,
i.e. we are happy to adjust on our side too. Even if it gets disabled
often, we would still be in a better situation than not having the test
at all.

The Linux version (hash or tag) should ideally be updated from time to
time (e.g. every kernel `-rc1`), and each update should only contain
that change.

Link: rust-lang#2567 [1]
Link: rust-lang#2678 [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to ojeda/rust-bindgen that referenced this pull request Jun 18, 2024
Rust for Linux, so far, has pinned the Rust compiler and `bindgen`
versions. The kernel is looking into expanding that support to several
versions, i.e. establishing a minimum supported version, so that the
kernel can start to be more easily built. In particular, it should be
possible to build the kernel using the tools provided directly by Linux
distributions. In order to help achieve that goal, the Rust project has
added the kernel to its Rust pre-merge CI.

This commit does the same for `bindgen`. In particular, it adds a quick,
build-only test of the Rust code in the kernel as an extra step in the
`test` workflow.

This is intended to be an end-to-end test that runs what kernel
developers/users would do. In particular, it is useful to catch certain
issues that go beyond the C header comparisons. For instance, it would
have been able to catch an issue like the `--version` option unexpectedly
requiring a header in 0.69.0 (fixed in 0.69.1) [1].

It would also have detected another issue present in 0.66.0 and 0.66.1:
a panic handling certain C headers with string literals containing
an interior NUL [2]. While the kernel is not really a stable test,
and such an issue would still require that a proper test is added,
it is nevertheless a good test case of non-trivial C headers that may
trigger edge cases like that.

Of course, `bindgen` may need to disable the test for different reasons,
i.e. there is no expectation to block any urgent/important PR, and the
kernel can also call `bindgen` differently depending on the version,
i.e. we are happy to adjust on our side too. Even if it gets disabled
often, we would still be in a better situation than not having the test
at all.

The Linux version (hash or tag) should ideally be updated from time to
time (e.g. every kernel `-rc1`), and each update should only contain
that change.

Link: rust-lang#2567 [1]
Link: rust-lang#2678 [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to ojeda/rust-bindgen that referenced this pull request Jun 18, 2024
Rust for Linux, so far, has pinned the Rust compiler and `bindgen`
versions. The kernel is looking into expanding that support to several
versions, i.e. establishing a minimum supported version, so that the
kernel can start to be more easily built. In particular, it should be
possible to build the kernel using the tools provided directly by Linux
distributions. In order to help achieve that goal, the Rust project has
added the kernel to its Rust pre-merge CI.

This commit does the same for `bindgen`. In particular, it adds a quick,
build-only test of the Rust code in the kernel as an extra step in the
`test` workflow.

This is intended to be an end-to-end test that runs what kernel
developers/users would do. In particular, it is useful to catch certain
issues that go beyond the C header comparisons. For instance, it would
have been able to catch an issue like the `--version` option unexpectedly
requiring a header in 0.69.0 (fixed in 0.69.1) [1].

It would also have detected another issue present in 0.66.0 and 0.66.1:
a panic handling certain C headers with string literals containing
an interior NUL [2]. While the kernel is not really a stable test,
and such an issue would still require that a proper test is added,
it is nevertheless a good test case of non-trivial C headers that may
trigger edge cases like that.

Of course, `bindgen` may need to disable the test for different reasons,
i.e. there is no expectation to block any urgent/important PR, and the
kernel can also call `bindgen` differently depending on the version,
i.e. we are happy to adjust on our side too. Even if it gets disabled
often, we would still be in a better situation than not having the test
at all.

The Linux version (hash or tag) should ideally be updated from time to
time (e.g. every kernel `-rc1`), and each update should only contain
that change.

Link: rust-lang#2567 [1]
Link: rust-lang#2678 [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to ojeda/rust-bindgen that referenced this pull request Jun 18, 2024
Rust for Linux, so far, has pinned the Rust compiler and `bindgen`
versions. The kernel is looking into expanding that support to several
versions, i.e. establishing a minimum supported version, so that the
kernel can start to be more easily built. In particular, it should be
possible to build the kernel using the tools provided directly by Linux
distributions. In order to help achieve that goal, the Rust project has
added the kernel to its Rust pre-merge CI.

This commit does the same for `bindgen`. In particular, it adds a quick,
build-only test of the Rust code in the kernel as an extra step in the
`test` workflow.

This is intended to be an end-to-end test that runs what kernel
developers/users would do. In particular, it is useful to catch certain
issues that go beyond the C header comparisons. For instance, it would
have been able to catch an issue like the `--version` option unexpectedly
requiring a header in 0.69.0 (fixed in 0.69.1) [1].

It would also have detected another issue present in 0.66.0 and 0.66.1:
a panic handling certain C headers with string literals containing
an interior NUL [2]. While the kernel is not really a stable test,
and such an issue would still require that a proper test is added,
it is nevertheless a good test case of non-trivial C headers that may
trigger edge cases like that.

Of course, `bindgen` may need to disable the test for different reasons,
i.e. there is no expectation to block any urgent/important PR, and the
kernel can also call `bindgen` differently depending on the version,
i.e. we are happy to adjust on our side too. Even if it gets disabled
often, we would still be in a better situation than not having the test
at all.

The Linux version (hash or tag) should ideally be updated from time to
time (e.g. every kernel `-rc1`), and each update should only contain
that change.

Link: rust-lang#2567 [1]
Link: rust-lang#2678 [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to ojeda/rust-bindgen that referenced this pull request Jun 18, 2024
Rust for Linux, so far, has pinned the Rust compiler and `bindgen`
versions. The kernel is looking into expanding that support to several
versions, i.e. establishing a minimum supported version, so that the
kernel can start to be more easily built. In particular, it should be
possible to build the kernel using the tools provided directly by Linux
distributions. In order to help achieve that goal, the Rust project has
added the kernel to its Rust pre-merge CI.

This commit does the same for `bindgen`. In particular, it adds a quick,
build-only test of the Rust code in the kernel as an extra step in the
`test` workflow.

This is intended to be an end-to-end test that runs what kernel
developers/users would do. In particular, it is useful to catch certain
issues that go beyond the C header comparisons. For instance, it would
have been able to catch an issue like the `--version` option unexpectedly
requiring a header in 0.69.0 (fixed in 0.69.1) [1].

It would also have detected another issue present in 0.66.0 and 0.66.1:
a panic handling certain C headers with string literals containing
an interior NUL [2]. While the kernel is not really a stable test,
and such an issue would still require that a proper test is added,
it is nevertheless a good test case of non-trivial C headers that may
trigger edge cases like that.

Of course, `bindgen` may need to disable the test for different reasons,
i.e. there is no expectation to block any urgent/important PR, and the
kernel can also call `bindgen` differently depending on the version,
i.e. we are happy to adjust on our side too. Even if it gets disabled
often, we would still be in a better situation than not having the test
at all.

The Linux version (hash or tag) should ideally be updated from time to
time (e.g. every kernel `-rc1`), and each update should only contain
that change.

Link: rust-lang#2678 [1]
Link: rust-lang#2567 [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
@ojeda ojeda mentioned this pull request Jun 18, 2024
ojeda added a commit to ojeda/rust-bindgen that referenced this pull request Jun 19, 2024
Rust for Linux, so far, has pinned the Rust compiler and `bindgen`
versions. The kernel is looking into expanding that support to several
versions, i.e. establishing a minimum supported version, so that the
kernel can start to be more easily built. In particular, it should be
possible to build the kernel using the tools provided directly by Linux
distributions. In order to help achieve that goal, the Rust project has
added the kernel to its Rust pre-merge CI.

This commit does the same for `bindgen`. In particular, it adds a quick,
build-only test of the Rust code in the kernel as an extra step in the
`test` workflow.

This is intended to be an end-to-end test that runs what kernel
developers/users would do. In particular, it is useful to catch certain
issues that go beyond the C header comparisons. For instance, it would
have been able to catch an issue like the `--version` option unexpectedly
requiring a header in 0.69.0 (fixed in 0.69.1) [1].

It would also have detected another issue present in 0.66.0 and 0.66.1:
a panic handling certain C headers with string literals containing
an interior NUL [2]. While the kernel is not really a stable test,
and such an issue would still require that a proper test is added,
it is nevertheless a good test case of non-trivial C headers that may
trigger edge cases like that.

Of course, `bindgen` may need to disable the test for different reasons,
i.e. there is no expectation to block any urgent/important PR, and the
kernel can also call `bindgen` differently depending on the version,
i.e. we are happy to adjust on our side too. Even if it gets disabled
often, we would still be in a better situation than not having the test
at all.

The Linux version (hash or tag) should ideally be updated from time to
time (e.g. every kernel `-rc1`), and each update should only contain
that change.

Link: rust-lang#2678 [1]
Link: rust-lang#2567 [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to ojeda/rust-bindgen that referenced this pull request Jun 19, 2024
Rust for Linux, so far, has pinned the Rust compiler and `bindgen`
versions. The kernel is looking into expanding that support to several
versions, i.e. establishing a minimum supported version, so that the
kernel can start to be more easily built. In particular, it should be
possible to build the kernel using the tools provided directly by Linux
distributions. In order to help achieve that goal, the Rust project has
added the kernel to its Rust pre-merge CI.

This commit does the same for `bindgen`. In particular, it adds a quick,
build-only test of the Rust code in the kernel as an extra step in the
`test` workflow.

This is intended to be an end-to-end test that runs what kernel
developers/users would do. In particular, it is useful to catch certain
issues that go beyond the C header comparisons. For instance, it would
have been able to catch an issue like the `--version` option unexpectedly
requiring a header in 0.69.0 (fixed in 0.69.1) [1].

It would also have detected another issue present in 0.66.0 and 0.66.1:
a panic handling certain C headers with string literals containing
an interior NUL [2]. While the kernel is not really a stable test,
and such an issue would still require that a proper test is added,
it is nevertheless a good test case of non-trivial C headers that may
trigger edge cases like that.

Of course, `bindgen` may need to disable the test for different reasons,
i.e. there is no expectation to block any urgent/important PR, and the
kernel can also call `bindgen` differently depending on the version,
i.e. we are happy to adjust on our side too. Even if it gets disabled
often, we would still be in a better situation than not having the test
at all.

The Linux version (hash or tag) should ideally be updated from time to
time (e.g. every kernel `-rc1`), and each update should only contain
that change.

Link: rust-lang#2678 [1]
Link: rust-lang#2567 [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to ojeda/rust-bindgen that referenced this pull request Jun 19, 2024
Rust for Linux, so far, has pinned the Rust compiler and `bindgen`
versions. The kernel is looking into expanding that support to several
versions, i.e. establishing a minimum supported version, so that the
kernel can start to be more easily built. In particular, it should be
possible to build the kernel using the tools provided directly by Linux
distributions. In order to help achieve that goal, the Rust project has
added the kernel to its Rust pre-merge CI.

This commit does the same for `bindgen`. In particular, it adds a quick,
build-only test of the Rust code in the kernel as an extra step in the
`test` workflow.

This is intended to be an end-to-end test that runs what kernel
developers/users would do. In particular, it is useful to catch certain
issues that go beyond the C header comparisons. For instance, it would
have been able to catch an issue like the `--version` option unexpectedly
requiring a header in 0.69.0 (fixed in 0.69.1) [1].

It would also have detected another issue present in 0.66.0 and 0.66.1:
a panic handling certain C headers with string literals containing
an interior NUL [2]. While the kernel is not really a stable test,
and such an issue would still require that a proper test is added,
it is nevertheless a good test case of non-trivial C headers that may
trigger edge cases like that.

Of course, `bindgen` may need to disable the test for different reasons,
i.e. there is no expectation to block any urgent/important PR, and the
kernel can also call `bindgen` differently depending on the version,
i.e. we are happy to adjust on our side too. Even if it gets disabled
often, we would still be in a better situation than not having the test
at all.

The Linux version (hash or tag) should ideally be updated from time to
time (e.g. every kernel `-rc1`), and each update should only contain
that change.

Link: rust-lang#2678 [1]
Link: rust-lang#2567 [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to ojeda/linux that referenced this pull request Jun 30, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

We could make it an error, but 1) it is going to fail anyway later in the
build, 2) we would disable `RUST` automatically, which is also painful,
3) someone could be using a patched `bindgen` at that version, 4) the
interior NUL may go away in the headers (however unlikely). Thus just
warn about it so that users know why it is failing.

In addition, add a test for the new case.

Link: rust-lang/rust-bindgen#2567 [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to ojeda/linux that referenced this pull request Jul 1, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

We could make it an error, but 1) it is going to fail anyway later in the
build, 2) we would disable `RUST` automatically, which is also painful,
3) someone could be using a patched `bindgen` at that version, 4) the
interior NUL may go away in the headers (however unlikely). Thus just
warn about it so that users know why it is failing.

In addition, add a test for the new case.

Link: rust-lang/rust-bindgen#2567 [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to ojeda/linux that referenced this pull request Jul 1, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

We could make it an error, but 1) it is going to fail anyway later in the
build, 2) we would disable `RUST` automatically, which is also painful,
3) someone could be using a patched `bindgen` at that version, 4) the
interior NUL may go away in the headers (however unlikely). Thus just
warn about it so that users know why it is failing.

In addition, add a test for the new case.

Link: rust-lang/rust-bindgen#2567 [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to ojeda/linux that referenced this pull request Jul 1, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

We could make it an error, but 1) it is going to fail anyway later in the
build, 2) we would disable `RUST` automatically, which is also painful,
3) someone could be using a patched `bindgen` at that version, 4) the
interior NUL may go away in the headers (however unlikely). Thus just
warn about it so that users know why it is failing.

In addition, add a test for the new case.

Link: rust-lang/rust-bindgen#2567 [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this pull request Jul 1, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

We could make it an error, but 1) it is going to fail anyway later in the
build, 2) we would disable `RUST` automatically, which is also painful,
3) someone could be using a patched `bindgen` at that version, 4) the
interior NUL may go away in the headers (however unlikely). Thus just
warn about it so that users know why it is failing.

In addition, add a test for the new case.

Link: rust-lang/rust-bindgen#2567 [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
fbq pushed a commit to Rust-for-Linux/linux that referenced this pull request Jul 1, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

We could make it an error, but 1) it is going to fail anyway later in the
build, 2) we would disable `RUST` automatically, which is also painful,
3) someone could be using a patched `bindgen` at that version, 4) the
interior NUL may go away in the headers (however unlikely). Thus just
warn about it so that users know why it is failing.

In addition, add a test for the new case.

Link: rust-lang/rust-bindgen#2567 [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://lore.kernel.org/r/20240701183625.665574-8-ojeda@kernel.org
Darksonn pushed a commit to Darksonn/linux that referenced this pull request Jul 4, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

We could make it an error, but 1) it is going to fail anyway later in the
build, 2) we would disable `RUST` automatically, which is also painful,
3) someone could be using a patched `bindgen` at that version, 4) the
interior NUL may go away in the headers (however unlikely). Thus just
warn about it so that users know why it is failing.

In addition, add a test for the new case.

Link: rust-lang/rust-bindgen#2567 [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Benno Lossin <benno.lossin@proton.me>
fbq pushed a commit to Rust-for-Linux/linux that referenced this pull request Jul 8, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

We could make it an error, but 1) it is going to fail anyway later in the
build, 2) we would disable `RUST` automatically, which is also painful,
3) someone could be using a patched `bindgen` at that version, 4) the
interior NUL may go away in the headers (however unlikely). Thus just
warn about it so that users know why it is failing.

In addition, add a test for the new case.

Link: rust-lang/rust-bindgen#2567 [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://lore.kernel.org/r/20240701183625.665574-8-ojeda@kernel.org
ojeda added a commit to ojeda/linux that referenced this pull request Jul 9, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

Since some distributions may have patched it (e.g. Debian did [2]),
check if that seems to be the case (after the version check matches),
in order to avoid printing a warning in that case.

We could make it an error, but 1) it is going to fail anyway later
in the build, 2) we would disable `RUST`, which is also painful, 3)
someone could have patched it in a way that still makes our extra check
fail (however unlikely), 4) the interior NUL may go away in the headers
(however unlikely). Thus just warn about it so that users know why it
is failing.

In addition, add a couple tests for the new cases.

Link: rust-lang/rust-bindgen#2567 [1]
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069047 [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
fbq pushed a commit to Rust-for-Linux/linux that referenced this pull request Jul 9, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

Since some distributions may have patched it (e.g. Debian did [2]),
check if that seems to be the case (after the version check matches),
in order to avoid printing a warning in that case.

We could make it an error, but 1) it is going to fail anyway later
in the build, 2) we would disable `RUST`, which is also painful, 3)
someone could have patched it in a way that still makes our extra check
fail (however unlikely), 4) the interior NUL may go away in the headers
(however unlikely). Thus just warn about it so that users know why it
is failing.

In addition, add a couple tests for the new cases.

Link: rust-lang/rust-bindgen#2567 [1]
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069047 [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://lore.kernel.org/r/20240709160615.998336-11-ojeda@kernel.org
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this pull request Jul 10, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

Since some distributions may have patched it (e.g. Debian did [2]),
check if that seems to be the case (after the version check matches),
in order to avoid printing a warning in that case.

We could make it an error, but 1) it is going to fail anyway later
in the build, 2) we would disable `RUST`, which is also painful, 3)
someone could have patched it in a way that still makes our extra check
fail (however unlikely), 4) the interior NUL may go away in the headers
(however unlikely). Thus just warn about it so that users know why it
is failing.

In addition, add a couple tests for the new cases.

Link: rust-lang/rust-bindgen#2567 [1]
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069047 [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to Rust-for-Linux/linux that referenced this pull request Jul 10, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

Since some distributions may have patched it (e.g. Debian did [2]),
check if that seems to be the case (after the version check matches),
in order to avoid printing a warning in that case.

We could make it an error, but 1) it is going to fail anyway later
in the build, 2) we would disable `RUST`, which is also painful, 3)
someone could have patched it in a way that still makes our extra check
fail (however unlikely), 4) the interior NUL may go away in the headers
(however unlikely). Thus just warn about it so that users know why it
is failing.

In addition, add a couple tests for the new cases.

Link: rust-lang/rust-bindgen#2567 [1]
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069047 [2]
Link: https://lore.kernel.org/r/20240709160615.998336-11-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
herrnst pushed a commit to herrnst/linux-asahi that referenced this pull request Jul 30, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

Since some distributions may have patched it (e.g. Debian did [2]),
check if that seems to be the case (after the version check matches),
in order to avoid printing a warning in that case.

We could make it an error, but 1) it is going to fail anyway later
in the build, 2) we would disable `RUST`, which is also painful, 3)
someone could have patched it in a way that still makes our extra check
fail (however unlikely), 4) the interior NUL may go away in the headers
(however unlikely). Thus just warn about it so that users know why it
is failing.

In addition, add a couple tests for the new cases.

Link: rust-lang/rust-bindgen#2567 [1]
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069047 [2]
Link: https://lore.kernel.org/r/20240709160615.998336-11-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
herrnst pushed a commit to herrnst/linux-asahi that referenced this pull request Jul 30, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

Since some distributions may have patched it (e.g. Debian did [2]),
check if that seems to be the case (after the version check matches),
in order to avoid printing a warning in that case.

We could make it an error, but 1) it is going to fail anyway later
in the build, 2) we would disable `RUST`, which is also painful, 3)
someone could have patched it in a way that still makes our extra check
fail (however unlikely), 4) the interior NUL may go away in the headers
(however unlikely). Thus just warn about it so that users know why it
is failing.

In addition, add a couple tests for the new cases.

Link: rust-lang/rust-bindgen#2567 [1]
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069047 [2]
Link: https://lore.kernel.org/r/20240709160615.998336-11-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
herrnst pushed a commit to herrnst/linux-asahi that referenced this pull request Jul 30, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

Since some distributions may have patched it (e.g. Debian did [2]),
check if that seems to be the case (after the version check matches),
in order to avoid printing a warning in that case.

We could make it an error, but 1) it is going to fail anyway later
in the build, 2) we would disable `RUST`, which is also painful, 3)
someone could have patched it in a way that still makes our extra check
fail (however unlikely), 4) the interior NUL may go away in the headers
(however unlikely). Thus just warn about it so that users know why it
is failing.

In addition, add a couple tests for the new cases.

Link: rust-lang/rust-bindgen#2567 [1]
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069047 [2]
Link: https://lore.kernel.org/r/20240709160615.998336-11-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
herrnst pushed a commit to herrnst/linux-asahi that referenced this pull request Aug 19, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

Since some distributions may have patched it (e.g. Debian did [2]),
check if that seems to be the case (after the version check matches),
in order to avoid printing a warning in that case.

We could make it an error, but 1) it is going to fail anyway later
in the build, 2) we would disable `RUST`, which is also painful, 3)
someone could have patched it in a way that still makes our extra check
fail (however unlikely), 4) the interior NUL may go away in the headers
(however unlikely). Thus just warn about it so that users know why it
is failing.

In addition, add a couple tests for the new cases.

Link: rust-lang/rust-bindgen#2567 [1]
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069047 [2]
Link: https://lore.kernel.org/r/20240709160615.998336-11-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
herrnst pushed a commit to herrnst/linux-asahi that referenced this pull request Aug 19, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

Since some distributions may have patched it (e.g. Debian did [2]),
check if that seems to be the case (after the version check matches),
in order to avoid printing a warning in that case.

We could make it an error, but 1) it is going to fail anyway later
in the build, 2) we would disable `RUST`, which is also painful, 3)
someone could have patched it in a way that still makes our extra check
fail (however unlikely), 4) the interior NUL may go away in the headers
(however unlikely). Thus just warn about it so that users know why it
is failing.

In addition, add a couple tests for the new cases.

Link: rust-lang/rust-bindgen#2567 [1]
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069047 [2]
Link: https://lore.kernel.org/r/20240709160615.998336-11-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
herrnst pushed a commit to herrnst/linux-asahi that referenced this pull request Aug 19, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

Since some distributions may have patched it (e.g. Debian did [2]),
check if that seems to be the case (after the version check matches),
in order to avoid printing a warning in that case.

We could make it an error, but 1) it is going to fail anyway later
in the build, 2) we would disable `RUST`, which is also painful, 3)
someone could have patched it in a way that still makes our extra check
fail (however unlikely), 4) the interior NUL may go away in the headers
(however unlikely). Thus just warn about it so that users know why it
is failing.

In addition, add a couple tests for the new cases.

Link: rust-lang/rust-bindgen#2567 [1]
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069047 [2]
Link: https://lore.kernel.org/r/20240709160615.998336-11-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
jannau pushed a commit to AsahiLinux/linux that referenced this pull request Aug 31, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

Since some distributions may have patched it (e.g. Debian did [2]),
check if that seems to be the case (after the version check matches),
in order to avoid printing a warning in that case.

We could make it an error, but 1) it is going to fail anyway later
in the build, 2) we would disable `RUST`, which is also painful, 3)
someone could have patched it in a way that still makes our extra check
fail (however unlikely), 4) the interior NUL may go away in the headers
(however unlikely). Thus just warn about it so that users know why it
is failing.

In addition, add a couple tests for the new cases.

Link: rust-lang/rust-bindgen#2567 [1]
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069047 [2]
Link: https://lore.kernel.org/r/20240709160615.998336-11-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
herrnst pushed a commit to herrnst/linux-asahi that referenced this pull request Sep 4, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

Since some distributions may have patched it (e.g. Debian did [2]),
check if that seems to be the case (after the version check matches),
in order to avoid printing a warning in that case.

We could make it an error, but 1) it is going to fail anyway later
in the build, 2) we would disable `RUST`, which is also painful, 3)
someone could have patched it in a way that still makes our extra check
fail (however unlikely), 4) the interior NUL may go away in the headers
(however unlikely). Thus just warn about it so that users know why it
is failing.

In addition, add a couple tests for the new cases.

Link: rust-lang/rust-bindgen#2567 [1]
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069047 [2]
Link: https://lore.kernel.org/r/20240709160615.998336-11-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
herrnst pushed a commit to herrnst/linux-asahi that referenced this pull request Sep 4, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

Since some distributions may have patched it (e.g. Debian did [2]),
check if that seems to be the case (after the version check matches),
in order to avoid printing a warning in that case.

We could make it an error, but 1) it is going to fail anyway later
in the build, 2) we would disable `RUST`, which is also painful, 3)
someone could have patched it in a way that still makes our extra check
fail (however unlikely), 4) the interior NUL may go away in the headers
(however unlikely). Thus just warn about it so that users know why it
is failing.

In addition, add a couple tests for the new cases.

Link: rust-lang/rust-bindgen#2567 [1]
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069047 [2]
Link: https://lore.kernel.org/r/20240709160615.998336-11-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
herrnst pushed a commit to herrnst/linux-asahi that referenced this pull request Sep 4, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

Since some distributions may have patched it (e.g. Debian did [2]),
check if that seems to be the case (after the version check matches),
in order to avoid printing a warning in that case.

We could make it an error, but 1) it is going to fail anyway later
in the build, 2) we would disable `RUST`, which is also painful, 3)
someone could have patched it in a way that still makes our extra check
fail (however unlikely), 4) the interior NUL may go away in the headers
(however unlikely). Thus just warn about it so that users know why it
is failing.

In addition, add a couple tests for the new cases.

Link: rust-lang/rust-bindgen#2567 [1]
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069047 [2]
Link: https://lore.kernel.org/r/20240709160615.998336-11-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
jannau pushed a commit to AsahiLinux/linux that referenced this pull request Sep 9, 2024
`bindgen` versions 0.66.0 and 0.66.1 panic due to C string literals with
NUL characters [1]:

    panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-0.66.0/codegen/mod.rs:717:71:
    called `Result::unwrap()` on an `Err` value: FromBytesWithNulError { kind: InteriorNul(4) }

Thus, in preparation for supporting several `bindgen` versions, add a
version check to warn the user about it.

Since some distributions may have patched it (e.g. Debian did [2]),
check if that seems to be the case (after the version check matches),
in order to avoid printing a warning in that case.

We could make it an error, but 1) it is going to fail anyway later
in the build, 2) we would disable `RUST`, which is also painful, 3)
someone could have patched it in a way that still makes our extra check
fail (however unlikely), 4) the interior NUL may go away in the headers
(however unlikely). Thus just warn about it so that users know why it
is failing.

In addition, add a couple tests for the new cases.

Link: rust-lang/rust-bindgen#2567 [1]
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069047 [2]
Link: https://lore.kernel.org/r/20240709160615.998336-11-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
github-merge-queue bot pushed a commit that referenced this pull request Sep 19, 2024
Rust for Linux, so far, has pinned the Rust compiler and `bindgen`
versions. The kernel is looking into expanding that support to several
versions, i.e. establishing a minimum supported version, so that the
kernel can start to be more easily built. In particular, it should be
possible to build the kernel using the tools provided directly by Linux
distributions. In order to help achieve that goal, the Rust project has
added the kernel to its Rust pre-merge CI.

This commit does the same for `bindgen`. In particular, it adds a quick,
build-only test of the Rust code in the kernel as an extra step in the
`test` workflow.

This is intended to be an end-to-end test that runs what kernel
developers/users would do. In particular, it is useful to catch certain
issues that go beyond the C header comparisons. For instance, it would
have been able to catch an issue like the `--version` option unexpectedly
requiring a header in 0.69.0 (fixed in 0.69.1) [1].

It would also have detected another issue present in 0.66.0 and 0.66.1:
a panic handling certain C headers with string literals containing
an interior NUL [2]. While the kernel is not really a stable test,
and such an issue would still require that a proper test is added,
it is nevertheless a good test case of non-trivial C headers that may
trigger edge cases like that.

Of course, `bindgen` may need to disable the test for different reasons,
i.e. there is no expectation to block any urgent/important PR, and the
kernel can also call `bindgen` differently depending on the version,
i.e. we are happy to adjust on our side too. Even if it gets disabled
often, we would still be in a better situation than not having the test
at all.

The Linux version (hash or tag) should ideally be updated from time to
time (e.g. every kernel `-rc1`), and each update should only contain
that change.

Link: #2678 [1]
Link: #2567 [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants