-
Notifications
You must be signed in to change notification settings - Fork 13.4k
error on calls to ABIs that cannot be called #142597
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
base: master
Are you sure you want to change the base?
Conversation
|
8a2bc53
to
e88e066
Compare
// Some ABIs cannot be called from rust, either because rust does not know how to generate | ||
// code for the call, or because a call does not semantically make sense. | ||
if !abi.can_be_called_with_call_expr() { | ||
self.tcx.dcx().emit_err(crate::errors::AbiCannotBeCalled { span, abi: abi.as_str() }); |
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.
In the case of interrupt
, we also do not want to reach any iret
instructions.
match self { | ||
// Rust doesn't know how to call functions with this ABI. | ||
ExternAbi::Custom => false, | ||
|
||
// These define entry points for the host, and cannot be called on the GPU. | ||
ExternAbi::PtxKernel | ExternAbi::GpuKernel => false, | ||
|
||
// The interrupt ABIs should only be called by the CPU. | ||
// They have complex pre- and postconditions. | ||
ExternAbi::AvrInterrupt | ||
| ExternAbi::AvrNonBlockingInterrupt | ||
| ExternAbi::Msp430Interrupt | ||
| ExternAbi::RiscvInterruptM | ||
| ExternAbi::RiscvInterruptS | ||
| ExternAbi::X86Interrupt => false, | ||
|
||
ExternAbi::C { .. } | ||
| ExternAbi::System { .. } | ||
| ExternAbi::Rust | ||
| ExternAbi::RustCall | ||
| ExternAbi::RustCold | ||
| ExternAbi::Unadjusted | ||
| ExternAbi::EfiApi | ||
| ExternAbi::Aapcs { .. } | ||
| ExternAbi::CCmseNonSecureCall | ||
| ExternAbi::CCmseNonSecureEntry | ||
| ExternAbi::Cdecl { .. } | ||
| ExternAbi::Stdcall { .. } | ||
| ExternAbi::Fastcall { .. } | ||
| ExternAbi::Thiscall { .. } | ||
| ExternAbi::Vectorcall { .. } | ||
| ExternAbi::SysV64 { .. } | ||
| ExternAbi::Win64 { .. } => true, | ||
} |
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.
Asking this about all ExternAbi
, instead of performing the analysis at the site and using a match
on CanonAbi
which allows handling the Interrupt
variant which includes all its nested variants, and also matches directly with whether we can generate code, seems... less than ideal. We should be rejecting anything that has a nonsensical mapping in rustc_ast_lowering
after #142134 lands, and hitting AbiMapping::Invalid
after trying to canonize_abi
for the case suggests something has gone direly wrong and we are fine if we ICE on 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.
So, do you want me to do that now? Or wait? Is there a better way of getting at a CanonAbi
than
match AbiMap::from_target(&self.sess().target).canonize_abi(abi, false) {
AbiMapping::Direct(canon_abi) | AbiMapping::Deprecated(canon_abi) => {
}
}
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.
Right now, there is not!
Probably should shove it in a field on Target at least.
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.
it's still not exactly clear to me what you want to see happen to this PR (versus what changes would be good to make down the line)
We recently added
extern "custom"
, which cannot be called using a rust call expression. But there are more ABIs that can't be called in that way, because the call does not semantically make sense.More details are in #140566 (comment)
r? @workingjubilee