CFI: Only erase leading non-passed arguments #123438
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, we had `NO_SELF_TYPE_ERASURE`, a negative configuration. Now we have `ERASE_SELF_TYPE`, a positive configuration.
KCFI needs to be able to tell which kind of `ReifyShim` it is examining in order to decide whether to use a concrete type (`FnPtr` case) or an abstract case (`Vtable` case). You can *almost* tell this from context, but there is one case where you can't - if a trait has a method which is *not* `#[track_caller]`, with an impl that *is* `#[track_caller]`, both the vtable and a function pointer created from that method will be `ReifyShim(def_id)`. Currently, the reason is optional to ensure no additional unique `ReifyShim`s are added without KCFI on. However, the case in which an extra `ReifyShim` is created is sufficiently rare that this may be worth revisiting to reduce complexity.
Adds support for both CFI and KCFI for attaching concrete and abstract types to functions. KCFI does this through generation of `ReifyShim` on any function pointer that could go in a vtable, and checking the `ReifyReason` when emitting the instance. CFI does this by attaching both the concrete and abstract type to every instance. TypeID codegen tests are switched to be anchored on the left rather than the right in order to allow emission of additional type attachments. Fixes rust-lang#115953
Since we're now using an approach which is a variant of rust-lang#123082 (that transforms closures into dynamic Fn traits but isolating it to the Fn call methods only) instead of rust-lang#121962 or rust-lang#122573, skipping non-passed arguments shouldn't be necessary for KCFI anymore and we can claim back the reduced granularity. This reverts commit f2f0d25.
The purpose of erasing non-passed arguments was to support the compiler's implicit cast between `call(ZSTClosure, (T, U, ..))` and `fn(T, U, ..)`. We can narrow this erasure by only performing it if the call Abi was "rust-call".
rustbot
added
PG-exploit-mitigations
|
☔ The latest upstream changes (presumably #123455) made this pull request unmergeable. Please resolve the merge conflicts. |
|
As I mentioned to you on Zulip already, this doesn't really solve the problem, it just gates removing all non-passed arguments of Rust ABI functions/methods/calls, which is actually only where they're applicable/exist and interest us, behind a typeid option. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR switches to only erasing the first non-passed argument, and only when it's a "rust-call" function.
r? @compiler-errors
cc @rcvalle
I've left this as draft because I don't want to try to land adjustments until KCFI function pointers are in, and they just failed bors for unknown reasons.