Skip to content

Commit 171223e

Browse files
committed
reject unsound toggling of RISCV target features
1 parent d185062 commit 171223e

File tree

2 files changed

+67
-4
lines changed

2 files changed

+67
-4
lines changed

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
}

compiler/rustc_target/src/target_features.rs

+66-3
Original file line numberDiff line numberDiff line change
@@ -590,9 +590,72 @@ 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+
// Given that this is a negative feature, consider this before stabilizing:
615+
// does it really make sense to enable this feature in an individual
616+
// function with `#[target_feature]`?
617+
nightly_feature: sym::riscv_target_feature,
618+
allow_toggle: |target, enable| {
619+
match &*target.llvm_abiname {
620+
_ if !enable => {
621+
// This is a negative feature, *disabling* it is always okay.
622+
Ok(())
623+
}
624+
"ilp32e" | "lp64e" => {
625+
// Embedded ABIs should already have the feature anyway, it's fine to enable
626+
// it again from an ABI perspective.
627+
Ok(())
628+
}
629+
_ => {
630+
// *Not* an embedded ABI. Enabling `e` is invalid.
631+
Err("feature is incompatible with ABI")
632+
}
633+
}
634+
},
635+
},
636+
&[],
637+
),
638+
(
639+
"f",
640+
Stability::Unstable {
641+
nightly_feature: sym::riscv_target_feature,
642+
allow_toggle: |target, enable| {
643+
match &*target.llvm_abiname {
644+
"ilp32f" | "ilp32d" | "lp64f" | "lp64d" if !enable => {
645+
// The ABI requires the `f` feature, so it cannot be disabled.
646+
Err("feature is required by ABI")
647+
}
648+
_ => Ok(()),
649+
}
650+
},
651+
},
652+
&[],
653+
),
654+
(
655+
"forced-atomics",
656+
Stability::Forbidden { reason: "unsound because it changes the ABI of atomic operations" },
657+
&[],
658+
),
596659
("m", STABLE, &[]),
597660
("relax", unstable(sym::riscv_target_feature), &[]),
598661
("unaligned-scalar-mem", unstable(sym::riscv_target_feature), &[]),

0 commit comments

Comments
 (0)