llvm: change data layout bug to an error and make it trigger more #120062

davidtwco · 2024-01-17T15:50:41Z

Don't skip the inconsistent data layout check for custom LLVMs or non-built-in targets.

With #118708, all targets will have a simple test that would trigger this error if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this error won't happen with our LLVM because each target will have been confirmed to work. With non-builtin targets, this error is probably useful to have because you can change the data layout in your target and if it is wrong then that could lead to bugs.

When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. I'm not sure if this is something that people do and is okay, but I doubt it?

CFG_LLVM_ROOT was also always set during local development with download-ci-llvm so this bug would never trigger locally.

In #33446, two points are raised:

In the issue itself, changing this from a bug! to a proper error is what is suggested, by using isCompatibleDataLayout from LLVM, but that function still just does the same thing that we do and check for equality, so I've avoided the additional code necessary to do that FFI call.
@Mark-Simulacrum suggests a different check is necessary to maintain backwards compatibility with old LLVM versions. I don't know how often this comes up, but we can do that with some simple string manipulation + LLVM version checks as happens already for LLVM 17 just above this diff.

rustbot · 2024-01-17T15:50:50Z

r? @wesleywiser

(rustbot has picked a reviewer for you, use r? to override)

tests/ui/codegen/mismatched-data-layouts.stderr

Don't skip the inconsistent data layout check for custom LLVMs. With rust-lang#118708, all targets will have a simple test that would trigger this check if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this check won't trigger with our LLVM because each target will have been confirmed to work. With non-builtin targets, this check is probably useful to have because you can change the data layout in your target and if its wrong then that could lead to bugs. When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. `CFG_LLVM_ROOT` was also always set during local development with `download-ci-llvm` so this bug would never trigger locally. Signed-off-by: David Wood <david@davidtw.co>

compiler/rustc_codegen_llvm/src/context.rs

tests/ui/codegen/mismatched-data-layouts.stderr

wesleywiser · 2024-01-23T17:44:17Z

Thanks @davidtwco!

@bors r+

bors · 2024-01-23T17:44:20Z

📌 Commit 46652dd has been approved by wesleywiser

It is now in the queue for this repository.

…r=wesleywiser llvm: change data layout bug to an error and make it trigger more Fixes rust-lang#33446. Don't skip the inconsistent data layout check for custom LLVMs or non-built-in targets. With rust-lang#118708, all targets will have a simple test that would trigger this error if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this error won't happen with our LLVM because each target will have been confirmed to work. With non-builtin targets, this error is probably useful to have because you can change the data layout in your target and if it is wrong then that could lead to bugs. When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. I'm not sure if this is something that people do and is okay, but I doubt it? `CFG_LLVM_ROOT` was also always set during local development with `download-ci-llvm` so this bug would never trigger locally. In rust-lang#33446, two points are raised: - In the issue itself, changing this from a `bug!` to a proper error is what is suggested, by using `isCompatibleDataLayout` from LLVM, but that function still just does the same thing that we do and check for equality, so I've avoided the additional code necessary to do that FFI call. - `@Mark-Simulacrum` suggests a different check is necessary to maintain backwards compatibility with old LLVM versions. I don't know how often this comes up, but we can do that with some simple string manipulation + LLVM version checks as happens already for LLVM 17 just above this diff.

Rollup of 12 pull requests Successful merges: - rust-lang#112806 (Small code improvements in `collect_intra_doc_links.rs`) - rust-lang#119460 (coverage: Never emit improperly-ordered coverage regions) - rust-lang#119766 (Split tait and impl trait in assoc items logic) - rust-lang#120062 (llvm: change data layout bug to an error and make it trigger more) - rust-lang#120099 (linker: Refactor library linking methods in `trait Linker`) - rust-lang#120139 (Do not normalize closure signature when building `FnOnce` shim) - rust-lang#120160 (Manually implement derived `NonZero` traits.) - rust-lang#120171 (Fix assume and assert in jump threading) - rust-lang#120183 (Add `#[coverage(off)]` to closures introduced by `#[test]` and `#[bench]`) - rust-lang#120195 (add several resolution test cases) - rust-lang#120259 (Split Diagnostics for Uncommon Codepoints: Add List to Display Characters Involved) - rust-lang#120261 (Provide structured suggestion to use trait objects in some cases of `if` arm type divergence) r? `@ghost` `@rustbot` modify labels: rollup

…r=wesleywiser llvm: change data layout bug to an error and make it trigger more Fixes rust-lang#33446. Don't skip the inconsistent data layout check for custom LLVMs or non-built-in targets. With rust-lang#118708, all targets will have a simple test that would trigger this error if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this error won't happen with our LLVM because each target will have been confirmed to work. With non-builtin targets, this error is probably useful to have because you can change the data layout in your target and if it is wrong then that could lead to bugs. When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. I'm not sure if this is something that people do and is okay, but I doubt it? `CFG_LLVM_ROOT` was also always set during local development with `download-ci-llvm` so this bug would never trigger locally. In rust-lang#33446, two points are raised: - In the issue itself, changing this from a `bug!` to a proper error is what is suggested, by using `isCompatibleDataLayout` from LLVM, but that function still just does the same thing that we do and check for equality, so I've avoided the additional code necessary to do that FFI call. - ``@Mark-Simulacrum`` suggests a different check is necessary to maintain backwards compatibility with old LLVM versions. I don't know how often this comes up, but we can do that with some simple string manipulation + LLVM version checks as happens already for LLVM 17 just above this diff.

Rollup of 7 pull requests Successful merges: - rust-lang#119460 (coverage: Never emit improperly-ordered coverage regions) - rust-lang#120062 (llvm: change data layout bug to an error and make it trigger more) - rust-lang#120124 (Split assembly tests for ELF and MachO) - rust-lang#120185 (coverage: Don't instrument `#[automatically_derived]` functions) - rust-lang#120265 (Remove no-system-llvm) - rust-lang#120277 (Normalize field types before checking validity) - rust-lang#120285 (Remove extra # from url in suggestion) r? `@ghost` `@rustbot` modify labels: rollup

…r=wesleywiser llvm: change data layout bug to an error and make it trigger more Fixes rust-lang#33446. Don't skip the inconsistent data layout check for custom LLVMs or non-built-in targets. With rust-lang#118708, all targets will have a simple test that would trigger this error if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this error won't happen with our LLVM because each target will have been confirmed to work. With non-builtin targets, this error is probably useful to have because you can change the data layout in your target and if it is wrong then that could lead to bugs. When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. I'm not sure if this is something that people do and is okay, but I doubt it? `CFG_LLVM_ROOT` was also always set during local development with `download-ci-llvm` so this bug would never trigger locally. In rust-lang#33446, two points are raised: - In the issue itself, changing this from a `bug!` to a proper error is what is suggested, by using `isCompatibleDataLayout` from LLVM, but that function still just does the same thing that we do and check for equality, so I've avoided the additional code necessary to do that FFI call. - ```@Mark-Simulacrum``` suggests a different check is necessary to maintain backwards compatibility with old LLVM versions. I don't know how often this comes up, but we can do that with some simple string manipulation + LLVM version checks as happens already for LLVM 17 just above this diff.

Rollup of 7 pull requests Successful merges: - rust-lang#119460 (coverage: Never emit improperly-ordered coverage regions) - rust-lang#120062 (llvm: change data layout bug to an error and make it trigger more) - rust-lang#120124 (Split assembly tests for ELF and MachO) - rust-lang#120165 (Switch `NonZero` alias direction.) - rust-lang#120185 (coverage: Don't instrument `#[automatically_derived]` functions) - rust-lang#120265 (Remove no-system-llvm) - rust-lang#120285 (Remove extra # from url in suggestion) r? `@ghost` `@rustbot` modify labels: rollup

…r=wesleywiser llvm: change data layout bug to an error and make it trigger more Fixes rust-lang#33446. Don't skip the inconsistent data layout check for custom LLVMs or non-built-in targets. With rust-lang#118708, all targets will have a simple test that would trigger this error if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this error won't happen with our LLVM because each target will have been confirmed to work. With non-builtin targets, this error is probably useful to have because you can change the data layout in your target and if it is wrong then that could lead to bugs. When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. I'm not sure if this is something that people do and is okay, but I doubt it? `CFG_LLVM_ROOT` was also always set during local development with `download-ci-llvm` so this bug would never trigger locally. In rust-lang#33446, two points are raised: - In the issue itself, changing this from a `bug!` to a proper error is what is suggested, by using `isCompatibleDataLayout` from LLVM, but that function still just does the same thing that we do and check for equality, so I've avoided the additional code necessary to do that FFI call. - ````@Mark-Simulacrum```` suggests a different check is necessary to maintain backwards compatibility with old LLVM versions. I don't know how often this comes up, but we can do that with some simple string manipulation + LLVM version checks as happens already for LLVM 17 just above this diff.

Rollup of 8 pull requests Successful merges: - rust-lang#119305 (Add `AsyncFn` family of traits) - rust-lang#119389 (Provide more context on recursive `impl` evaluation overflow) - rust-lang#120062 (llvm: change data layout bug to an error and make it trigger more) - rust-lang#120099 (linker: Refactor library linking methods in `trait Linker`) - rust-lang#120201 (Bump some deps with syn 1.0 dependencies) - rust-lang#120230 (Assert that a single scope is passed to `for_scope`) - rust-lang#120278 (Remove --fatal-warnings on wasm targets) - rust-lang#120292 (coverage: Dismantle `Instrumentor` and flatten span refinement) r? `@ghost` `@rustbot` modify labels: rollup

bors · 2024-01-27T12:19:44Z

⌛ Testing commit 46652dd with merge 8af70c7...

bors · 2024-01-27T14:18:05Z

☀️ Test successful - checks-actions
Approved by: wesleywiser
Pushing 8af70c7 to master...

rust-timer · 2024-01-27T16:20:22Z

Finished benchmarking commit (8af70c7): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.6%	[2.2%, 3.0%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.6%	[2.2%, 3.0%]	2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.4%	[2.4%, 2.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 660.284s -> 662.371s (0.32%)
Artifact size: 308.12 MiB -> 308.10 MiB (-0.00%)

test: data layout fix for `x86_64-unknown-none-gnu` Tests are broken due to LLVM target layout check in rustc. See <rust-lang/rust#120062>.

…cuviper Update data layouts in custom target tests for LLVM 18 Apply the data layout changes from rust-lang#116672 to custom target specs as well, as we started validating them since rust-lang#120062. Fixes rust-lang#120492. r? `@cuviper`

…cuviper Update data layouts in custom target tests for LLVM 18 Apply the data layout changes from rust-lang#116672 to custom target specs as well, as we started validating them since rust-lang#120062. Fixes rust-lang#120492. r? ``@cuviper``

…cuviper Update data layouts in custom target tests for LLVM 18 Apply the data layout changes from rust-lang#116672 to custom target specs as well, as we started validating them since rust-lang#120062. Fixes rust-lang#120492. r? ```@cuviper```

Rollup merge of rust-lang#120529 - nikic:llvm-18-datalayout-fixes, r=cuviper Update data layouts in custom target tests for LLVM 18 Apply the data layout changes from rust-lang#116672 to custom target specs as well, as we started validating them since rust-lang#120062. Fixes rust-lang#120492. r? ```@cuviper```

This is the next upgrade to the Rust toolchain, from 1.77.1 to 1.78.0 (i.e. the latest) [1]. See the upgrade policy [2] and the comments on the first upgrade in commit 3ed03f4 ("rust: upgrade to Rust 1.68.2"). # Unstable features There have been no changes to the set of unstable features used in our own code. Therefore, the only unstable features allowed to be used outside the `kernel` crate is still `new_uninit`. However, since we are finally dropping our `alloc` fork [3], all the unstable features used by `alloc` (~30 language ones, ~60 library ones) are not a concern anymore. This reduces the maintanance burden, increases the chances of new compiler versions working without changes and gets us closer to the goal of supporting several compiler versions. It also means that, ignoring non-language/library features, we are currently left with just the few language features needed to implement the kernel `Arc`, the `new_uninit` library feature, the `compiler_builtins` marker and the few `no_*` `cfg`s we pass when compiling `core`/`alloc`. Please see [4] for details. # Required changes ## LLVM's data layout Rust 1.77.0 (i.e. the previous upgrade) introduced a check for matching LLVM data layouts [5]. Then, Rust 1.78.0 upgraded LLVM's bundled major version from 17 to 18 [6], which changed the data layout in x86 [7]. Thus update the data layout in our custom target specification for x86 so that the compiler does not complain about the mismatch: error: data-layout for target `target-5559158138856098584`, `e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128`, differs from LLVM target's `x86_64-linux-gnu` default layout, `e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128` In the future, the goal is to drop the custom target specification files. Meanwhile, if we want to support other LLVM versions used in `rustc` (e.g. for LTO), we will need to add some extra logic (e.g. conditional on LLVM's version, or extracting the data layout from an existing built-in target specification). ## `unused_imports` Rust's `unused_imports` lint covers both unused and redudant imports. Now, in 1.78.0, the lint detects more cases of redundant imports [8]. Thus the previous commit cleaned them up. ## Clippy's `new_without_default` Clippy now suggests to implement `Default` even when `new()` is `const`, since `Default::default()` may call `const` functions even if it is not `const` itself [9]. Thus the previous commit added the implementation. # Other changes in Rust Rust 1.78.0 introduces `feature(asm_goto)` [10] [11]. This feature was discussed in the past [12]. Rust 1.78.0 introduced support for mutable pointers to Rust statics, including a test case for the Linux kernel's `VTABLE` use case [13]. Rust 1.78.0 with debug assertions enabled (i.e. `-Cdebug-assertions=y`, kernel's `CONFIG_RUST_DEBUG_ASSERTIONS=y`) will now always check all unsafe preconditions, without a way to opt-out for particular cases [14]. Rust 1.78.0 also improved a couple issues we reported when giving feedback for the new `--check-cfg` feature [15] [16]. # `alloc` upgrade and reviewing As mentioned above, compiler upgrades will not update `alloc` anymore, since we are dropping our `alloc` fork [3]. As a bonus, even if that series is not applied, the new compiler release happens to build cleanly the existing `alloc` too (i.e. the previous version's). Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1770-2024-03-21 [1] Link: https://rust-for-linux.com/rust-version-policy [2] Link: https://lore.kernel.org/rust-for-linux/20240328013603.206764-1-wedsonaf@gmail.com/ [3] Link: Rust-for-Linux#2 [4] Link: rust-lang/rust#120062 [5] Link: rust-lang/rust#120055 [6] Link: https://reviews.llvm.org/D86310 [7] Link: rust-lang/rust#117772 [8] Link: rust-lang/rust-clippy#10903 [9] Link: rust-lang/rust#119365 [10] Link: rust-lang/rust#119364 [11] Link: https://lore.kernel.org/rust-for-linux/ZWipTZysC2YL7qsq@Boquns-Mac-mini.home/ [12] Link: rust-lang/rust#120932 [13] Link: rust-lang/rust#120969 [14] Link: rust-lang/rust#121202 [15] Link: rust-lang/rust#121237 [16] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

This is the next upgrade to the Rust toolchain, from 1.77.1 to 1.78.0 (i.e. the latest) [1]. See the upgrade policy [2] and the comments on the first upgrade in commit 3ed03f4 ("rust: upgrade to Rust 1.68.2"). # Unstable features There have been no changes to the set of unstable features used in our own code. Therefore, the only unstable features allowed to be used outside the `kernel` crate is still `new_uninit`. However, since we are finally dropping our `alloc` fork [3], all the unstable features used by `alloc` (~30 language ones, ~60 library ones) are not a concern anymore. This reduces the maintenance burden, increases the chances of new compiler versions working without changes and gets us closer to the goal of supporting several compiler versions. It also means that, ignoring non-language/library features, we are currently left with just the few language features needed to implement the kernel `Arc`, the `new_uninit` library feature, the `compiler_builtins` marker and the few `no_*` `cfg`s we pass when compiling `core`/`alloc`. Please see [4] for details. # Required changes ## LLVM's data layout Rust 1.77.0 (i.e. the previous upgrade) introduced a check for matching LLVM data layouts [5]. Then, Rust 1.78.0 upgraded LLVM's bundled major version from 17 to 18 [6], which changed the data layout in x86 [7]. Thus update the data layout in our custom target specification for x86 so that the compiler does not complain about the mismatch: error: data-layout for target `target-5559158138856098584`, `e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128`, differs from LLVM target's `x86_64-linux-gnu` default layout, `e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128` In the future, the goal is to drop the custom target specification files. Meanwhile, if we want to support other LLVM versions used in `rustc` (e.g. for LTO), we will need to add some extra logic (e.g. conditional on LLVM's version, or extracting the data layout from an existing built-in target specification). ## `unused_imports` Rust's `unused_imports` lint covers both unused and redundant imports. Now, in 1.78.0, the lint detects more cases of redundant imports [8]. Thus one of the previous patches cleaned them up. ## Clippy's `new_without_default` Clippy now suggests to implement `Default` even when `new()` is `const`, since `Default::default()` may call `const` functions even if it is not `const` itself [9]. Thus one of the previous patches implemented it. # Other changes in Rust Rust 1.78.0 introduced `feature(asm_goto)` [10] [11]. This feature was discussed in the past [12]. Rust 1.78.0 introduced support for mutable pointers to Rust statics, including a test case for the Linux kernel's `VTABLE` use case [13]. Rust 1.78.0 with debug assertions enabled (i.e. `-Cdebug-assertions=y`, kernel's `CONFIG_RUST_DEBUG_ASSERTIONS=y`) now always checks all unsafe preconditions, without a way to opt-out for particular cases [14]. Rust 1.78.0 also improved a couple issues we reported when giving feedback for the new `--check-cfg` feature [15] [16]. # `alloc` upgrade and reviewing As mentioned above, compiler upgrades will not update `alloc` anymore, since we are dropping our `alloc` fork [3]. Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1780-2024-05-02 [1] Link: https://rust-for-linux.com/rust-version-policy [2] Link: https://lore.kernel.org/rust-for-linux/20240328013603.206764-1-wedsonaf@gmail.com/ [3] Link: Rust-for-Linux#2 [4] Link: rust-lang/rust#120062 [5] Link: rust-lang/rust#120055 [6] Link: https://reviews.llvm.org/D86310 [7] Link: rust-lang/rust#117772 [8] Link: rust-lang/rust-clippy#10903 [9] Link: rust-lang/rust#119365 [10] Link: rust-lang/rust#119364 [11] Link: https://lore.kernel.org/rust-for-linux/ZWipTZysC2YL7qsq@Boquns-Mac-mini.home/ [12] Link: rust-lang/rust#120932 [13] Link: rust-lang/rust#120969 [14] Link: rust-lang/rust#121202 [15] Link: rust-lang/rust#121237 [16] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

This is the next upgrade to the Rust toolchain, from 1.77.1 to 1.78.0 (i.e. the latest) [1]. See the upgrade policy [2] and the comments on the first upgrade in commit 3ed03f4 ("rust: upgrade to Rust 1.68.2"). # Unstable features There have been no changes to the set of unstable features used in our own code. Therefore, the only unstable features allowed to be used outside the `kernel` crate is still `new_uninit`. However, since we are finally dropping our `alloc` fork [3], all the unstable features used by `alloc` (~30 language ones, ~60 library ones) are not a concern anymore. This reduces the maintenance burden, increases the chances of new compiler versions working without changes and gets us closer to the goal of supporting several compiler versions. It also means that, ignoring non-language/library features, we are currently left with just the few language features needed to implement the kernel `Arc`, the `new_uninit` library feature, the `compiler_builtins` marker and the few `no_*` `cfg`s we pass when compiling `core`/`alloc`. Please see [4] for details. # Required changes ## LLVM's data layout Rust 1.77.0 (i.e. the previous upgrade) introduced a check for matching LLVM data layouts [5]. Then, Rust 1.78.0 upgraded LLVM's bundled major version from 17 to 18 [6], which changed the data layout in x86 [7]. Thus update the data layout in our custom target specification for x86 so that the compiler does not complain about the mismatch: error: data-layout for target `target-5559158138856098584`, `e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128`, differs from LLVM target's `x86_64-linux-gnu` default layout, `e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128` In the future, the goal is to drop the custom target specifications. Meanwhile, if we want to support other LLVM versions used in `rustc` (e.g. for LTO), we will need to add some extra logic (e.g. conditional on LLVM's version, or extracting the data layout from an existing built-in target specification). ## `unused_imports` Rust's `unused_imports` lint covers both unused and redundant imports. Now, in 1.78.0, the lint detects more cases of redundant imports [8]. Thus one of the previous patches cleaned them up. ## Clippy's `new_without_default` Clippy now suggests to implement `Default` even when `new()` is `const`, since `Default::default()` may call `const` functions even if it is not `const` itself [9]. Thus one of the previous patches implemented it. # Other changes in Rust Rust 1.78.0 introduced `feature(asm_goto)` [10] [11]. This feature was discussed in the past [12]. Rust 1.78.0 introduced support for mutable pointers to Rust statics, including a test case for the Linux kernel's `VTABLE` use case [13]. Rust 1.78.0 with debug assertions enabled (i.e. `-Cdebug-assertions=y`, kernel's `CONFIG_RUST_DEBUG_ASSERTIONS=y`) now always checks all unsafe preconditions, without a way to opt-out for particular cases [14]. Rust 1.78.0 also improved a couple issues we reported when giving feedback for the new `--check-cfg` feature [15] [16]. # `alloc` upgrade and reviewing As mentioned above, compiler upgrades will not update `alloc` anymore, since we are dropping our `alloc` fork [3]. Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1780-2024-05-02 [1] Link: https://rust-for-linux.com/rust-version-policy [2] Link: https://lore.kernel.org/rust-for-linux/20240328013603.206764-1-wedsonaf@gmail.com/ [3] Link: Rust-for-Linux#2 [4] Link: rust-lang/rust#120062 [5] Link: rust-lang/rust#120055 [6] Link: https://reviews.llvm.org/D86310 [7] Link: rust-lang/rust#117772 [8] Link: rust-lang/rust-clippy#10903 [9] Link: rust-lang/rust#119365 [10] Link: rust-lang/rust#119364 [11] Link: https://lore.kernel.org/rust-for-linux/ZWipTZysC2YL7qsq@Boquns-Mac-mini.home/ [12] Link: rust-lang/rust#120932 [13] Link: rust-lang/rust#120969 [14] Link: rust-lang/rust#121202 [15] Link: rust-lang/rust#121237 [16] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

This is the next upgrade to the Rust toolchain, from 1.77.1 to 1.78.0 (i.e. the latest) [1]. See the upgrade policy [2] and the comments on the first upgrade in commit 3ed03f4 ("rust: upgrade to Rust 1.68.2"). It is much smaller than previous upgrades, since the `alloc` fork was dropped in commit 9d0441b ("rust: alloc: remove our fork of the `alloc` crate") [3]. # Unstable features There have been no changes to the set of unstable features used in our own code. Therefore, the only unstable features allowed to be used outside the `kernel` crate is still `new_uninit`. However, since we finally dropped our `alloc` fork [3], all the unstable features used by `alloc` (~30 language ones, ~60 library ones) are not a concern anymore. This reduces the maintenance burden, increases the chances of new compiler versions working without changes and gets us closer to the goal of supporting several compiler versions. It also means that, ignoring non-language/library features, we are currently left with just the few language features needed to implement the kernel `Arc`, the `new_uninit` library feature, the `compiler_builtins` marker and the few `no_*` `cfg`s we pass when compiling `core`/`alloc`. Please see [4] for details. # Required changes ## LLVM's data layout Rust 1.77.0 (i.e. the previous upgrade) introduced a check for matching LLVM data layouts [5]. Then, Rust 1.78.0 upgraded LLVM's bundled major version from 17 to 18 [6], which changed the data layout in x86 [7]. Thus update the data layout in our custom target specification for x86 so that the compiler does not complain about the mismatch: error: data-layout for target `target-5559158138856098584`, `e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128`, differs from LLVM target's `x86_64-linux-gnu` default layout, `e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128` In the future, the goal is to drop the custom target specifications. Meanwhile, if we want to support other LLVM versions used in `rustc` (e.g. for LTO), we will need to add some extra logic (e.g. conditional on LLVM's version, or extracting the data layout from an existing built-in target specification). ## `unused_imports` Rust's `unused_imports` lint covers both unused and redundant imports. Now, in 1.78.0, the lint detects more cases of redundant imports [8]. Thus one of the previous patches cleaned them up. ## Clippy's `new_without_default` Clippy now suggests to implement `Default` even when `new()` is `const`, since `Default::default()` may call `const` functions even if it is not `const` itself [9]. Thus one of the previous patches implemented it. # Other changes in Rust Rust 1.78.0 introduced `feature(asm_goto)` [10] [11]. This feature was discussed in the past [12]. Rust 1.78.0 introduced `feature(const_refs_to_static)` [13] to allow referencing statics in constants and extended `feature(const_mut_refs)` to allow raw mutable pointers in constants. Together, this should cover the kernel's `VTABLE` use case. In fact, the implementation [14] in upstream Rust added a test case for it [15]. Rust 1.78.0 with debug assertions enabled (i.e. `-Cdebug-assertions=y`, kernel's `CONFIG_RUST_DEBUG_ASSERTIONS=y`) now always checks all unsafe preconditions, though without a way to opt-out for particular cases [16]. It would be ideal to have a way to selectively disable certain checks per-call site for this one (i.e. not just per check but for particular instances of a check), even if the vast majority of the checks remain in place [17]. Rust 1.78.0 also improved a couple issues we reported when giving feedback for the new `--check-cfg` feature [18] [19]. # `alloc` upgrade and reviewing As mentioned above, compiler upgrades will not update `alloc` anymore, since we dropped our `alloc` fork [3]. Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1780-2024-05-02 [1] Link: https://rust-for-linux.com/rust-version-policy [2] Link: https://lore.kernel.org/rust-for-linux/20240328013603.206764-1-wedsonaf@gmail.com/ [3] Link: Rust-for-Linux#2 [4] Link: rust-lang/rust#120062 [5] Link: rust-lang/rust#120055 [6] Link: https://reviews.llvm.org/D86310 [7] Link: rust-lang/rust#117772 [8] Link: rust-lang/rust-clippy#10903 [9] Link: rust-lang/rust#119365 [10] Link: rust-lang/rust#119364 [11] Link: https://lore.kernel.org/rust-for-linux/ZWipTZysC2YL7qsq@Boquns-Mac-mini.home/ [12] Link: rust-lang/rust#119618 [13] Link: rust-lang/rust#120932 [14] Link: https://github.com/rust-lang/rust/pull/120932/files#diff-e6fc1622c46054cd46b1d225c5386c5554564b3b0fa8a03c2dc2d8627a1079d9 [15] Link: rust-lang/rust#120969 [16] Link: Rust-for-Linux#354 [17] Link: rust-lang/rust#121202 [18] Link: rust-lang/rust#121237 [19] Reviewed-by: Alice Ryhl <aliceryhl@google.com> Link: https://lore.kernel.org/r/20240401212303.537355-4-ojeda@kernel.org [ Added a few more details and links I mentioned in the list. - Miguel ] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

rustbot assigned wesleywiser Jan 17, 2024

This comment has been minimized.

Sign in to view

nikic reviewed Jan 17, 2024

View reviewed changes

tests/ui/codegen/mismatched-data-layouts.stderr Outdated Show resolved Hide resolved

davidtwco force-pushed the llvm-data-layout-check branch from 8801304 to 46652dd Compare January 18, 2024 10:46

wesleywiser reviewed Jan 22, 2024

View reviewed changes

compiler/rustc_codegen_llvm/src/context.rs Show resolved Hide resolved

tests/ui/codegen/mismatched-data-layouts.stderr Show resolved Hide resolved

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 23, 2024

fmease mentioned this pull request Jan 23, 2024

Rollup of 12 pull requests #120279

Closed

fmease mentioned this pull request Jan 24, 2024

Rollup of 7 pull requests #120289

Closed

fmease mentioned this pull request Jan 24, 2024

Rollup of 7 pull requests #120290

Closed

fmease mentioned this pull request Jan 24, 2024

Rollup of 8 pull requests #120304

Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 27, 2024

bors merged commit 8af70c7 into rust-lang:master Jan 27, 2024
12 checks passed

rustbot added this to the 1.77.0 milestone Jan 27, 2024

This was referenced Jan 28, 2024

Fix data layout for stage 3 target rust-osdev/bootloader#413

Merged

[v0.9] Fix data layout for x86_64-bootloader target rust-osdev/bootloader#415

Merged

Update data layouts in target specifications phil-opp/blog_os#1271

Merged

weihanglo mentioned this pull request Jan 28, 2024

test: data layout fix for x86_64-unknown-none-gnu rust-lang/cargo#13362

Merged

bors added a commit to rust-lang/cargo that referenced this pull request Jan 28, 2024

Auto merge of #13362 - weihanglo:data-layout, r=ehuss

95e113c

test: data layout fix for `x86_64-unknown-none-gnu` Tests are broken due to LLVM target layout check in rustc. See <rust-lang/rust#120062>.

davidtwco deleted the llvm-data-layout-check branch January 29, 2024 11:12

nikic mentioned this pull request Jan 31, 2024

Update data layouts in custom target tests for LLVM 18 #120529

Merged

phil-opp mentioned this pull request Feb 2, 2024

Fix data layout in custom target used for testing rust-osdev/x86_64#454

Merged

phil-opp mentioned this pull request Feb 15, 2024

De-stabilize target spec JSON #71009

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

llvm: change data layout bug to an error and make it trigger more #120062

llvm: change data layout bug to an error and make it trigger more #120062

davidtwco commented Jan 17, 2024

rustbot commented Jan 17, 2024

This comment has been minimized.

wesleywiser commented Jan 23, 2024

bors commented Jan 23, 2024

bors commented Jan 27, 2024

bors commented Jan 27, 2024

rust-timer commented Jan 27, 2024

llvm: change data layout bug to an error and make it trigger more #120062

llvm: change data layout bug to an error and make it trigger more #120062

Conversation

davidtwco commented Jan 17, 2024

rustbot commented Jan 17, 2024

This comment has been minimized.

wesleywiser commented Jan 23, 2024

bors commented Jan 23, 2024

bors commented Jan 27, 2024

bors commented Jan 27, 2024

rust-timer commented Jan 27, 2024

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size