From 46652dd254bc9c413d14cec637c7f21ac6601e05 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 17 Jan 2024 14:26:26 +0000 Subject: [PATCH] llvm: simplify data layout check Don't skip the inconsistent data layout check for custom LLVMs. With #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 --- compiler/rustc_codegen_llvm/messages.ftl | 3 ++ compiler/rustc_codegen_llvm/src/context.rs | 38 +++++-------------- compiler/rustc_codegen_llvm/src/errors.rs | 9 +++++ src/bootstrap/src/core/build_steps/compile.rs | 5 --- src/tools/tidy/src/ui_tests.rs | 7 ++-- tests/run-make/target-specs/Makefile | 2 +- tests/ui/codegen/mismatched-data-layout.json | 13 +++++++ tests/ui/codegen/mismatched-data-layouts.rs | 14 +++++++ .../ui/codegen/mismatched-data-layouts.stderr | 4 ++ 9 files changed, 57 insertions(+), 38 deletions(-) create mode 100644 tests/ui/codegen/mismatched-data-layout.json create mode 100644 tests/ui/codegen/mismatched-data-layouts.rs create mode 100644 tests/ui/codegen/mismatched-data-layouts.stderr diff --git a/compiler/rustc_codegen_llvm/messages.ftl b/compiler/rustc_codegen_llvm/messages.ftl index 7a86ddc7556..d5bc04f594d 100644 --- a/compiler/rustc_codegen_llvm/messages.ftl +++ b/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 diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index 1d1b6e6148d..b0f0b650e77 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -34,6 +34,7 @@ use smallvec::SmallVec; use libc::c_uint; +use std::borrow::Borrow; use std::cell::{Cell, RefCell}; use std::ffi::CStr; use std::str; @@ -147,8 +148,7 @@ 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); @@ -156,33 +156,13 @@ pub unsafe fn create_module<'ll>( 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, + }); } } diff --git a/compiler/rustc_codegen_llvm/src/errors.rs b/compiler/rustc_codegen_llvm/src/errors.rs index 697ce602298..d82ff6656f4 100644 --- a/compiler/rustc_codegen_llvm/src/errors.rs +++ b/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, +} diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 190f0fe3cdd..663ec6fe086 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -1103,16 +1103,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 diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index 95cf9f8cb13..85553d2e338 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -24,9 +24,10 @@ 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 diff --git a/tests/run-make/target-specs/Makefile b/tests/run-make/target-specs/Makefile index 62d5365a73d..161b6602185 100644 --- a/tests/run-make/target-specs/Makefile +++ b/tests/run-make/target-specs/Makefile @@ -9,4 +9,4 @@ all: $(RUSTC) -Z unstable-options --target=my-awesome-platform.json --print target-spec-json > $(TMPDIR)/test-platform.json && $(RUSTC) -Z unstable-options --target=$(TMPDIR)/test-platform.json --print target-spec-json | diff -q $(TMPDIR)/test-platform.json - $(RUSTC) foo.rs --target=definitely-not-builtin-target 2>&1 | $(CGREP) 'may not set is_builtin' $(RUSTC) foo.rs --target=endianness-mismatch 2>&1 | $(CGREP) '"data-layout" claims architecture is little-endian' - $(RUSTC) foo.rs --target=mismatching-data-layout --crate-type=lib + $(RUSTC) foo.rs --target=mismatching-data-layout --crate-type=lib 2>&1 | $(CGREP) 'data-layout for target' diff --git a/tests/ui/codegen/mismatched-data-layout.json b/tests/ui/codegen/mismatched-data-layout.json new file mode 100644 index 00000000000..4cb0602dc75 --- /dev/null +++ b/tests/ui/codegen/mismatched-data-layout.json @@ -0,0 +1,13 @@ +{ + "llvm-target": "x86_64-unknown-none-gnu", + "data-layout": "e-m:e-i64:64-f80:128-n8:16:32:64-S128", + "arch": "x86_64", + "target-endian": "little", + "target-pointer-width": "64", + "target-c-int-width": "32", + "os": "unknown", + "linker-flavor": "ld.lld", + "linker": "rust-lld", + "executables": true +} + diff --git a/tests/ui/codegen/mismatched-data-layouts.rs b/tests/ui/codegen/mismatched-data-layouts.rs new file mode 100644 index 00000000000..047ec155fdc --- /dev/null +++ b/tests/ui/codegen/mismatched-data-layouts.rs @@ -0,0 +1,14 @@ +// This test checks that data layout mismatches emit an error. +// +// build-fail +// needs-llvm-components: x86 +// compile-flags: --crate-type=lib --target={{src-base}}/codegen/mismatched-data-layout.json -Z unstable-options +// error-pattern: differs from LLVM target's +// normalize-stderr-test: "`, `[A-Za-z0-9-:]*`" -> "`, `normalized data layout`" +// normalize-stderr-test: "layout, `[A-Za-z0-9-:]*`" -> "layout, `normalized data layout`" + +#![feature(lang_items, no_core, auto_traits)] +#![no_core] + +#[lang = "sized"] +trait Sized {} diff --git a/tests/ui/codegen/mismatched-data-layouts.stderr b/tests/ui/codegen/mismatched-data-layouts.stderr new file mode 100644 index 00000000000..1fe242266df --- /dev/null +++ b/tests/ui/codegen/mismatched-data-layouts.stderr @@ -0,0 +1,4 @@ +error: data-layout for target `mismatched-data-layout-7814813422914914169`, `normalized data layout`, differs from LLVM target's `x86_64-unknown-none-gnu` default layout, `normalized data layout` + +error: aborting due to 1 previous error +