Skip to content

Commit

Permalink
Rollup merge of #112454 - ferrocene:pa-compiletest-dynamic-linking, r…
Browse files Browse the repository at this point in the history
…=davidtwco

Make compiletest aware of targets without dynamic linking

Some parts of the compiletest internals and some tests require dynamic linking to work, which is not supported by all targets. Before this PR, this was handled by if branches matching on the target name.

This PR loads whether a target supports dynamic linking or not from the target spec, and adds a `// needs-dynamic-linking` attribute for tests that require it. Note that I was not able to replace all the old conditions based on the target name, as some targets have `dynamic_linking: true` in their spec but pretend they don't have it in compiletest.

Also, to get this to work I had to *partially* revert #111472 (cc `@djkoloski` `@tmandry` `@bjorn3).` On one hand, only the target spec contains whether a target supports dynamic linking, but on the other hand a subset of the fields can be overridden through `-C` flags (as far as I'm aware only `-C panic=$strategy`). The solution I came up with is to take the target spec as the base, and then override the panic strategy based on `--print=cfg`. Hopefully that should not break y'all again.
  • Loading branch information
matthiaskrgr authored Jun 27, 2023
2 parents 1880e83 + 767c4b9 commit 353dd71
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 66 deletions.
90 changes: 26 additions & 64 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ impl TargetCfgs {
let mut all_families = HashSet::new();
let mut all_pointer_widths = HashSet::new();

for (target, cfg) in targets.into_iter() {
for (target, cfg) in targets.iter() {
all_archs.insert(cfg.arch.clone());
all_oses.insert(cfg.os.clone());
all_oses_and_envs.insert(cfg.os_and_env());
Expand All @@ -464,11 +464,11 @@ impl TargetCfgs {
}
all_pointer_widths.insert(format!("{}bit", cfg.pointer_width));

all_targets.insert(target.into());
all_targets.insert(target.clone());
}

Self {
current: Self::get_current_target_config(config),
current: Self::get_current_target_config(config, &targets),
all_targets,
all_archs,
all_oses,
Expand All @@ -480,16 +480,20 @@ impl TargetCfgs {
}
}

fn get_current_target_config(config: &Config) -> TargetCfg {
let mut arch = None;
let mut os = None;
let mut env = None;
let mut abi = None;
let mut families = Vec::new();
let mut pointer_width = None;
let mut endian = None;
let mut panic = None;

fn get_current_target_config(
config: &Config,
targets: &HashMap<String, TargetCfg>,
) -> TargetCfg {
let mut cfg = targets[&config.target].clone();

// To get the target information for the current target, we take the target spec obtained
// from `--print=all-target-specs-json`, and then we enrich it with the information
// gathered from `--print=cfg --target=$target`.
//
// This is done because some parts of the target spec can be overridden with `-C` flags,
// which are respected for `--print=cfg` but not for `--print=all-target-specs-json`. The
// code below extracts them from `--print=cfg`: make sure to only override fields that can
// actually be changed with `-C` flags.
for config in
rustc_output(config, &["--print=cfg", "--target", &config.target]).trim().lines()
{
Expand All @@ -507,60 +511,16 @@ impl TargetCfgs {
})
.unwrap_or_else(|| (config, None));

match name {
"target_arch" => {
arch = Some(value.expect("target_arch should be a key-value pair").to_string());
}
"target_os" => {
os = Some(value.expect("target_os sould be a key-value pair").to_string());
}
"target_env" => {
env = Some(value.expect("target_env should be a key-value pair").to_string());
}
"target_abi" => {
abi = Some(value.expect("target_abi should be a key-value pair").to_string());
}
"target_family" => {
families
.push(value.expect("target_family should be a key-value pair").to_string());
}
"target_pointer_width" => {
pointer_width = Some(
value
.expect("target_pointer_width should be a key-value pair")
.parse::<u32>()
.expect("target_pointer_width should be a valid u32"),
);
}
"target_endian" => {
endian = Some(match value.expect("target_endian should be a key-value pair") {
"big" => Endian::Big,
"little" => Endian::Little,
_ => panic!("target_endian should be either 'big' or 'little'"),
});
}
"panic" => {
panic = Some(match value.expect("panic should be a key-value pair") {
"abort" => PanicStrategy::Abort,
"unwind" => PanicStrategy::Unwind,
_ => panic!("panic should be either 'abort' or 'unwind'"),
});
}
_ => (),
match (name, value) {
// Can be overridden with `-C panic=$strategy`.
("panic", Some("abort")) => cfg.panic = PanicStrategy::Abort,
("panic", Some("unwind")) => cfg.panic = PanicStrategy::Unwind,
("panic", other) => panic!("unexpected value for panic cfg: {other:?}"),
_ => {}
}
}

TargetCfg {
arch: arch.expect("target configuration should specify target_arch"),
os: os.expect("target configuration should specify target_os"),
env: env.expect("target configuration should specify target_env"),
abi: abi.expect("target configuration should specify target_abi"),
families,
pointer_width: pointer_width
.expect("target configuration should specify target_pointer_width"),
endian: endian.expect("target configuration should specify target_endian"),
panic: panic.expect("target configuration should specify panic"),
}
cfg
}
}

Expand All @@ -582,6 +542,8 @@ pub struct TargetCfg {
endian: Endian,
#[serde(rename = "panic-strategy", default)]
pub(crate) panic: PanicStrategy,
#[serde(default)]
pub(crate) dynamic_linking: bool,
}

impl TargetCfg {
Expand Down
5 changes: 5 additions & 0 deletions src/tools/compiletest/src/header/needs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ pub(super) fn handle_needs(
condition: config.git_hash,
ignore_reason: "ignored when git hashes have been omitted for building",
},
Need {
name: "needs-dynamic-linking",
condition: config.target_cfg().dynamic_linking,
ignore_reason: "ignored on targets without dynamic linking",
},
];

let (name, comment) = match ln.split_once([':', ' ']) {
Expand Down
2 changes: 1 addition & 1 deletion src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1810,8 +1810,8 @@ impl<'test> TestCx<'test> {
|| self.config.target.contains("wasm32")
|| self.config.target.contains("nvptx")
|| self.is_vxworks_pure_static()
|| self.config.target.contains("sgx")
|| self.config.target.contains("bpf")
|| !self.config.target_cfg().dynamic_linking
{
// We primarily compile all auxiliary libraries as dynamic libraries
// to avoid code size bloat and large binaries as much as possible
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/issues/issue-12133-3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// aux-build:issue-12133-dylib2.rs
// ignore-emscripten no dylib support
// ignore-musl
// ignore-sgx no dylib support
// needs-dynamic-linking

// pretty-expanded FIXME #23616

Expand Down
1 change: 1 addition & 0 deletions tests/ui/issues/issue-85461.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// compile-flags: -Cinstrument-coverage -Ccodegen-units=4 --crate-type dylib -Copt-level=0
// build-pass
// needs-profiler-support
// needs-dynamic-linking

// Regression test for #85461 where MSVC sometimes fails to link instrument-coverage binaries
// with dead code and #[inline(always)].
Expand Down
1 change: 1 addition & 0 deletions tests/ui/proc-macro/crt-static.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
// build-pass
// force-host
// no-prefer-dynamic
// needs-dynamic-linking

#![crate_type = "proc-macro"]

Expand Down

0 comments on commit 353dd71

Please sign in to comment.