The (stable) neon aarch64 target feature is unsound: it changes the float ABI

[RalfJung]

This is an instance of #116344, but since it affects a target feature marked as "stable" I made a separate issue. See rust-lang/compiler-team#780 for the MCP that approved forbidding the toggling of target features that are unsound due to their ABI impact. Stabilization of neon happened in #90621. I am not sure where the FCP for this occurred.

The summary of the problem is that code compiled with -C target-feature=-neon on an aarch64 target, if it calls any pre-compiled function from the standard library that involves f32/f64 arguments, will use the wrong ABI and hence cause UB. I am working towards marking such features as "forbidden" so that we can fix this soundness hole. But now we are hitting the same where a relevant feature is also already stable, so making it "forbidden" would be a breaking change... so we'll have to figure out something more clever. Like maybe only forbidding disabling the feature? Note however that on the aarch64-unknown-none-softfloat target, +neon is unsound for the same reason.

Cc @Amanieu @workingjubilee

[jacobbramley]

aarch64-unknown-none-softfloat +neon could potentially use some soft-float procedure-call standard but use Neon/FP inside functions. That's what the AArch32 softfp ABIs used to do, and the use-case is possibly similar. It'd be a change of behaviour for code compiled with different Rust versions, perhaps.

[RalfJung]

aarch64-unknown-none-softfloat +neon could potentially use some soft-float procedure-call standard but use Neon/FP inside functions.

I don't think LLVM currently supports this. On other architectures, it is possible to force a soft-float ABI while still having FP instructions and registers available inside functions (e.g. +soft-float,+sse2 on x86), but aarch64 does not seem to have that option right now.

[apiraino]

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

[workingjubilee]

imo the major reason that we support neon as a feature flag is so that people can write cfg(target_feature = "neon") or target_feature(enable = "neon") for aarch64 embedded targets (since while the FPU should always be present, you do have to enable it first, and that might not be automatic for some embedded programs...), not so they can unsoundly globally disable it on targets that assume its presence.

[RalfJung]

target_feature(enable = "neon") only makes sense on targets that don't already have that target feature... I guess that would be aarch64-unknown-none-softfloat?

However, it's also a super dangerous attribute. If that function takes or returns f32/f64, or calls any function that takes or returns f32/f64, we have ABI conflicts. Here's a demo of that. We could allow enabling neon without enabling fp-armv8, but unfortunately Rust enables both when just enabling neon...

But given current LLVM I don't think there is a sound way to tell it to build code that makes use of the FPU on an aarch64 softfloat target. So seems like the proper fix is to match what other targets do, and add a soft-float target feature that forces the use of the softfloat ABI even if neon / an FPU is present?

[RalfJung]

I filed an LLVM issue; I hope I used the right words here since there's still a lot I don't understand around target features. ;)
llvm/llvm-project#110632

[pinskia]

Note for aarch64 there is NO defined soft-float ABI. Even though advanced simd (and fp) are optional part of armv8-a, there is no parts out there where it is not included. I have no idea why LLVM has a softfloat aarch64 target. There is a way to forbid all simd and fp usage (in both LLVM and GCC) but that does not change the ABI.

[workingjubilee]

However, it's also a super dangerous attribute. If that function takes or returns f32/f64, or calls any function that takes or returns f32/f64, we have ABI conflicts. Here's a demo of that. We could allow enabling neon without enabling fp-armv8, but unfortunately Rust enables both when just enabling neon...

I took the position that we should do this because

If FEAT_FP is implemented, then FEAT_AdvSIMD is implemented.

There is no situation where one is enabled and the other is not.

[RalfJung]

There is a way to forbid all simd and fp usage (in both LLVM and GCC) but that does not change the ABI.

It does change the ABI though, as one can easily verify on godbolt: https://rust.godbolt.org/z/aP55vT1bf

[RalfJung]

There is no situation where one is enabled and the other is not.

Yeah that makes perfect sense if one just looks at which features the hardware supports, but sadly LLVM conflates that aspect with whether a softfloat or hardfloat ABI should be used.

[workingjubilee]

LLVM has many bugs, yes.

[workingjubilee]

arguably the first bug is "having a soft-float ABI for AArch64".

[RalfJung]

arguably the first bug is "having a soft-float ABI for AArch64".

That's the argument being brought up in the LLVM issue, yeah.

[pinskia]

That's the argument being brought up in the LLVM issue, yeah.

Actually it is defined here: ARM-software/abi-aa#232 . But it is only for the R cores rather than the A cores.

[pinskia]

From the ABI: "This variant is incompatible with the base procedure call standard and toolchains are not required to support it."

[workingjubilee]

huh, interesting.

[RalfJung]

The problen is that LLVM does (pretend to?) support it, just in an incomplete way. So it seems reasonable to ask whether the existing support could be made a bit less incomplete.

[RalfJung]

Given that there is no standard softfloat ABI for this target, maybe the right thing to do is to make sure that on an aarch64 softfloat target, we never use the LLVM-defined ABI for floats, but instead use PassMode::Cast and pass floats like integers (for all ABIs). Then we are entirely independent from LLVM on those targets.

(Suggested by @Amanieu , or at least I think that is what they meant.)

We'd still have to forbid disabling neon on aarch64 hardfloat targets. But that doesn't seem unreasonable, we're basically saying those targets require float and SIMD support.

[RalfJung]

The other (valid) question being raised on the LLVM side is, what is even the usecase for ever allowing the use of neon or the FPU on the softfloat target? The target exists for kernels that want to avoid the cost of saving float registers on a context switch. So that usecase certainly does not want to ever enable the neon feature, right? Who does need the neon feature? Why did we bother stabilizing it?