Skip to content

Commit

Permalink
[target-spec] actually deserialize target_family and target_endian co…
Browse files Browse the repository at this point in the history
…rrectly

Oof, this is a pretty bad bug -- we weren't deserializing these correctly.

Add some spot checks, some snapshot tests, and remove some old `rustc_version`
code.
  • Loading branch information
sunshowers committed Dec 22, 2024
1 parent 8990357 commit 97c1b03
Show file tree
Hide file tree
Showing 8 changed files with 129 additions and 25 deletions.
11 changes: 0 additions & 11 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"arch": "powerpc64",
"abi-return-struct-as-int": true,
"cpu": "ppc64",
"crt-objects-fallback": "false",
"crt-static-respected": true,
"data-layout": "E-m:e-Fn32-i64:64-n32:64",
"default-dwarf-version": 2,
"dynamic-linking": true,
"has-rpath": true,
"has-thread-local": true,
"is-builtin": true,
"linker-flavor": "gnu-cc",
"llvm-target": "powerpc64-unknown-freebsd",
"max-atomic-width": 64,
"metadata": {
"description": "PPC64 FreeBSD (ELFv1 and ELFv2)",
"host_tools": true,
"std": true,
"tier": 3
},
"os": "freebsd",
"position-independent-executables": true,
"pre-link-args": {
"gnu-cc": ["-m64"],
"gnu-lld-cc": ["-m64"]
},
"relro-level": "full",
"stack-probes": {
"kind": "inline"
},
"target-endian": "middle",
"target-family": ["unix"],
"target-mcount": "_mcount",
"target-pointer-width": "64"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"arch": "powerpc64",
"abi-return-struct-as-int": true,
"cpu": "ppc64",
"crt-objects-fallback": "false",
"crt-static-respected": true,
"data-layout": "E-m:e-Fn32-i64:64-n32:64",
"default-dwarf-version": 2,
"dynamic-linking": true,
"has-rpath": true,
"has-thread-local": true,
"is-builtin": true,
"linker-flavor": "gnu-cc",
"llvm-target": "powerpc64-unknown-freebsd",
"max-atomic-width": 64,
"metadata": {
"description": "PPC64 FreeBSD (ELFv1 and ELFv2)",
"host_tools": true,
"std": true,
"tier": 3
},
"os": "freebsd",
"position-independent-executables": true,
"pre-link-args": {
"gnu-cc": ["-m64"],
"gnu-lld-cc": ["-m64"]
},
"relro-level": "full",
"stack-probes": {
"kind": "inline"
},
"target-endian": "big",
"target-family": "none",
"target-mcount": "_mcount",
"target-pointer-width": "64"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
source: target-spec-miette/tests/snapshot/custom.rs
expression: "format!(\"{:?}\", miette::Report::new_boxed(diagnostic))"
snapshot_kind: text
---
× error deserializing custom target JSON for `my-target`
╰─▶ unknown variant `middle`, expected `little` or `big` at line 32 column
29
╭─[32:29]
31 │ },
32"target-endian": "middle",
· ▲
· ╰── unknown variant `middle`, expected `little` or `big`
33"target-family": ["unix"],
╰────
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
source: target-spec-miette/tests/snapshot/custom.rs
expression: "format!(\"{:?}\", miette::Report::new_boxed(diagnostic))"
snapshot_kind: text
---
× error deserializing custom target JSON for `my-target`
╰─▶ invalid type: string "none", expected a sequence at line 33 column 27
╭─[33:27]
32"target-endian": "big",
33"target-family": "none",
· ▲
· ╰── invalid type: string "none", expected a sequence
34"target-mcount": "_mcount",
╰────
1 change: 0 additions & 1 deletion target-spec/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ unicode-ident = "1.0.14"
guppy-workspace-hack.workspace = true

[dev-dependencies]
rustc_version = "0.4.1"
test-case = "3.3.1"
toml = "0.5.11"

Expand Down
40 changes: 28 additions & 12 deletions target-spec/src/custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ use serde::{Deserialize, Serialize};
#[derive(Clone, Debug, Deserialize, Serialize, Eq, Hash, Ord, PartialEq, PartialOrd)]
#[serde(rename_all = "kebab-case")]
pub(crate) struct TargetDefinition {
// TODO: it would be nice to use target-spec-json for this, but that has a
// few limitations as of v0.1:
//
// * target-pointer-width is a string, not an integer.
// * Os and Env deserialized to enums, but we would really like them to be strings.
//
// ---
arch: String,
#[serde(rename = "target-pointer-width", with = "target_pointer_width")]
pointer_width: u8,
Expand All @@ -34,9 +41,9 @@ pub(crate) struct TargetDefinition {
#[serde(default)]
vendor: Option<String>,
#[serde(default)]
families: Vec<String>,
target_family: Vec<String>,
#[serde(default)]
endian: Endian,
target_endian: Endian,
#[serde(default)]
min_atomic_width: Option<u16>,
#[serde(default)]
Expand Down Expand Up @@ -76,9 +83,9 @@ impl TargetDefinition {
arch: Arch::new(self.arch),
env: self.env.map(Env::new),
vendor: self.vendor.map(Vendor::new),
families: Families::new(self.families.into_iter().map(Family::new)),
families: Families::new(self.target_family.into_iter().map(Family::new)),
pointer_width: self.pointer_width,
endian: self.endian.to_cfg_expr(),
endian: self.target_endian.to_cfg_expr(),
has_atomics: HasAtomics::new(has_atomics),
panic: self.panic_strategy.to_cfg_expr(),
}
Expand Down Expand Up @@ -156,13 +163,6 @@ mod tests {

#[test]
fn test_all_builtin_specs_recognized() {
let version = rustc_version::version().expect("rustc_version succeeded");
if version.minor < 70 {
// all-target-specs-json is only present on Rust 1.70 and above.
println!("** skipping, minor version {} < 70", version.minor);
return;
}

let rustc_bin: String = std::env::var("RUSTC").unwrap_or_else(|_| "rustc".to_owned());
let output = Command::new(rustc_bin)
// Used for -Zunstable-options. This is test-only code so it doesn't matter.
Expand All @@ -178,12 +178,28 @@ mod tests {
eprintln!("*** testing {triple}");
// Just make sure this doesn't panic. (If this becomes fallible in the future, then this
// shouldn't return an error either.)
target_def.clone().into_target_info(triple.into());
target_def.clone().into_target_info(triple.clone().into());
let json =
serde_json::to_string(&target_def).expect("target def serialized successfully");
eprintln!("* minified json: {json}");
let target_def_2 = serde_json::from_str(&json).expect("target def 2 deserialized");
assert_eq!(target_def, target_def_2, "matches");

// Do some spot checks for things like big-endian targets.
if triple.starts_with("powerpc-") || triple.starts_with("powerpc64-") {
assert_eq!(
target_def.target_endian,
Endian::Big,
"powerpc is big-endian"
);
}
if triple.contains("-linux") {
assert!(
target_def.target_family.contains(&"unix".to_owned()),
"linux target_family should contain unix (was {:#?})",
target_def.target_family,
);
}
}
}
}
1 change: 0 additions & 1 deletion workspace-hack/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ petgraph = { version = "0.6.5", default-features = false, features = ["graphmap"
regex = { version = "1.10.5", default-features = false, features = ["perf", "std"] }
regex-automata = { version = "0.4.5", default-features = false, features = ["dfa", "hybrid", "meta", "nfa", "perf", "unicode"] }
regex-syntax = { version = "0.8.2" }
semver = { version = "1.0.23", features = ["serde"] }
serde = { version = "1.0.215", features = ["alloc", "derive"] }
serde_json = { version = "1.0.133", features = ["unbounded_depth"] }
textwrap = { version = "0.16.0" }
Expand Down

0 comments on commit 97c1b03

Please sign in to comment.