-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
-Csoft-float flag is unsound #129893
Comments
Hey ARM Group! This bug has been identified as a good "ARM candidate". cc @adamgemmell @hug-dev @jacobbramley @JamieCunliffe @joaopaulocarreiro @raw-bin @Stammark |
Rustc just always forwards this flag to LLVM, so it seems possible that the Cc @parched who added this part to the docs in #36261. Does LLVM document anywhere what this flag does, and which targets it affects? |
(hi from from the embedded side. i'm mostly familiar with the One use of the flag is to allow using the FPU while still using the soft-float ABI for compatibility with other code. However, this can already be done by using the I can't think of any other uses, so I wouldn't oppose deprecating/removing it. |
Seems like this flag is called
Yes indeed, if there's a way to tell LLVM "use soft-float ABI but also use FPU", in a way that doesn't affect the ABI at all as is entirely link-compatible with fully soft-float code, then that's what should be done. |
Hm, either I am misunderstanding the assembly or this feature does not work as advertised? https://godbolt.org/z/53W75s3fc seems to show that EDIT: Ah no I just can't read assembly. This still uses hard-float operations but first moves the data from |
Here are the
So, only one tier 2 target is affected. And it's a tier 2 target without listed maintainers. |
armv7-sony-vita-newlibeabihf is meant to be used with a single cpu only which supports floats, so there is probably no need for soft-float support for that target. |
Turning off the hard float ABI on a hard float target sounds like a "why do we even have that lever" kind of deal. I can't imagine why anyone would want to and I expect it would end badly if they tried. As others have said, adding FPU instructions into a soft-float build is a entirely different matter, as is enabling Helium (MVE) or anything else that affects the juicy centre of a function but not its hard outer shell. |
On the Cortex-R52, the FPU is mandatory and if anyone wants compatibility with older soft-float code they can either use armv7r-none-eabi, or they can come and make a compelling case for the v8-R version (kernel developers? idk). I'm not minded to add it on a whim because it seems pretty niche. |
This flag has a very "from the days when there was an OABI" vibe to it. |
I think we should start by warning when this flag is used, maybe that will get someone to explain their hypothetical use-case that cannot possibly be replaced by the |
All right, I made #129897 do exactly that now. |
WG-prioritization assigning priority on Zulip. Also, (related discussion). @rustbot label -I-prioritize +P-medium |
deprecate -Csoft-float because it is unsound (and not fixable) See rust-lang#129893 for details. The general sentiment there seems to be that this flag has no use and sound alternatives exist, so let's add this warning and see if anyone out there disagrees. Also show a different warning on targets where it does nothing (as documented since rust-lang#36261): it seems to correspond to `-mfloat-abi` in GCC/clang, which is an ARM-specific option. To be really sure it does nothing, only forward the flag to LLVM for eabihf targets. This should not change behavior but makes me sleep better ;)
Rollup merge of rust-lang#129897 - RalfJung:soft-float-ignored, r=Urgau deprecate -Csoft-float because it is unsound (and not fixable) See rust-lang#129893 for details. The general sentiment there seems to be that this flag has no use and sound alternatives exist, so let's add this warning and see if anyone out there disagrees. Also show a different warning on targets where it does nothing (as documented since rust-lang#36261): it seems to correspond to `-mfloat-abi` in GCC/clang, which is an ARM-specific option. To be really sure it does nothing, only forward the flag to LLVM for eabihf targets. This should not change behavior but makes me sleep better ;)
A good soft/no-float story is highly desired by Rust for Linux (and other kernel developers): Rust-for-Linux/linux#514 There's the clippy lint to avoid floats, but it runs into the same issue as this flag: lints are crate per crate, while this should be binary by binary. |
That will almost surely evolve around |
@bjorn3 just made me aware of this amazing flag:
This is quite unsound: if code gets compiled with
-Csoft-float
and calls code from the standard library that uses the hard float ABI, we have UB. Generally we need different target triples for softfloat vs hardfloat ABIs, since (as per the discussion in rust-lang/lang-team#235) code within a single target should be ABI compatible. Cargo even (unstably) allows overwritingRUSTFLAGS
on a per-crate basis, so we better make sure crates compiled with different flags can be linked with each other.This was added a looooong time ago in #9617. I couldn't find any discussion regarding its soundness.
We have e.g.
arm-unknown-linux-musleabi
andarm-unknown-linux-musleabihf
, so using the*hf
target but with-Csoft-float
also seems kind of unnecessary. (But I have not checked whether alleabihf
targets have a correspondingeabi
target.)According to the documentation, this can only be used by ARM targets. So paging in some ARM folks -- is this used in practice, and if yes, how do people avoid the soundness problems?
@rustbot ping arm
The text was updated successfully, but these errors were encountered: