Skip to content

Commit 6111eda

Browse files
committed
reject unsound toggling of RISCV target features
1 parent 74e2ac4 commit 6111eda

File tree

2 files changed

+64
-4
lines changed

2 files changed

+64
-4
lines changed

Diff for: compiler/rustc_target/src/spec/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -3166,7 +3166,7 @@ impl Target {
31663166
// Note that the `lp64e` is still unstable as it's not (yet) part of the ELF psABI.
31673167
check_matches!(
31683168
&*self.llvm_abiname,
3169-
"lp64" | "lp64f" | "lp64d" | "lp64q" | "lp64e",
3169+
"lp64" | "lp64f" | "lp64d" | "lp64e",
31703170
"invalid RISC-V ABI name"
31713171
);
31723172
}

Diff for: compiler/rustc_target/src/target_features.rs

+63-3
Original file line numberDiff line numberDiff line change
@@ -590,9 +590,69 @@ const RISCV_FEATURES: &[(&str, StabilityUncomputed, ImpliedFeatures)] = &[
590590
// tidy-alphabetical-start
591591
("a", STABLE, &["zaamo", "zalrsc"]),
592592
("c", STABLE, &[]),
593-
("d", unstable(sym::riscv_target_feature), &["f"]),
594-
("e", unstable(sym::riscv_target_feature), &[]),
595-
("f", unstable(sym::riscv_target_feature), &[]),
593+
(
594+
"d",
595+
Stability::Unstable {
596+
nightly_feature: sym::riscv_target_feature,
597+
allow_toggle: |target, enable| match &*target.llvm_abiname {
598+
"ilp32d" | "lp64d" if !enable => {
599+
// The ABI requires the `d` feature, so it cannot be disabled.
600+
Err("feature is required by ABI")
601+
}
602+
"ilp32e" if enable => {
603+
// The `d` feature apparently is incompatible with this ABI.
604+
Err("feature is incompatible with ABI")
605+
}
606+
_ => Ok(()),
607+
},
608+
},
609+
&["f"],
610+
),
611+
(
612+
"e",
613+
Stability::Unstable {
614+
nightly_feature: sym::riscv_target_feature,
615+
allow_toggle: |target, enable| {
616+
match &*target.llvm_abiname {
617+
_ if !enable => {
618+
// This is a negative feature, *disabling* it is always okay.
619+
Ok(())
620+
}
621+
"ilp32e" | "lp64e" => {
622+
// Embedded ABIs should already have the feature anyway, it's fine to enable
623+
// it again.
624+
Ok(())
625+
}
626+
_ => {
627+
// *Not* an embedded ABI. Enabling `e` is invalid.
628+
Err("feature is required by ABI")
629+
}
630+
}
631+
},
632+
},
633+
&[],
634+
),
635+
(
636+
"f",
637+
Stability::Unstable {
638+
nightly_feature: sym::riscv_target_feature,
639+
allow_toggle: |target, enable| {
640+
match &*target.llvm_abiname {
641+
"ilp32f" | "ilp32d" | "lp64f" | "lp64d" if !enable => {
642+
// The ABI requires the `f` feature, so it cannot be disabled.
643+
Err("feature is required by ABI")
644+
}
645+
_ => Ok(()),
646+
}
647+
},
648+
},
649+
&[],
650+
),
651+
(
652+
"forced-atomics",
653+
Stability::Forbidden { reason: "unsound because it changes the ABI of atomic operations" },
654+
&[],
655+
),
596656
("m", STABLE, &[]),
597657
("relax", unstable(sym::riscv_target_feature), &[]),
598658
("unaligned-scalar-mem", unstable(sym::riscv_target_feature), &[]),

0 commit comments

Comments
 (0)