1
Fork 0

Rollup merge of #133463 - taiki-e:aarch64-asm-x18, r=Amanieu

Fix handling of x18 in AArch64 inline assembly on ohos/trusty or with -Zfixed-x18

Currently AArch64 inline assembly allows using x18 on ohos/trusty or with -Zfixed-x18.

7db7489f9b/compiler/rustc_target/src/asm/aarch64.rs (L74-L76)

However, x18 is reserved in these environments and should not be allowed in the input/output operands of inline assemblies as it is in Android, Windows, etc..

7db7489f9b/compiler/rustc_target/src/spec/targets/aarch64_unknown_linux_ohos.rs (L19)
7db7489f9b/compiler/rustc_target/src/spec/targets/aarch64_unknown_trusty.rs (L18)
7db7489f9b/compiler/rustc_codegen_llvm/src/llvm_util.rs (L764-L771)

(As for ohos, +reserve-x18 is [redundant](c417b7a695 (diff-0ddf23e0bf2b28b2d05f842f087d1e6f694e8e06d1765e8d0f10d47fddcdff9c)) since 7a966b9188 that starting using llvm's ohos targets. So removed it from target-spec.)

This fix may potentially break the code for tier 2 target (aarch64-unknown-linux-ohos). (As for others, aarch64-unknown-trusty is tier 3 and -Zfixed-x18 is unstable so breaking them should be fine.)
However, in any case, it seems suspicious that the code that is broken by this was sound.

r? `@Amanieu`

`@rustbot` label O-AArch64 +A-inline-assembly
This commit is contained in:
Guillaume Gomez 2024-11-28 12:06:02 +01:00 committed by GitHub
commit 23bab15d73
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 80 additions and 49 deletions

View file

@ -1,7 +1,7 @@
use std::fmt;
use rustc_data_structures::fx::FxIndexSet;
use rustc_span::Symbol;
use rustc_span::{Symbol, sym};
use super::{InlineAsmArch, InlineAsmType, ModifierInfo};
use crate::spec::{RelocModel, Target};
@ -71,18 +71,26 @@ impl AArch64InlineAsmRegClass {
}
}
pub(crate) fn target_reserves_x18(target: &Target) -> bool {
target.os == "android" || target.os == "fuchsia" || target.is_like_osx || target.is_like_windows
pub(crate) fn target_reserves_x18(target: &Target, target_features: &FxIndexSet<Symbol>) -> bool {
// See isX18ReservedByDefault in LLVM for targets reserve x18 by default:
// https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/TargetParser/AArch64TargetParser.cpp#L102-L105
// Note that +reserve-x18 is currently not set for the above targets.
target.os == "android"
|| target.os == "fuchsia"
|| target.env == "ohos"
|| target.is_like_osx
|| target.is_like_windows
|| target_features.contains(&sym::reserve_x18)
}
fn reserved_x18(
_arch: InlineAsmArch,
_reloc_model: RelocModel,
_target_features: &FxIndexSet<Symbol>,
target_features: &FxIndexSet<Symbol>,
target: &Target,
_is_clobber: bool,
) -> Result<(), &'static str> {
if target_reserves_x18(target) {
if target_reserves_x18(target, target_features) {
Err("x18 is a reserved register on this target")
} else {
Ok(())

View file

@ -965,11 +965,13 @@ impl InlineAsmClobberAbi {
_ => Err(&["C", "system", "efiapi", "aapcs"]),
},
InlineAsmArch::AArch64 => match name {
"C" | "system" | "efiapi" => Ok(if aarch64::target_reserves_x18(target) {
InlineAsmClobberAbi::AArch64NoX18
} else {
InlineAsmClobberAbi::AArch64
}),
"C" | "system" | "efiapi" => {
Ok(if aarch64::target_reserves_x18(target, target_features) {
InlineAsmClobberAbi::AArch64NoX18
} else {
InlineAsmClobberAbi::AArch64
})
}
_ => Err(&["C", "system", "efiapi"]),
},
InlineAsmArch::Arm64EC => match name {

View file

@ -16,7 +16,6 @@ pub(crate) fn target() -> Target {
data_layout: "e-m:e-p270:32:32-p271:32:32-p272:64:64-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128-Fn32".into(),
arch: "aarch64".into(),
options: TargetOptions {
features: "+reserve-x18".into(),
mcount: "\u{1}_mcount".into(),
stack_probes: StackProbeType::Inline,
supported_sanitizers: SanitizerSet::ADDRESS

View file

@ -228,6 +228,12 @@ const AARCH64_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[
("rcpc3", Unstable(sym::aarch64_unstable_target_feature), &["rcpc2"]),
// FEAT_RDM
("rdm", Stable, &["neon"]),
// This is needed for inline assembly, but shouldn't be stabilized as-is
// since it should be enabled globally using -Zfixed-x18, not
// #[target_feature].
// Note that cfg(target_feature = "reserve-x18") is currently not set for
// targets that reserve x18 by default.
("reserve-x18", Unstable(sym::aarch64_unstable_target_feature), &[]),
// FEAT_SB
("sb", Stable, &[]),
// FEAT_SHA1 & FEAT_SHA256