Skip to content

Commit 56ebadf

Browse files
borsbors
committed
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
2 parents 0b77301 + 4727f25 commit 56ebadf

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+256
-253
lines changed

‎compiler/rustc_codegen_llvm/messages.ftl

+3
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ codegen_llvm_lto_dylib = lto cannot be used for `dylib` crate type without `-Zdy
3939
4040
codegen_llvm_lto_proc_macro = lto cannot be used for `proc-macro` crate type without `-Zdylib-lto`
4141
42+
codegen_llvm_mismatch_data_layout =
43+
data-layout for target `{$rustc_target}`, `{$rustc_layout}`, differs from LLVM target's `{$llvm_target}` default layout, `{$llvm_layout}`
44+
4245
codegen_llvm_missing_features =
4346
add the missing features in a `target_feature` attribute
4447

‎compiler/rustc_codegen_llvm/src/context.rs

+9-29
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ use rustc_target::spec::{HasTargetSpec, RelocModel, Target, TlsModel};
3434
use smallvec::SmallVec;
3535

3636
use libc::c_uint;
37+
use std::borrow::Borrow;
3738
use std::cell::{Cell, RefCell};
3839
use std::ffi::CStr;
3940
use std::str;
@@ -155,42 +156,21 @@ pub unsafe fn create_module<'ll>(
155156
}
156157

157158
// Ensure the data-layout values hardcoded remain the defaults.
158-
if sess.target.is_builtin {
159-
// tm is disposed by its drop impl
159+
{
160160
let tm = crate::back::write::create_informational_target_machine(tcx.sess);
161161
llvm::LLVMRustSetDataLayoutFromTargetMachine(llmod, &tm);
162162

163163
let llvm_data_layout = llvm::LLVMGetDataLayoutStr(llmod);
164164
let llvm_data_layout = str::from_utf8(CStr::from_ptr(llvm_data_layout).to_bytes())
165165
.expect("got a non-UTF8 data-layout from LLVM");
166166

167-
// Unfortunately LLVM target specs change over time, and right now we
168-
// don't have proper support to work with any more than one
169-
// `data_layout` than the one that is in the rust-lang/rust repo. If
170-
// this compiler is configured against a custom LLVM, we may have a
171-
// differing data layout, even though we should update our own to use
172-
// that one.
173-
//
174-
// As an interim hack, if CFG_LLVM_ROOT is not an empty string then we
175-
// disable this check entirely as we may be configured with something
176-
// that has a different target layout.
177-
//
178-
// Unsure if this will actually cause breakage when rustc is configured
179-
// as such.
180-
//
181-
// FIXME(#34960)
182-
let cfg_llvm_root = option_env!("CFG_LLVM_ROOT").unwrap_or("");
183-
let custom_llvm_used = !cfg_llvm_root.trim().is_empty();
184-
185-
if !custom_llvm_used && target_data_layout != llvm_data_layout {
186-
bug!(
187-
"data-layout for target `{rustc_target}`, `{rustc_layout}`, \
188-
differs from LLVM target's `{llvm_target}` default layout, `{llvm_layout}`",
189-
rustc_target = sess.opts.target_triple,
190-
rustc_layout = target_data_layout,
191-
llvm_target = sess.target.llvm_target,
192-
llvm_layout = llvm_data_layout
193-
);
167+
if target_data_layout != llvm_data_layout {
168+
tcx.dcx().emit_err(crate::errors::MismatchedDataLayout {
169+
rustc_target: sess.opts.target_triple.to_string().as_str(),
170+
rustc_layout: target_data_layout.as_str(),
171+
llvm_target: sess.target.llvm_target.borrow(),
172+
llvm_layout: llvm_data_layout,
173+
});
194174
}
195175
}
196176

‎compiler/rustc_codegen_llvm/src/errors.rs

+9
Original file line numberDiff line numberDiff line change
@@ -244,3 +244,12 @@ pub(crate) struct CopyBitcode {
244244
pub struct UnknownCompression {
245245
pub algorithm: &'static str,
246246
}
247+
248+
#[derive(Diagnostic)]
249+
#[diag(codegen_llvm_mismatch_data_layout)]
250+
pub struct MismatchedDataLayout<'a> {
251+
pub rustc_target: &'a str,
252+
pub rustc_layout: &'a str,
253+
pub llvm_target: &'a str,
254+
pub llvm_layout: &'a str,
255+
}

‎compiler/rustc_const_eval/src/transform/validate.rs

+14-9
Original file line numberDiff line numberDiff line change
@@ -810,13 +810,18 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
810810
let adt_def = self.tcx.adt_def(def_id);
811811
assert!(adt_def.is_union());
812812
assert_eq!(idx, FIRST_VARIANT);
813-
let dest = adt_def.non_enum_variant().fields[field].ty(self.tcx, args);
814-
if fields.len() != 1 {
813+
let dest_ty = self.tcx.normalize_erasing_regions(
814+
self.param_env,
815+
adt_def.non_enum_variant().fields[field].ty(self.tcx, args),
816+
);
817+
if fields.len() == 1 {
818+
let src_ty = fields.raw[0].ty(self.body, self.tcx);
819+
if !self.mir_assign_valid_types(src_ty, dest_ty) {
820+
self.fail(location, "union field has the wrong type");
821+
}
822+
} else {
815823
self.fail(location, "unions should have one initialized field");
816824
}
817-
if !self.mir_assign_valid_types(fields.raw[0].ty(self.body, self.tcx), dest) {
818-
self.fail(location, "union field has the wrong type");
819-
}
820825
}
821826
AggregateKind::Adt(def_id, idx, args, _, None) => {
822827
let adt_def = self.tcx.adt_def(def_id);
@@ -826,10 +831,10 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
826831
self.fail(location, "adt has the wrong number of initialized fields");
827832
}
828833
for (src, dest) in std::iter::zip(fields, &variant.fields) {
829-
if !self.mir_assign_valid_types(
830-
src.ty(self.body, self.tcx),
831-
dest.ty(self.tcx, args),
832-
) {
834+
let dest_ty = self
835+
.tcx
836+
.normalize_erasing_regions(self.param_env, dest.ty(self.tcx, args));
837+
if !self.mir_assign_valid_types(src.ty(self.body, self.tcx), dest_ty) {
833838
self.fail(location, "adt field has the wrong type");
834839
}
835840
}

‎compiler/rustc_mir_transform/src/coverage/mod.rs

+45-1
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ fn make_code_region(
329329
start_line = source_map.doctest_offset_line(&file.name, start_line);
330330
end_line = source_map.doctest_offset_line(&file.name, end_line);
331331

332-
Some(CodeRegion {
332+
check_code_region(CodeRegion {
333333
file_name,
334334
start_line: start_line as u32,
335335
start_col: start_col as u32,
@@ -338,6 +338,39 @@ fn make_code_region(
338338
})
339339
}
340340

341+
/// If `llvm-cov` sees a code region that is improperly ordered (end < start),
342+
/// it will immediately exit with a fatal error. To prevent that from happening,
343+
/// discard regions that are improperly ordered, or might be interpreted in a
344+
/// way that makes them improperly ordered.
345+
fn check_code_region(code_region: CodeRegion) -> Option<CodeRegion> {
346+
let CodeRegion { file_name: _, start_line, start_col, end_line, end_col } = code_region;
347+
348+
// Line/column coordinates are supposed to be 1-based. If we ever emit
349+
// coordinates of 0, `llvm-cov` might misinterpret them.
350+
let all_nonzero = [start_line, start_col, end_line, end_col].into_iter().all(|x| x != 0);
351+
// Coverage mappings use the high bit of `end_col` to indicate that a
352+
// region is actually a "gap" region, so make sure it's unset.
353+
let end_col_has_high_bit_unset = (end_col & (1 << 31)) == 0;
354+
// If a region is improperly ordered (end < start), `llvm-cov` will exit
355+
// with a fatal error, which is inconvenient for users and hard to debug.
356+
let is_ordered = (start_line, start_col) <= (end_line, end_col);
357+
358+
if all_nonzero && end_col_has_high_bit_unset && is_ordered {
359+
Some(code_region)
360+
} else {
361+
debug!(
362+
?code_region,
363+
?all_nonzero,
364+
?end_col_has_high_bit_unset,
365+
?is_ordered,
366+
"Skipping code region that would be misinterpreted or rejected by LLVM"
367+
);
368+
// If this happens in a debug build, ICE to make it easier to notice.
369+
debug_assert!(false, "Improper code region: {code_region:?}");
370+
None
371+
}
372+
}
373+
341374
fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
342375
// Only instrument functions, methods, and closures (not constants since they are evaluated
343376
// at compile time by Miri).
@@ -351,7 +384,18 @@ fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
351384
return false;
352385
}
353386

387+
// Don't instrument functions with `#[automatically_derived]` on their
388+
// enclosing impl block, on the assumption that most users won't care about
389+
// coverage for derived impls.
390+
if let Some(impl_of) = tcx.impl_of_method(def_id.to_def_id())
391+
&& tcx.is_automatically_derived(impl_of)
392+
{
393+
trace!("InstrumentCoverage skipped for {def_id:?} (automatically derived)");
394+
return false;
395+
}
396+
354397
if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::NO_COVERAGE) {
398+
trace!("InstrumentCoverage skipped for {def_id:?} (`#[coverage(off)]`)");
355399
return false;
356400
}
357401

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -3155,7 +3155,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
31553155
} else {
31563156
// FIXME: we may suggest array::repeat instead
31573157
err.help("consider using `core::array::from_fn` to initialize the array");
3158-
err.help("see https://doc.rust-lang.org/stable/std/array/fn.from_fn.html# for more information");
3158+
err.help("see https://doc.rust-lang.org/stable/std/array/fn.from_fn.html for more information");
31593159
}
31603160

31613161
if self.tcx.sess.is_nightly_build()

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

-5
Original file line numberDiff line numberDiff line change
@@ -1113,16 +1113,11 @@ pub fn rustc_cargo_env(
11131113
/// Pass down configuration from the LLVM build into the build of
11141114
/// rustc_llvm and rustc_codegen_llvm.
11151115
fn rustc_llvm_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelection) {
1116-
let target_config = builder.config.target_config.get(&target);
1117-
11181116
if builder.is_rust_llvm(target) {
11191117
cargo.env("LLVM_RUSTLLVM", "1");
11201118
}
11211119
let llvm::LlvmResult { llvm_config, .. } = builder.ensure(llvm::Llvm { target });
11221120
cargo.env("LLVM_CONFIG", &llvm_config);
1123-
if let Some(s) = target_config.and_then(|c| c.llvm_config.as_ref()) {
1124-
cargo.env("CFG_LLVM_ROOT", s);
1125-
}
11261121

11271122
// Some LLVM linker flags (-L and -l) may be needed to link `rustc_llvm`. Its build script
11281123
// expects these to be passed via the `LLVM_LINKER_FLAGS` env variable, separated by

‎src/tools/compiletest/src/header.rs

-3
Original file line numberDiff line numberDiff line change
@@ -1109,9 +1109,6 @@ fn ignore_lldb(config: &Config, line: &str) -> IgnoreDecision {
11091109
}
11101110

11111111
fn ignore_llvm(config: &Config, line: &str) -> IgnoreDecision {
1112-
if config.system_llvm && line.starts_with("no-system-llvm") {
1113-
return IgnoreDecision::Ignore { reason: "ignored when the system LLVM is used".into() };
1114-
}
11151112
if let Some(needed_components) =
11161113
config.parse_name_value_directive(line, "needs-llvm-components")
11171114
{

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

+12-9
Original file line numberDiff line numberDiff line change
@@ -242,15 +242,6 @@ fn aux_build() {
242242
);
243243
}
244244

245-
#[test]
246-
fn no_system_llvm() {
247-
let config: Config = cfg().system_llvm(false).build();
248-
assert!(!check_ignore(&config, "// no-system-llvm"));
249-
250-
let config: Config = cfg().system_llvm(true).build();
251-
assert!(check_ignore(&config, "// no-system-llvm"));
252-
}
253-
254245
#[test]
255246
fn llvm_version() {
256247
let config: Config = cfg().llvm_version("8.1.2").build();
@@ -266,6 +257,18 @@ fn llvm_version() {
266257
assert!(!check_ignore(&config, "// min-llvm-version: 9.0"));
267258
}
268259

260+
#[test]
261+
fn system_llvm_version() {
262+
let config: Config = cfg().system_llvm(true).llvm_version("17.0.0").build();
263+
assert!(check_ignore(&config, "// min-system-llvm-version: 18.0"));
264+
265+
let config: Config = cfg().system_llvm(true).llvm_version("18.0.0").build();
266+
assert!(!check_ignore(&config, "// min-system-llvm-version: 18.0"));
267+
268+
let config: Config = cfg().llvm_version("17.0.0").build();
269+
assert!(!check_ignore(&config, "// min-system-llvm-version: 18.0"));
270+
}
271+
269272
#[test]
270273
fn ignore_target() {
271274
let config: Config = cfg().target("x86_64-unknown-linux-gnu").build();

‎src/tools/tidy/src/ui_tests.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,10 @@ const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[
2424

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

‎tests/assembly/targets/targets-elf.rs

+1-64
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,5 @@
11
// assembly-output: emit-asm
22
// ignore-tidy-linelength
3-
// revisions: aarch64_apple_darwin
4-
// [aarch64_apple_darwin] compile-flags: --target aarch64-apple-darwin
5-
// [aarch64_apple_darwin] needs-llvm-components: aarch64
6-
// revisions: aarch64_apple_ios
7-
// [aarch64_apple_ios] compile-flags: --target aarch64-apple-ios
8-
// [aarch64_apple_ios] needs-llvm-components: aarch64
9-
// revisions: aarch64_apple_ios_macabi
10-
// [aarch64_apple_ios_macabi] compile-flags: --target aarch64-apple-ios-macabi
11-
// [aarch64_apple_ios_macabi] needs-llvm-components: aarch64
12-
// revisions: aarch64_apple_ios_sim
13-
// [aarch64_apple_ios_sim] compile-flags: --target aarch64-apple-ios-sim
14-
// [aarch64_apple_ios_sim] needs-llvm-components: aarch64
15-
// revisions: aarch64_apple_tvos
16-
// [aarch64_apple_tvos] compile-flags: --target aarch64-apple-tvos
17-
// [aarch64_apple_tvos] needs-llvm-components: aarch64
18-
// revisions: aarch64_apple_tvos_sim
19-
// [aarch64_apple_tvos_sim] compile-flags: --target aarch64-apple-tvos-sim
20-
// [aarch64_apple_tvos_sim] needs-llvm-components: aarch64
21-
// revisions: aarch64_apple_watchos
22-
// [aarch64_apple_watchos] compile-flags: --target aarch64-apple-watchos
23-
// [aarch64_apple_watchos] needs-llvm-components: aarch64
24-
// revisions: aarch64_apple_watchos_sim
25-
// [aarch64_apple_watchos_sim] compile-flags: --target aarch64-apple-watchos-sim
26-
// [aarch64_apple_watchos_sim] needs-llvm-components: aarch64
273
// revisions: aarch64_be_unknown_linux_gnu
284
// [aarch64_be_unknown_linux_gnu] compile-flags: --target aarch64_be-unknown-linux-gnu
295
// [aarch64_be_unknown_linux_gnu] needs-llvm-components: aarch64
@@ -93,15 +69,6 @@
9369
// revisions: aarch64_wrs_vxworks
9470
// [aarch64_wrs_vxworks] compile-flags: --target aarch64-wrs-vxworks
9571
// [aarch64_wrs_vxworks] needs-llvm-components: aarch64
96-
// revisions: arm64_32_apple_watchos
97-
// [arm64_32_apple_watchos] compile-flags: --target arm64_32-apple-watchos
98-
// [arm64_32_apple_watchos] needs-llvm-components: aarch64
99-
// revisions: arm64e_apple_darwin
100-
// [arm64e_apple_darwin] compile-flags: --target arm64e-apple-darwin
101-
// [arm64e_apple_darwin] needs-llvm-components: aarch64
102-
// revisions: arm64e_apple_ios
103-
// [arm64e_apple_ios] compile-flags: --target arm64e-apple-ios
104-
// [arm64e_apple_ios] needs-llvm-components: aarch64
10572
// revisions: arm_linux_androideabi
10673
// [arm_linux_androideabi] compile-flags: --target arm-linux-androideabi
10774
// [arm_linux_androideabi] needs-llvm-components: arm
@@ -201,18 +168,12 @@
201168
// revisions: armv7a_none_eabihf
202169
// [armv7a_none_eabihf] compile-flags: --target armv7a-none-eabihf
203170
// [armv7a_none_eabihf] needs-llvm-components: arm
204-
// revisions: armv7k_apple_watchos
205-
// [armv7k_apple_watchos] compile-flags: --target armv7k-apple-watchos
206-
// [armv7k_apple_watchos] needs-llvm-components: arm
207171
// revisions: armv7r_none_eabi
208172
// [armv7r_none_eabi] compile-flags: --target armv7r-none-eabi
209173
// [armv7r_none_eabi] needs-llvm-components: arm
210174
// revisions: armv7r_none_eabihf
211175
// [armv7r_none_eabihf] compile-flags: --target armv7r-none-eabihf
212176
// [armv7r_none_eabihf] needs-llvm-components: arm
213-
// revisions: armv7s_apple_ios
214-
// [armv7s_apple_ios] compile-flags: --target armv7s-apple-ios
215-
// [armv7s_apple_ios] needs-llvm-components: arm
216177
// FIXME: disabled since it fails on CI saying the csky component is missing
217178
/*
218179
revisions: csky_unknown_linux_gnuabiv2
@@ -228,9 +189,6 @@
228189
// revisions: hexagon_unknown_none_elf
229190
// [hexagon_unknown_none_elf] compile-flags: --target hexagon-unknown-none-elf
230191
// [hexagon_unknown_none_elf] needs-llvm-components: hexagon
231-
// revisions: i386_apple_ios
232-
// [i386_apple_ios] compile-flags: --target i386-apple-ios
233-
// [i386_apple_ios] needs-llvm-components: x86
234192
// revisions: i586_pc_nto_qnx700
235193
// [i586_pc_nto_qnx700] compile-flags: --target i586-pc-nto-qnx700
236194
// [i586_pc_nto_qnx700] needs-llvm-components: x86
@@ -243,9 +201,6 @@
243201
// revisions: i586_unknown_netbsd
244202
// [i586_unknown_netbsd] compile-flags: --target i586-unknown-netbsd
245203
// [i586_unknown_netbsd] needs-llvm-components: x86
246-
// revisions: i686_apple_darwin
247-
// [i686_apple_darwin] compile-flags: --target i686-apple-darwin
248-
// [i686_apple_darwin] needs-llvm-components: x86
249204
// revisions: i686_linux_android
250205
// [i686_linux_android] compile-flags: --target i686-linux-android
251206
// [i686_linux_android] needs-llvm-components: x86
@@ -534,21 +489,6 @@
534489
// revisions: wasm64_unknown_unknown
535490
// [wasm64_unknown_unknown] compile-flags: --target wasm64-unknown-unknown
536491
// [wasm64_unknown_unknown] needs-llvm-components: webassembly
537-
// revisions: x86_64_apple_darwin
538-
// [x86_64_apple_darwin] compile-flags: --target x86_64-apple-darwin
539-
// [x86_64_apple_darwin] needs-llvm-components: x86
540-
// revisions: x86_64_apple_ios
541-
// [x86_64_apple_ios] compile-flags: --target x86_64-apple-ios
542-
// [x86_64_apple_ios] needs-llvm-components: x86
543-
// revisions: x86_64_apple_ios_macabi
544-
// [x86_64_apple_ios_macabi] compile-flags: --target x86_64-apple-ios-macabi
545-
// [x86_64_apple_ios_macabi] needs-llvm-components: x86
546-
// revisions: x86_64_apple_tvos
547-
// [x86_64_apple_tvos] compile-flags: --target x86_64-apple-tvos
548-
// [x86_64_apple_tvos] needs-llvm-components: x86
549-
// revisions: x86_64_apple_watchos_sim
550-
// [x86_64_apple_watchos_sim] compile-flags: --target x86_64-apple-watchos-sim
551-
// [x86_64_apple_watchos_sim] needs-llvm-components: x86
552492
// revisions: x86_64_fortanix_unknown_sgx
553493
// [x86_64_fortanix_unknown_sgx] compile-flags: --target x86_64-fortanix-unknown-sgx
554494
// [x86_64_fortanix_unknown_sgx] needs-llvm-components: x86
@@ -615,9 +555,6 @@
615555
// revisions: x86_64_wrs_vxworks
616556
// [x86_64_wrs_vxworks] compile-flags: --target x86_64-wrs-vxworks
617557
// [x86_64_wrs_vxworks] needs-llvm-components: x86
618-
// revisions: x86_64h_apple_darwin
619-
// [x86_64h_apple_darwin] compile-flags: --target x86_64h-apple-darwin
620-
// [x86_64h_apple_darwin] needs-llvm-components: x86
621558

622559
// Sanity-check that each target can produce assembly code.
623560

@@ -633,4 +570,4 @@ pub fn test() -> u8 {
633570
42
634571
}
635572

636-
// CHECK: .section
573+
// CHECK: .text

0 commit comments

Comments
 (0)