Skip to content

Commit

Permalink
Auto merge of #86062 - nagisa:nagisa/what-a-lie, r=estebank
Browse files Browse the repository at this point in the history
Do not allow JSON targets to set is-builtin: true

Note that this will affect (and make builds fail for) all of the projects out there that have target files invalid in this way. Crater, however, does not really cover these kinds of the codebases, so it is quite difficult to measure the impact. That said, the target files invalid in this way can start causing build failures each time LLVM is upgraded, anyway, so it is probably a good opportunity to disallow this property, entirely.

Another approach considered was to simply not parse this field anymore, which would avoid making the builds explicitly fail, but it wasn't clear to me if `is-builtin` was always set unintentionally… In case this was the case, I'd expect people to file a feature request stating specifically for what purpose they were using `is-builtin`.

Fixes #86017
  • Loading branch information
bors committed Jul 17, 2021
2 parents 0cd12d6 + f4701cd commit 153df0f
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 6 deletions.
11 changes: 6 additions & 5 deletions compiler/rustc_codegen_llvm/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,12 @@ pub unsafe fn create_module(

if !custom_llvm_used && target_data_layout != llvm_data_layout {
bug!(
"data-layout for builtin `{}` target, `{}`, \
differs from LLVM default, `{}`",
sess.target.llvm_target,
target_data_layout,
llvm_data_layout
"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
);
}
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_target/src/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2015,6 +2015,10 @@ impl Target {
key!(supported_sanitizers, SanitizerSet)?;
key!(default_adjusted_cabi, Option<Abi>)?;

if base.is_builtin {
// This can cause unfortunate ICEs later down the line.
return Err(format!("may not set is_builtin for targets not built-in"));
}
// Each field should have been read using `Json::remove_key` so any keys remaining are unused.
let remaining_keys = obj.as_object().ok_or("Expected JSON object for target")?.keys();
Ok((
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
"executables": true,
"has-elf-tls": true,
"has-rpath": true,
"is-builtin": true,
"linker-is-gnu": true,
"llvm-target": "x86_64-unknown-linux-gnu",
"max-atomic-width": 64,
Expand Down
2 changes: 2 additions & 0 deletions src/test/run-make-fulldeps/target-specs/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ all:
RUST_TARGET_PATH=. $(RUSTC) foo.rs --target=my-awesome-platform --crate-type=lib --emit=asm
RUST_TARGET_PATH=. $(RUSTC) foo.rs --target=my-x86_64-unknown-linux-gnu-platform --crate-type=lib --emit=asm
$(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=mismatching-data-layout
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"arch": "x86_64",
"is-builtin": true,
"data-layout": "e-m:e-i64:64-f80:128-n8:16:32:64-S128",
"llvm-target": "x86_64-unknown-unknown-gnu",
"target-pointer-width": "64"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"arch": "x86_64",
"data-layout": "e-m:e-i64:16:32:64",
"llvm-target": "x86_64-unknown-unknown-gnu",
"target-pointer-width": "64"
}

0 comments on commit 153df0f

Please sign in to comment.