Skip to content

Commit 3ef83e2

Browse files
RalfJungRalfJung
committed
ensure JSON-defined targets are consistent
1 parent dff3e7c commit 3ef83e2

File tree

4 files changed

+335
-190
lines changed

4 files changed

+335
-190
lines changed

‎compiler/rustc_target/src/spec/mod.rs

+333-3
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434
//! the target's settings, though `target-feature` and `link-args` will *add*
3535
//! to the list specified by the target, rather than replace.
3636
37+
// ignore-tidy-filelength
38+
3739
use std::borrow::Cow;
3840
use std::collections::BTreeMap;
3941
use std::hash::{Hash, Hasher};
@@ -1605,13 +1607,11 @@ macro_rules! supported_targets {
16051607

16061608
#[cfg(test)]
16071609
mod tests {
1608-
mod tests_impl;
1609-
16101610
// Cannot put this into a separate file without duplication, make an exception.
16111611
$(
16121612
#[test] // `#[test]`
16131613
fn $module() {
1614-
tests_impl::test_target(crate::spec::targets::$module::target());
1614+
crate::spec::targets::$module::target().test_target()
16151615
}
16161616
)+
16171617
}
@@ -1998,6 +1998,14 @@ impl TargetWarnings {
19981998
}
19991999
}
20002000

2001+
/// For the [`Target::check_consistency`] function, determines whether the given target is a builtin or a JSON
2002+
/// target.
2003+
#[derive(Copy, Clone, Debug, PartialEq)]
2004+
enum TargetKind {
2005+
Json,
2006+
Builtin,
2007+
}
2008+
20012009
/// Everything `rustc` knows about how to compile for a specific target.
20022010
///
20032011
/// Every field here must be specified, and has no default value.
@@ -2846,6 +2854,327 @@ impl Target {
28462854
self.max_atomic_width.unwrap_or_else(|| self.pointer_width.into())
28472855
}
28482856

2857+
fn check_consistency(&self, kind: TargetKind) -> Result<(), String> {
2858+
macro_rules! check {
2859+
($b:expr, $($msg:tt)*) => {
2860+
if !$b {
2861+
return Err(format!($($msg)*));
2862+
}
2863+
}
2864+
}
2865+
macro_rules! check_eq {
2866+
($left:expr, $right:expr, $($msg:tt)*) => {
2867+
if ($left) != ($right) {
2868+
return Err(format!($($msg)*));
2869+
}
2870+
}
2871+
}
2872+
macro_rules! check_ne {
2873+
($left:expr, $right:expr, $($msg:tt)*) => {
2874+
if ($left) == ($right) {
2875+
return Err(format!($($msg)*));
2876+
}
2877+
}
2878+
}
2879+
macro_rules! check_matches {
2880+
($left:expr, $right:pat, $($msg:tt)*) => {
2881+
if !matches!($left, $right) {
2882+
return Err(format!($($msg)*));
2883+
}
2884+
}
2885+
}
2886+
2887+
check_eq!(
2888+
self.is_like_osx,
2889+
self.vendor == "apple",
2890+
"`is_like_osx` must be set if and only if `vendor` is `apple`"
2891+
);
2892+
check_eq!(
2893+
self.is_like_solaris,
2894+
self.os == "solaris" || self.os == "illumos",
2895+
"`is_like_solaris` must be set if and only if `os` is `solaris` or `illumos`"
2896+
);
2897+
check_eq!(
2898+
self.is_like_windows,
2899+
self.os == "windows" || self.os == "uefi",
2900+
"`is_like_windows` must be set if and only if `os` is `windows` or `uefi`"
2901+
);
2902+
check_eq!(
2903+
self.is_like_wasm,
2904+
self.arch == "wasm32" || self.arch == "wasm64",
2905+
"`is_like_wasm` must be set if and only if `arch` is `wasm32` or `wasm64`"
2906+
);
2907+
if self.is_like_msvc {
2908+
check!(self.is_like_windows, "if `is_like_msvc` is set, `is_like_windows` must be set");
2909+
}
2910+
if self.os == "emscripten" {
2911+
check!(self.is_like_wasm, "the `emcscripten` os only makes sense on wasm-like targets");
2912+
}
2913+
2914+
// Check that default linker flavor is compatible with some other key properties.
2915+
check_eq!(
2916+
self.is_like_osx,
2917+
matches!(self.linker_flavor, LinkerFlavor::Darwin(..)),
2918+
"`linker_flavor` must be `darwin` if and only if `is_like_osx` is set"
2919+
);
2920+
check_eq!(
2921+
self.is_like_msvc,
2922+
matches!(self.linker_flavor, LinkerFlavor::Msvc(..)),
2923+
"`linker_flavor` must be `msvc` if and only if `is_like_msvc` is set"
2924+
);
2925+
check_eq!(
2926+
self.is_like_wasm && self.os != "emscripten",
2927+
matches!(self.linker_flavor, LinkerFlavor::WasmLld(..)),
2928+
"`linker_flavor` must be `wasm-lld` if and only if `is_like_wasm` is set and the `os` is not `emscripten`",
2929+
);
2930+
check_eq!(
2931+
self.os == "emscripten",
2932+
matches!(self.linker_flavor, LinkerFlavor::EmCc),
2933+
"`linker_flavor` must be `em-cc` if and only if `os` is `emscripten`"
2934+
);
2935+
check_eq!(
2936+
self.arch == "bpf",
2937+
matches!(self.linker_flavor, LinkerFlavor::Bpf),
2938+
"`linker_flavor` must be `bpf` if and only if `arch` is `bpf`"
2939+
);
2940+
check_eq!(
2941+
self.arch == "nvptx64",
2942+
matches!(self.linker_flavor, LinkerFlavor::Ptx),
2943+
"`linker_flavor` must be `ptc` if and only if `arch` is `nvptx64`"
2944+
);
2945+
2946+
for args in [
2947+
&self.pre_link_args,
2948+
&self.late_link_args,
2949+
&self.late_link_args_dynamic,
2950+
&self.late_link_args_static,
2951+
&self.post_link_args,
2952+
] {
2953+
for (&flavor, flavor_args) in args {
2954+
check!(!flavor_args.is_empty(), "linker flavor args must not be empty");
2955+
// Check that flavors mentioned in link args are compatible with the default flavor.
2956+
match self.linker_flavor {
2957+
LinkerFlavor::Gnu(..) => {
2958+
check_matches!(
2959+
flavor,
2960+
LinkerFlavor::Gnu(..),
2961+
"mixing GNU and non-GNU linker flavors"
2962+
);
2963+
}
2964+
LinkerFlavor::Darwin(..) => {
2965+
check_matches!(
2966+
flavor,
2967+
LinkerFlavor::Darwin(..),
2968+
"mixing Darwin and non-Darwin linker flavors"
2969+
)
2970+
}
2971+
LinkerFlavor::WasmLld(..) => {
2972+
check_matches!(
2973+
flavor,
2974+
LinkerFlavor::WasmLld(..),
2975+
"mixing wasm and non-wasm linker flavors"
2976+
)
2977+
}
2978+
LinkerFlavor::Unix(..) => {
2979+
check_matches!(
2980+
flavor,
2981+
LinkerFlavor::Unix(..),
2982+
"mixing unix and non-unix linker flavors"
2983+
);
2984+
}
2985+
LinkerFlavor::Msvc(..) => {
2986+
check_matches!(
2987+
flavor,
2988+
LinkerFlavor::Msvc(..),
2989+
"mixing MSVC and non-MSVC linker flavors"
2990+
);
2991+
}
2992+
LinkerFlavor::EmCc
2993+
| LinkerFlavor::Bpf
2994+
| LinkerFlavor::Ptx
2995+
| LinkerFlavor::Llbc => {
2996+
check_eq!(flavor, self.linker_flavor, "mixing different linker flavors")
2997+
}
2998+
}
2999+
3000+
// Check that link args for cc and non-cc versions of flavors are consistent.
3001+
let check_noncc = |noncc_flavor| -> Result<(), String> {
3002+
if let Some(noncc_args) = args.get(&noncc_flavor) {
3003+
for arg in flavor_args {
3004+
if let Some(suffix) = arg.strip_prefix("-Wl,") {
3005+
check!(
3006+
noncc_args.iter().any(|a| a == suffix),
3007+
" link args for cc and non-cc versions of flavors are not consistent"
3008+
);
3009+
}
3010+
}
3011+
}
3012+
Ok(())
3013+
};
3014+
3015+
match self.linker_flavor {
3016+
LinkerFlavor::Gnu(Cc::Yes, lld) => check_noncc(LinkerFlavor::Gnu(Cc::No, lld))?,
3017+
LinkerFlavor::WasmLld(Cc::Yes) => check_noncc(LinkerFlavor::WasmLld(Cc::No))?,
3018+
LinkerFlavor::Unix(Cc::Yes) => check_noncc(LinkerFlavor::Unix(Cc::No))?,
3019+
_ => {}
3020+
}
3021+
}
3022+
3023+
// Check that link args for lld and non-lld versions of flavors are consistent.
3024+
for cc in [Cc::No, Cc::Yes] {
3025+
check_eq!(
3026+
args.get(&LinkerFlavor::Gnu(cc, Lld::No)),
3027+
args.get(&LinkerFlavor::Gnu(cc, Lld::Yes)),
3028+
"link args for lld and non-lld versions of flavors are not consistent",
3029+
);
3030+
check_eq!(
3031+
args.get(&LinkerFlavor::Darwin(cc, Lld::No)),
3032+
args.get(&LinkerFlavor::Darwin(cc, Lld::Yes)),
3033+
"link args for lld and non-lld versions of flavors are not consistent",
3034+
);
3035+
}
3036+
check_eq!(
3037+
args.get(&LinkerFlavor::Msvc(Lld::No)),
3038+
args.get(&LinkerFlavor::Msvc(Lld::Yes)),
3039+
"link args for lld and non-lld versions of flavors are not consistent",
3040+
);
3041+
}
3042+
3043+
if self.link_self_contained.is_disabled() {
3044+
check!(
3045+
self.pre_link_objects_self_contained.is_empty()
3046+
&& self.post_link_objects_self_contained.is_empty(),
3047+
"if `link_self_contained` is disabled, then `pre_link_objects_self_contained` and `post_link_objects_self_contained` must be empty",
3048+
);
3049+
}
3050+
3051+
// If your target really needs to deviate from the rules below,
3052+
// except it and document the reasons.
3053+
// Keep the default "unknown" vendor instead.
3054+
check_ne!(self.vendor, "", "`vendor` cannot be empty");
3055+
check_ne!(self.os, "", "`os` cannot be empty");
3056+
if !self.can_use_os_unknown() {
3057+
// Keep the default "none" for bare metal targets instead.
3058+
check_ne!(
3059+
self.os,
3060+
"unknown",
3061+
"`unknown` os can only be used on particular targets; use `none` for bare-metal targets"
3062+
);
3063+
}
3064+
3065+
// Check dynamic linking stuff.
3066+
// FIXME (https://github.com/rust-lang/rust/issues/133459): We skip this for JSON targets
3067+
// since otherwise, our default values would fail this test. These checks are not critical
3068+
// for correctness, but we don't want to accidentally violate them for built-in targets.
3069+
if kind == TargetKind::Builtin {
3070+
// BPF: when targeting user space vms (like rbpf), those can load dynamic libraries.
3071+
// hexagon: when targeting QuRT, that OS can load dynamic libraries.
3072+
// wasm{32,64}: dynamic linking is inherent in the definition of the VM.
3073+
if self.os == "none"
3074+
&& (self.arch != "bpf"
3075+
&& self.arch != "hexagon"
3076+
&& self.arch != "wasm32"
3077+
&& self.arch != "wasm64")
3078+
{
3079+
check!(
3080+
!self.dynamic_linking,
3081+
"dynamic linking is not supported on this OS/architecture"
3082+
);
3083+
}
3084+
if self.only_cdylib
3085+
|| self.crt_static_allows_dylibs
3086+
|| !self.late_link_args_dynamic.is_empty()
3087+
{
3088+
check!(
3089+
self.dynamic_linking,
3090+
"dynamic linking must be allowed when `only_cdylib` or `crt_static_allows_dylibs` or `late_link_args_dynamic` are set"
3091+
);
3092+
}
3093+
// Apparently PIC was slow on wasm at some point, see comments in wasm_base.rs
3094+
if self.dynamic_linking && !self.is_like_wasm {
3095+
check_eq!(
3096+
self.relocation_model,
3097+
RelocModel::Pic,
3098+
"targets that support dynamic linking must use the `pic` relocation model"
3099+
);
3100+
}
3101+
if self.position_independent_executables {
3102+
check_eq!(
3103+
self.relocation_model,
3104+
RelocModel::Pic,
3105+
"targets that support position-independent executables must use the `pic` relocation model"
3106+
);
3107+
}
3108+
// The UEFI targets do not support dynamic linking but still require PIC (#101377).
3109+
if self.relocation_model == RelocModel::Pic && (self.os != "uefi") {
3110+
check!(
3111+
self.dynamic_linking || self.position_independent_executables,
3112+
"when the relocation model is `pic`, the target must support dynamic linking or use position-independent executables. \
3113+
Set the relocation model to `static` to avoid this requirement"
3114+
);
3115+
}
3116+
if self.static_position_independent_executables {
3117+
check!(
3118+
self.position_independent_executables,
3119+
"if `static_position_independent_executables` is set, then `position_independent_executables` must be set"
3120+
);
3121+
}
3122+
if self.position_independent_executables {
3123+
check!(
3124+
self.executables,
3125+
"if `position_independent_executables` is set then `executables` must be set"
3126+
);
3127+
}
3128+
}
3129+
3130+
// Check crt static stuff
3131+
if self.crt_static_default || self.crt_static_allows_dylibs {
3132+
check!(
3133+
self.crt_static_respected,
3134+
"static CRT can be enabled but `crt_static_respected` is not set"
3135+
);
3136+
}
3137+
3138+
// Check that RISC-V targets always specify which ABI they use.
3139+
match &*self.arch {
3140+
"riscv32" => {
3141+
check_matches!(
3142+
&*self.llvm_abiname,
3143+
"ilp32" | "ilp32f" | "ilp32d" | "ilp32e",
3144+
"invalid RISC-V ABI name"
3145+
);
3146+
}
3147+
"riscv64" => {
3148+
// Note that the `lp64e` is still unstable as it's not (yet) part of the ELF psABI.
3149+
check_matches!(
3150+
&*self.llvm_abiname,
3151+
"lp64" | "lp64f" | "lp64d" | "lp64q" | "lp64e",
3152+
"invalid RISC-V ABI name"
3153+
);
3154+
}
3155+
_ => {}
3156+
}
3157+
3158+
Ok(())
3159+
}
3160+
3161+
/// Test target self-consistency and JSON encoding/decoding roundtrip.
3162+
#[cfg(test)]
3163+
fn test_target(mut self) {
3164+
let recycled_target = Target::from_json(self.to_json()).map(|(j, _)| j);
3165+
self.update_to_cli();
3166+
self.check_consistency(TargetKind::Builtin).unwrap();
3167+
assert_eq!(recycled_target, Ok(self));
3168+
}
3169+
3170+
// Add your target to the whitelist if it has `std` library
3171+
// and you certainly want "unknown" for the OS name.
3172+
fn can_use_os_unknown(&self) -> bool {
3173+
self.llvm_target == "wasm32-unknown-unknown"
3174+
|| self.llvm_target == "wasm64-unknown-unknown"
3175+
|| (self.env == "sgx" && self.vendor == "fortanix")
3176+
}
3177+
28493178
/// Loads a target descriptor from a JSON object.
28503179
pub fn from_json(obj: Json) -> Result<(Target, TargetWarnings), String> {
28513180
// While ugly, this code must remain this way to retain
@@ -3452,6 +3781,7 @@ impl Target {
34523781
key!(supports_xray, bool);
34533782

34543783
base.update_from_cli();
3784+
base.check_consistency(TargetKind::Json)?;
34553785

34563786
// Each field should have been read using `Json::remove` so any keys remaining are unused.
34573787
let remaining_keys = obj.keys();

0 commit comments

Comments
 (0)