Skip to content

Commit 153df0f

Browse files
committed
Auto merge of #86062 - nagisa:nagisa/what-a-lie, r=estebank
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
2 parents 0cd12d6 + f4701cd commit 153df0f

File tree

6 files changed

+25
-6
lines changed

6 files changed

+25
-6
lines changed

compiler/rustc_codegen_llvm/src/context.rs

+6-5
Original file line numberDiff line numberDiff line change
@@ -149,11 +149,12 @@ pub unsafe fn create_module(
149149

150150
if !custom_llvm_used && target_data_layout != llvm_data_layout {
151151
bug!(
152-
"data-layout for builtin `{}` target, `{}`, \
153-
differs from LLVM default, `{}`",
154-
sess.target.llvm_target,
155-
target_data_layout,
156-
llvm_data_layout
152+
"data-layout for target `{rustc_target}`, `{rustc_layout}`, \
153+
differs from LLVM target's `{llvm_target}` default layout, `{llvm_layout}`",
154+
rustc_target = sess.opts.target_triple,
155+
rustc_layout = target_data_layout,
156+
llvm_target = sess.target.llvm_target,
157+
llvm_layout = llvm_data_layout
157158
);
158159
}
159160
}

compiler/rustc_target/src/spec/mod.rs

+4
Original file line numberDiff line numberDiff line change
@@ -2015,6 +2015,10 @@ impl Target {
20152015
key!(supported_sanitizers, SanitizerSet)?;
20162016
key!(default_adjusted_cabi, Option<Abi>)?;
20172017

2018+
if base.is_builtin {
2019+
// This can cause unfortunate ICEs later down the line.
2020+
return Err(format!("may not set is_builtin for targets not built-in"));
2021+
}
20182022
// Each field should have been read using `Json::remove_key` so any keys remaining are unused.
20192023
let remaining_keys = obj.as_object().ok_or("Expected JSON object for target")?.keys();
20202024
Ok((

src/test/run-make-fulldeps/rustdoc-target-spec-json-path/target.json

-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
"executables": true,
99
"has-elf-tls": true,
1010
"has-rpath": true,
11-
"is-builtin": true,
1211
"linker-is-gnu": true,
1312
"llvm-target": "x86_64-unknown-linux-gnu",
1413
"max-atomic-width": 64,

src/test/run-make-fulldeps/target-specs/Makefile

+2
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,5 @@ all:
77
RUST_TARGET_PATH=. $(RUSTC) foo.rs --target=my-awesome-platform --crate-type=lib --emit=asm
88
RUST_TARGET_PATH=. $(RUSTC) foo.rs --target=my-x86_64-unknown-linux-gnu-platform --crate-type=lib --emit=asm
99
$(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 -
10+
$(RUSTC) foo.rs --target=definitely-not-builtin-target 2>&1 | $(CGREP) 'may not set is_builtin'
11+
$(RUSTC) foo.rs --target=mismatching-data-layout
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"arch": "x86_64",
3+
"is-builtin": true,
4+
"data-layout": "e-m:e-i64:64-f80:128-n8:16:32:64-S128",
5+
"llvm-target": "x86_64-unknown-unknown-gnu",
6+
"target-pointer-width": "64"
7+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"arch": "x86_64",
3+
"data-layout": "e-m:e-i64:16:32:64",
4+
"llvm-target": "x86_64-unknown-unknown-gnu",
5+
"target-pointer-width": "64"
6+
}

0 commit comments

Comments
 (0)