Auto merge of #120289 - fmease:rollup-f3qh886, r=fmease

Rollup of 7 pull requests

Successful merges:

 - #119460 (coverage: Never emit improperly-ordered coverage regions)
 - #120062 (llvm: change data layout bug to an error and make it trigger more)
 - #120124 (Split assembly tests for ELF and MachO)
 - #120185 (coverage: Don't instrument `#[automatically_derived]` functions)
 - #120265 (Remove no-system-llvm)
 - #120277 (Normalize field types before checking validity)
 - #120285 (Remove extra # from url in suggestion)

r? `@ghost`
`@rustbot` modify labels: rollup

compiler/rustc_codegen_llvm/messages.ftl

@@ -39,6 +39,9 @@ codegen_llvm_lto_dylib = lto cannot be used for `dylib` crate type without `-Zdy
 
 codegen_llvm_lto_proc_macro = lto cannot be used for `proc-macro` crate type without `-Zdylib-lto`
 
+codegen_llvm_mismatch_data_layout =
+    data-layout for target `{$rustc_target}`, `{$rustc_layout}`, differs from LLVM target's `{$llvm_target}` default layout, `{$llvm_layout}`
+
 codegen_llvm_missing_features =
     add the missing features in a `target_feature` attribute
 

compiler/rustc_codegen_llvm/src/context.rs

@@ -34,6 +34,7 @@ use rustc_target::spec::{HasTargetSpec, RelocModel, Target, TlsModel};
 use smallvec::SmallVec;
 
 use libc::c_uint;
+use std::borrow::Borrow;
 use std::cell::{Cell, RefCell};
 use std::ffi::CStr;
 use std::str;
@@ -155,42 +156,21 @@ pub unsafe fn create_module<'ll>(
     }
 
     // Ensure the data-layout values hardcoded remain the defaults.
-    if sess.target.is_builtin {
-        // tm is disposed by its drop impl
+    {
         let tm = crate::back::write::create_informational_target_machine(tcx.sess);
         llvm::LLVMRustSetDataLayoutFromTargetMachine(llmod, &tm);
 
         let llvm_data_layout = llvm::LLVMGetDataLayoutStr(llmod);
         let llvm_data_layout = str::from_utf8(CStr::from_ptr(llvm_data_layout).to_bytes())
             .expect("got a non-UTF8 data-layout from LLVM");
 
-        // Unfortunately LLVM target specs change over time, and right now we
-        // don't have proper support to work with any more than one
-        // `data_layout` than the one that is in the rust-lang/rust repo. If
-        // this compiler is configured against a custom LLVM, we may have a
-        // differing data layout, even though we should update our own to use
-        // that one.
-        //
-        // As an interim hack, if CFG_LLVM_ROOT is not an empty string then we
-        // disable this check entirely as we may be configured with something
-        // that has a different target layout.
-        //
-        // Unsure if this will actually cause breakage when rustc is configured
-        // as such.
-        //
-        // FIXME(#34960)
-        let cfg_llvm_root = option_env!("CFG_LLVM_ROOT").unwrap_or("");
-        let custom_llvm_used = !cfg_llvm_root.trim().is_empty();
-
-        if !custom_llvm_used && target_data_layout != llvm_data_layout {
-            bug!(
-                "data-layout for target `{rustc_target}`, `{rustc_layout}`, \
-                  differs from LLVM target's `{llvm_target}` default layout, `{llvm_layout}`",
-                rustc_target = sess.opts.target_triple,
-                rustc_layout = target_data_layout,
-                llvm_target = sess.target.llvm_target,
-                llvm_layout = llvm_data_layout
-            );
+        if target_data_layout != llvm_data_layout {
+            tcx.dcx().emit_err(crate::errors::MismatchedDataLayout {
+                rustc_target: sess.opts.target_triple.to_string().as_str(),
+                rustc_layout: target_data_layout.as_str(),
+                llvm_target: sess.target.llvm_target.borrow(),
+                llvm_layout: llvm_data_layout,
+            });
         }
     }
 

compiler/rustc_codegen_llvm/src/errors.rs

@@ -244,3 +244,12 @@ pub(crate) struct CopyBitcode {
 pub struct UnknownCompression {
     pub algorithm: &'static str,
 }
+
+#[derive(Diagnostic)]
+#[diag(codegen_llvm_mismatch_data_layout)]
+pub struct MismatchedDataLayout<'a> {
+    pub rustc_target: &'a str,
+    pub rustc_layout: &'a str,
+    pub llvm_target: &'a str,
+    pub llvm_layout: &'a str,
+}

compiler/rustc_const_eval/src/transform/validate.rs

@@ -810,13 +810,18 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                     let adt_def = self.tcx.adt_def(def_id);
                     assert!(adt_def.is_union());
                     assert_eq!(idx, FIRST_VARIANT);
-                    let dest = adt_def.non_enum_variant().fields[field].ty(self.tcx, args);
-                    if fields.len() != 1 {
+                    let dest_ty = self.tcx.normalize_erasing_regions(
+                        self.param_env,
+                        adt_def.non_enum_variant().fields[field].ty(self.tcx, args),
+                    );
+                    if fields.len() == 1 {
+                        let src_ty = fields.raw[0].ty(self.body, self.tcx);
+                        if !self.mir_assign_valid_types(src_ty, dest_ty) {
+                            self.fail(location, "union field has the wrong type");
+                        }
+                    } else {
                         self.fail(location, "unions should have one initialized field");
                     }
-                    if !self.mir_assign_valid_types(fields.raw[0].ty(self.body, self.tcx), dest) {
-                        self.fail(location, "union field has the wrong type");
-                    }
                 }
                 AggregateKind::Adt(def_id, idx, args, _, None) => {
                     let adt_def = self.tcx.adt_def(def_id);
@@ -826,10 +831,10 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                         self.fail(location, "adt has the wrong number of initialized fields");
                     }
                     for (src, dest) in std::iter::zip(fields, &variant.fields) {
-                        if !self.mir_assign_valid_types(
-                            src.ty(self.body, self.tcx),
-                            dest.ty(self.tcx, args),
-                        ) {
+                        let dest_ty = self
+                            .tcx
+                            .normalize_erasing_regions(self.param_env, dest.ty(self.tcx, args));
+                        if !self.mir_assign_valid_types(src.ty(self.body, self.tcx), dest_ty) {
                             self.fail(location, "adt field has the wrong type");
                         }
                     }

compiler/rustc_mir_transform/src/coverage/mod.rs

@@ -329,7 +329,7 @@ fn make_code_region(
     start_line = source_map.doctest_offset_line(&file.name, start_line);
     end_line = source_map.doctest_offset_line(&file.name, end_line);
 
-    Some(CodeRegion {
+    check_code_region(CodeRegion {
         file_name,
         start_line: start_line as u32,
         start_col: start_col as u32,
@@ -338,6 +338,39 @@ fn make_code_region(
     })
 }
 
+/// If `llvm-cov` sees a code region that is improperly ordered (end < start),
+/// it will immediately exit with a fatal error. To prevent that from happening,
+/// discard regions that are improperly ordered, or might be interpreted in a
+/// way that makes them improperly ordered.
+fn check_code_region(code_region: CodeRegion) -> Option<CodeRegion> {
+    let CodeRegion { file_name: _, start_line, start_col, end_line, end_col } = code_region;
+
+    // Line/column coordinates are supposed to be 1-based. If we ever emit
+    // coordinates of 0, `llvm-cov` might misinterpret them.
+    let all_nonzero = [start_line, start_col, end_line, end_col].into_iter().all(|x| x != 0);
+    // Coverage mappings use the high bit of `end_col` to indicate that a
+    // region is actually a "gap" region, so make sure it's unset.
+    let end_col_has_high_bit_unset = (end_col & (1 << 31)) == 0;
+    // If a region is improperly ordered (end < start), `llvm-cov` will exit
+    // with a fatal error, which is inconvenient for users and hard to debug.
+    let is_ordered = (start_line, start_col) <= (end_line, end_col);
+
+    if all_nonzero && end_col_has_high_bit_unset && is_ordered {
+        Some(code_region)
+    } else {
+        debug!(
+            ?code_region,
+            ?all_nonzero,
+            ?end_col_has_high_bit_unset,
+            ?is_ordered,
+            "Skipping code region that would be misinterpreted or rejected by LLVM"
+        );
+        // If this happens in a debug build, ICE to make it easier to notice.
+        debug_assert!(false, "Improper code region: {code_region:?}");
+        None
+    }
+}
+
 fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
     // Only instrument functions, methods, and closures (not constants since they are evaluated
     // at compile time by Miri).
@@ -351,7 +384,18 @@ fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
         return false;
     }
 
+    // Don't instrument functions with `#[automatically_derived]` on their
+    // enclosing impl block, on the assumption that most users won't care about
+    // coverage for derived impls.
+    if let Some(impl_of) = tcx.impl_of_method(def_id.to_def_id())
+        && tcx.is_automatically_derived(impl_of)
+    {
+        trace!("InstrumentCoverage skipped for {def_id:?} (automatically derived)");
+        return false;
+    }
+
     if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::NO_COVERAGE) {
+        trace!("InstrumentCoverage skipped for {def_id:?} (`#[coverage(off)]`)");
         return false;
     }
 

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -3155,7 +3155,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                 } else {
                     // FIXME: we may suggest array::repeat instead
                     err.help("consider using `core::array::from_fn` to initialize the array");
-                    err.help("see https://doc.rust-lang.org/stable/std/array/fn.from_fn.html# for more information");
+                    err.help("see https://doc.rust-lang.org/stable/std/array/fn.from_fn.html for more information");
                 }
 
                 if self.tcx.sess.is_nightly_build()

src/bootstrap/src/core/build_steps/compile.rs

@@ -1113,16 +1113,11 @@ pub fn rustc_cargo_env(
 /// Pass down configuration from the LLVM build into the build of
 /// rustc_llvm and rustc_codegen_llvm.
 fn rustc_llvm_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelection) {
-    let target_config = builder.config.target_config.get(&target);
-
     if builder.is_rust_llvm(target) {
         cargo.env("LLVM_RUSTLLVM", "1");
     }
     let llvm::LlvmResult { llvm_config, .. } = builder.ensure(llvm::Llvm { target });
     cargo.env("LLVM_CONFIG", &llvm_config);
-    if let Some(s) = target_config.and_then(|c| c.llvm_config.as_ref()) {
-        cargo.env("CFG_LLVM_ROOT", s);
-    }
 
     // Some LLVM linker flags (-L and -l) may be needed to link `rustc_llvm`. Its build script
     // expects these to be passed via the `LLVM_LINKER_FLAGS` env variable, separated by

src/tools/compiletest/src/header.rs

@@ -1109,9 +1109,6 @@ fn ignore_lldb(config: &Config, line: &str) -> IgnoreDecision {
 }
 
 fn ignore_llvm(config: &Config, line: &str) -> IgnoreDecision {
-    if config.system_llvm && line.starts_with("no-system-llvm") {
-        return IgnoreDecision::Ignore { reason: "ignored when the system LLVM is used".into() };
-    }
     if let Some(needed_components) =
         config.parse_name_value_directive(line, "needs-llvm-components")
     {

src/tools/compiletest/src/header/tests.rs

@@ -242,15 +242,6 @@ fn aux_build() {
     );
 }
 
-#[test]
-fn no_system_llvm() {
-    let config: Config = cfg().system_llvm(false).build();
-    assert!(!check_ignore(&config, "// no-system-llvm"));
-
-    let config: Config = cfg().system_llvm(true).build();
-    assert!(check_ignore(&config, "// no-system-llvm"));
-}
-
 #[test]
 fn llvm_version() {
     let config: Config = cfg().llvm_version("8.1.2").build();
@@ -266,6 +257,18 @@ fn llvm_version() {
     assert!(!check_ignore(&config, "// min-llvm-version: 9.0"));
 }
 
+#[test]
+fn system_llvm_version() {
+    let config: Config = cfg().system_llvm(true).llvm_version("17.0.0").build();
+    assert!(check_ignore(&config, "// min-system-llvm-version: 18.0"));
+
+    let config: Config = cfg().system_llvm(true).llvm_version("18.0.0").build();
+    assert!(!check_ignore(&config, "// min-system-llvm-version: 18.0"));
+
+    let config: Config = cfg().llvm_version("17.0.0").build();
+    assert!(!check_ignore(&config, "// min-system-llvm-version: 18.0"));
+}
+
 #[test]
 fn ignore_target() {
     let config: Config = cfg().target("x86_64-unknown-linux-gnu").build();

src/tools/tidy/src/ui_tests.rs

@@ -24,9 +24,10 @@ const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[
 
 const EXTENSION_EXCEPTION_PATHS: &[&str] = &[
     "tests/ui/asm/named-asm-labels.s", // loading an external asm file to test named labels lint
-    "tests/ui/check-cfg/my-awesome-platform.json", // testing custom targets with cfgs
-    "tests/ui/commandline-argfile-badutf8.args", // passing args via a file
-    "tests/ui/commandline-argfile.args", // passing args via a file
+    "tests/ui/codegen/mismatched-data-layout.json", // testing mismatched data layout w/ custom targets
+    "tests/ui/check-cfg/my-awesome-platform.json",  // testing custom targets with cfgs
+    "tests/ui/commandline-argfile-badutf8.args",    // passing args via a file
+    "tests/ui/commandline-argfile.args",            // passing args via a file
     "tests/ui/crate-loading/auxiliary/libfoo.rlib", // testing loading a manually created rlib
     "tests/ui/include-macros/data.bin", // testing including data with the include macros
     "tests/ui/include-macros/file.txt", // testing including data with the include macros

tests/assembly/targets/targets-elf.rs

@@ -1,29 +1,5 @@
 // assembly-output: emit-asm
 // ignore-tidy-linelength
-// revisions: aarch64_apple_darwin
-// [aarch64_apple_darwin] compile-flags: --target aarch64-apple-darwin
-// [aarch64_apple_darwin] needs-llvm-components: aarch64
-// revisions: aarch64_apple_ios
-// [aarch64_apple_ios] compile-flags: --target aarch64-apple-ios
-// [aarch64_apple_ios] needs-llvm-components: aarch64
-// revisions: aarch64_apple_ios_macabi
-// [aarch64_apple_ios_macabi] compile-flags: --target aarch64-apple-ios-macabi
-// [aarch64_apple_ios_macabi] needs-llvm-components: aarch64
-// revisions: aarch64_apple_ios_sim
-// [aarch64_apple_ios_sim] compile-flags: --target aarch64-apple-ios-sim
-// [aarch64_apple_ios_sim] needs-llvm-components: aarch64
-// revisions: aarch64_apple_tvos
-// [aarch64_apple_tvos] compile-flags: --target aarch64-apple-tvos
-// [aarch64_apple_tvos] needs-llvm-components: aarch64
-// revisions: aarch64_apple_tvos_sim
-// [aarch64_apple_tvos_sim] compile-flags: --target aarch64-apple-tvos-sim
-// [aarch64_apple_tvos_sim] needs-llvm-components: aarch64
-// revisions: aarch64_apple_watchos
-// [aarch64_apple_watchos] compile-flags: --target aarch64-apple-watchos
-// [aarch64_apple_watchos] needs-llvm-components: aarch64
-// revisions: aarch64_apple_watchos_sim
-// [aarch64_apple_watchos_sim] compile-flags: --target aarch64-apple-watchos-sim
-// [aarch64_apple_watchos_sim] needs-llvm-components: aarch64
 // revisions: aarch64_be_unknown_linux_gnu
 // [aarch64_be_unknown_linux_gnu] compile-flags: --target aarch64_be-unknown-linux-gnu
 // [aarch64_be_unknown_linux_gnu] needs-llvm-components: aarch64
@@ -93,15 +69,6 @@
 // revisions: aarch64_wrs_vxworks
 // [aarch64_wrs_vxworks] compile-flags: --target aarch64-wrs-vxworks
 // [aarch64_wrs_vxworks] needs-llvm-components: aarch64
-// revisions: arm64_32_apple_watchos
-// [arm64_32_apple_watchos] compile-flags: --target arm64_32-apple-watchos
-// [arm64_32_apple_watchos] needs-llvm-components: aarch64
-// revisions: arm64e_apple_darwin
-// [arm64e_apple_darwin] compile-flags: --target arm64e-apple-darwin
-// [arm64e_apple_darwin] needs-llvm-components: aarch64
-// revisions: arm64e_apple_ios
-// [arm64e_apple_ios] compile-flags: --target arm64e-apple-ios
-// [arm64e_apple_ios] needs-llvm-components: aarch64
 // revisions: arm_linux_androideabi
 // [arm_linux_androideabi] compile-flags: --target arm-linux-androideabi
 // [arm_linux_androideabi] needs-llvm-components: arm
@@ -201,18 +168,12 @@
 // revisions: armv7a_none_eabihf
 // [armv7a_none_eabihf] compile-flags: --target armv7a-none-eabihf
 // [armv7a_none_eabihf] needs-llvm-components: arm
-// revisions: armv7k_apple_watchos
-// [armv7k_apple_watchos] compile-flags: --target armv7k-apple-watchos
-// [armv7k_apple_watchos] needs-llvm-components: arm
 // revisions: armv7r_none_eabi
 // [armv7r_none_eabi] compile-flags: --target armv7r-none-eabi
 // [armv7r_none_eabi] needs-llvm-components: arm
 // revisions: armv7r_none_eabihf
 // [armv7r_none_eabihf] compile-flags: --target armv7r-none-eabihf
 // [armv7r_none_eabihf] needs-llvm-components: arm
-// revisions: armv7s_apple_ios
-// [armv7s_apple_ios] compile-flags: --target armv7s-apple-ios
-// [armv7s_apple_ios] needs-llvm-components: arm
 // FIXME: disabled since it fails on CI saying the csky component is missing
 /*
     revisions: csky_unknown_linux_gnuabiv2
@@ -228,9 +189,6 @@
 // revisions: hexagon_unknown_none_elf
 // [hexagon_unknown_none_elf] compile-flags: --target hexagon-unknown-none-elf
 // [hexagon_unknown_none_elf] needs-llvm-components: hexagon
-// revisions: i386_apple_ios
-// [i386_apple_ios] compile-flags: --target i386-apple-ios
-// [i386_apple_ios] needs-llvm-components: x86
 // revisions: i586_pc_nto_qnx700
 // [i586_pc_nto_qnx700] compile-flags: --target i586-pc-nto-qnx700
 // [i586_pc_nto_qnx700] needs-llvm-components: x86
@@ -243,9 +201,6 @@
 // revisions: i586_unknown_netbsd
 // [i586_unknown_netbsd] compile-flags: --target i586-unknown-netbsd
 // [i586_unknown_netbsd] needs-llvm-components: x86
-// revisions: i686_apple_darwin
-// [i686_apple_darwin] compile-flags: --target i686-apple-darwin
-// [i686_apple_darwin] needs-llvm-components: x86
 // revisions: i686_linux_android
 // [i686_linux_android] compile-flags: --target i686-linux-android
 // [i686_linux_android] needs-llvm-components: x86
@@ -534,21 +489,6 @@
 // revisions: wasm64_unknown_unknown
 // [wasm64_unknown_unknown] compile-flags: --target wasm64-unknown-unknown
 // [wasm64_unknown_unknown] needs-llvm-components: webassembly
-// revisions: x86_64_apple_darwin
-// [x86_64_apple_darwin] compile-flags: --target x86_64-apple-darwin
-// [x86_64_apple_darwin] needs-llvm-components: x86
-// revisions: x86_64_apple_ios
-// [x86_64_apple_ios] compile-flags: --target x86_64-apple-ios
-// [x86_64_apple_ios] needs-llvm-components: x86
-// revisions: x86_64_apple_ios_macabi
-// [x86_64_apple_ios_macabi] compile-flags: --target x86_64-apple-ios-macabi
-// [x86_64_apple_ios_macabi] needs-llvm-components: x86
-// revisions: x86_64_apple_tvos
-// [x86_64_apple_tvos] compile-flags: --target x86_64-apple-tvos
-// [x86_64_apple_tvos] needs-llvm-components: x86
-// revisions: x86_64_apple_watchos_sim
-// [x86_64_apple_watchos_sim] compile-flags: --target x86_64-apple-watchos-sim
-// [x86_64_apple_watchos_sim] needs-llvm-components: x86
 // revisions: x86_64_fortanix_unknown_sgx
 // [x86_64_fortanix_unknown_sgx] compile-flags: --target x86_64-fortanix-unknown-sgx
 // [x86_64_fortanix_unknown_sgx] needs-llvm-components: x86
@@ -615,9 +555,6 @@
 // revisions: x86_64_wrs_vxworks
 // [x86_64_wrs_vxworks] compile-flags: --target x86_64-wrs-vxworks
 // [x86_64_wrs_vxworks] needs-llvm-components: x86
-// revisions: x86_64h_apple_darwin
-// [x86_64h_apple_darwin] compile-flags: --target x86_64h-apple-darwin
-// [x86_64h_apple_darwin] needs-llvm-components: x86
 
 // Sanity-check that each target can produce assembly code.
 
@@ -633,4 +570,4 @@ pub fn test() -> u8 {
     42
 }
 
-// CHECK: .section
+// CHECK: .text