-
Notifications
You must be signed in to change notification settings - Fork 1.6k
#[inline(required)]
#3711
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
#[inline(required)]
#3711
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,153 @@ | ||
- Feature Name: `inline(must)` and `inline(required)` | ||
- Start Date: 2024-09-17 | ||
- RFC PR: [rust-lang/rfcs#3711](https://github.com/rust-lang/rfcs/pull/3711) | ||
- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) | ||
|
||
# Summary | ||
[summary]: #summary | ||
|
||
rustc supports inlining functions with the `#[inline]`/`#[inline(always)]`/ | ||
`#[inline(never)]` attributes, but these are only hints to the compiler. | ||
Sometimes it is desirable to require inlining, not just suggest inlining: | ||
security APIs can depend on being inlined for their security properties, and | ||
performance intrinsics can work poorly if not inlined. | ||
|
||
# Motivation | ||
[motivation]: #motivation | ||
|
||
rustc's current inlining attributes are only hints, they do not guarantee that | ||
inlining is performed and users need to check manually whether inlining was | ||
performed. While this is desirable for the vast majority of use cases, there are | ||
circumstances where inlining is necessary to maintain the security properties of | ||
an API, or where performance of intrinsics is likely to suffer if not inlined. | ||
For example, consider: | ||
|
||
- Armv8.3-A's pointer authentication, when used explicitly with intrinsics[^1], | ||
rely on being inlined to guarantee their security properties. Rust should make | ||
it easier to guarantee that these intrinsics will be inlined and emit an error | ||
if that is not possible or has not occurred. | ||
- Arm's Neon and SVE performance intrinsics have work poorly if not inlined. | ||
Since users rely on these intrinsics for their application's performance, Rust | ||
should be able to warn users when these have not been inlined and performance | ||
will not be as expected. | ||
|
||
In general, for many intrinsics, inlining is not an optimisation but an | ||
important part of the semantics, regardless of optimisation level. | ||
Comment on lines
+34
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you truly do mean that inlining is part of the semantics, then I think a lot of work needs to be done here, including making this "inlining" happen for the MIR that Miri executes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assuming I'm reading this and the draft implementation right, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think my wording was too strong here, there wouldn't necessarily be unsoundness, but for something like SIMD intrinsics, they basically only make sense if you inline them - it isn't wrong if you don't, but it isn't more debuggable or anything like that, just slower. These intrinsics only make sense inlined. |
||
|
||
[^1]: Armv8.3-A's pointer authentication can be used implicitly (automatic, | ||
hint-based) and explicitly (intrinsics). Implicit pointer authentication is | ||
already implemented by Rust. Explicit pointer authentication intrinsics are not | ||
yet available in Rust. Exposing pointer authentication intrinsics would require | ||
this RFC, or something providing equivalent guarantees, as a prerequisite. | ||
Comment on lines
+39
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain or provide a link to why these pointer authentication intrinsics require inlining for correctness? Also my knee-jerk reaction to this need is to... make them actual intrinsics. And not even add the attribute this RFC proposes. If the attribute is going to be perma-unstable/internal-only, what's the difference between that and a lang item? I do not find this use-case compelling, and I'd much rather the RFC be motivated by something that we can't do using a well-established pattern based on lang items. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have a link, but for pointer authentication, the idea would be to upstream some functions into stdarch which use inline assembly1 and must be inlined. They have to be inlined because otherwise you're left with a sequence of instructions that can be used as a gadget to sign whatever you want, so you want to mix those sequences into the functions so they can't be used as a gadget. For these functions, Footnotes
|
||
|
||
# Guide-level explanation | ||
[guide-level-explanation]: #guide-level-explanation | ||
|
||
There are now be five variations of the `#[inline]` attribute, the | ||
existing bare `#[inline]`, `#[inline(always)]`, `#[inline(never)]` and | ||
the new `#[inline(must)]` and `#[inline(required)]`. Bare `#[inline]`, | ||
`#[inline(always)]` and `#[inline(never)]` are unchanged. `#[inline(must)]` | ||
and `#[inline(required)]` will always attempt to inline the annotated item, | ||
regardless of optimization level. | ||
|
||
If it is not possible to inline the annotated item then the compiler will emit | ||
a lint on the caller of the annotated item that the item could not be inlined, | ||
providing a reason why inlining was not possible. `#[inline(must)]` emits a | ||
warn-by-default lint and `#[inline(required)]` emits a deny-by-default lint, | ||
which can be used by items which must be inlined to uphold security properties. | ||
Comment on lines
+56
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not think that a deny-by-default lint is sufficiently stern for upholding security properties, especially if the point of this RFC is that auditing the assembly is not an option. A deny-by-default lint is silent when it fires in a dependency, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm unclear if anyone outside of a crypto library would ever use this for "security". Does an everyday user need to be forcing inlining when writing a webserver or whatever the heck? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah, you need to trust your dependencies which use
See #3711 (comment) for an example of the type of function where There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not trying to be too concerned about the process of it all, but would this idea better fit a Compiler MCP, rather than a general RFC? If it's intended to just eternally be some implementation detail. Generally, users expect RFCs to result in something that they can eventually use on stable. That also might reduce the bikeshed. |
||
Callers of annotated items can always override this lint with the usual | ||
`#[allow]`/`#[warn]`/`#[deny]`/`#[expect]` attributes. For example: | ||
|
||
```rust | ||
// somewhere/in/std/intrinsics.rs | ||
#[inline(required)] | ||
pub unsafe fn ptrauth_auth_and_load_32<const KEY: PtrAuthKey, const MODIFIER: u64, const OFFSET: u32>(value: u64) -> u32 { | ||
/* irrelevant detail */ | ||
} | ||
|
||
// main.rs | ||
#[warn(required_inline)] | ||
fn do_ptrauth_warn() { | ||
intrinsics::ptrauth_auth_and_load_32(/* ... */); | ||
//~^ WARN `ptrauth_auth_and_load_32` could not be inlined but requires inlining | ||
} | ||
``` | ||
|
||
Both `#[inline(must)]` and `#[inline(required)]` can optionally provide | ||
the user a justification for why the annotated item is enforcing inlining, | ||
such as `#[inline(must("maintain performance characteristics"))]` or | ||
`#[inline(required("uphold security properties"))]`. | ||
|
||
Failures to force inline should not occur sporadically, users will only | ||
encounter this lint when they call an annotated item from a location that the | ||
inliner cannot inline into (e.g. a coroutine - a current limitation of the MIR | ||
inliner), or when compiling with incompatible options (e.g. with code coverage - | ||
a current limitation of the MIR inliner/code coverage implementations). | ||
|
||
`#[inline(required)]` and `#[inline(must)]` differs from `#[inline(always)]` in | ||
that it emits the lint when inlining does not happen and rustc will guarantee | ||
that no heuristics/optimization fuel considerations are employed to consider | ||
whether to inline the item. | ||
|
||
`#[inline(must)]` and `#[inline(required)]` are intended to remain unstable | ||
indefinitely and be used only within the standard library (e.g. on intrinsics). | ||
This is to avoid misuse of the attribute which ultimately harms performance | ||
of Rust programs. As these attributes can be used in the standard library | ||
on the relevant intrinsics which require inlining for performance or security | ||
reasons, they will still be useful despite being indefinitely unstable. These | ||
could be stabilized if there were sufficient motivation for use of these | ||
inlining attributes in user code. | ||
|
||
# Reference-level explanation | ||
[reference-level-explanation]: #reference-level-explanation | ||
|
||
As LLVM does not provide a mechanism to require inlining, only a mechanism | ||
to provide hints as per the current `#[inline]` attributes, `#[inline(must)]` | ||
and `#[inline(required)]` will be implemented in rustc as part of the MIR | ||
inliner. Small modifications to the MIR inliner will make the MIR pass run | ||
unconditionally (i.e. with `-O0`), while only inlining non-`#[inline(must)]`/ | ||
`#[inline(required)]` items under the current conditions and always inlining | ||
`#[inline(must)]`/`#[inline(required)]` items. Any current limitations of the | ||
MIR inliner will also apply to `#[inline(must)]`/`#[inline(required)]` items and | ||
will be cases where the lint is emitted. `#[inline(must)]`/`#[inline(required)]` | ||
will be considered an alias of `#[inline(always)]` after MIR inlining when | ||
performing codegen. | ||
|
||
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
- It may be undesirable for the MIR inliner to be necessary for the correctness | ||
of a Rust program (i.e. for the `#[inline(required)]` case). | ||
|
||
# Rationale and alternatives | ||
[rationale-and-alternatives]: #rationale-and-alternatives | ||
|
||
- An "intrinsic macro" type of solution could be devised for the "security | ||
properties" use case instead as macros are inherently inlined. | ||
- Given the existence of security features with intrinsics that must be inlined | ||
to guarantee their security properties, not doing this (or something else) | ||
isn't a viable solution unless the project decides these are use cases that the | ||
project does not wish to support. | ||
- Instead of having two attributes and two lints, merge both attributes and lints | ||
and require users use `#[deny(..)]` when calling those intrinsics which must be | ||
inlined to uphold security properties. This RFC prefers to have two lints and | ||
attributes so that the annotation on the intrinsic decides whether it is | ||
error-worthy for it to not be inlined. | ||
|
||
# Prior art | ||
[prior-art]: #prior-art | ||
|
||
gcc and clang both have partial equivalents (e.g. [clang](https:// | ||
clang.llvm.org/docs/AttributeReference.html#always-inline-force-inline)) | ||
which ignore their inlining heuristics but still only guarantee that they | ||
will attempt inlining, and do not notify the user if inlining was not possible. | ||
|
||
# Unresolved questions | ||
[unresolved-questions]: #unresolved-questions | ||
|
||
There aren't any unresolved questions in this RFC currently. | ||
|
||
# Future possibilities | ||
[future-possibilities]: #future-possibilities | ||
|
||
This feature is fairly self-contained and doesn't lend itself to having future expansion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know exactly what words you meant here, but "have work poorly" is not it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, yeah, just missed that while rewriting this sentence at some point.