Skip to content

compiler: Allow extern "interrupt" fn() -> ! #143075

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

workingjubilee
Copy link
Member

While reviewing #142633 I overlooked a few details because I was kind of excited.

Fixes #143072

@rustbot
Copy link
Collaborator

rustbot commented Jun 27, 2025

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-test-infra-minicore Area: `minicore` test auxiliary and `//@ add-core-stubs` S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 27, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 27, 2025

This PR modifies tests/auxiliary/minicore.rs.

cc @jieyouxu

@folkertdev
Copy link
Contributor

folkertdev commented Jun 27, 2025

Now that the rules get more complex, I think the error on the return type should probably move to rustc_hir_typeck. The current implementation cannot reason about type aliases, so

type Unit = ();

#[cfg(any(x64, i686))]
extern "x86-interrupt" fn x86_ret_unit() -> Unit {
    unsafe { hint::unreachable_unchecked() }
}

gives

error: invalid signature for `extern "x86-interrupt"` function
  --> /home/folkertdev/rust/rust/tests/ui/abi/interrupt-returns-never-or-unit.rs:86:45
   |
LL | extern "x86-interrupt" fn x86_ret_unit() -> Unit {
   |                                             ^^^^
   |
   = note: functions with the "x86-interrupt" ABI cannot have a return type
help: remove the return type
  --> /home/folkertdev/rust/rust/tests/ui/abi/interrupt-returns-never-or-unit.rs:86:45
   |
LL | extern "x86-interrupt" fn x86_ret_unit() -> Unit {
   |                                             ^^^^

error: aborting due to 1 previous error

The error message is also incorrect now: the function can have a return type (so long as it's black () or !)

@workingjubilee workingjubilee force-pushed the interrupts-may-return-nevermore branch from 765309e to b5ab966 Compare June 27, 2025 18:05
@workingjubilee
Copy link
Member Author

I mean if I really want to get it right it should probably be even deeper, so we can check the actual layout.

@workingjubilee
Copy link
Member Author

I don't think I want to do that level of correctness before we decide we actually want these interrupt ABIs, though, and I, at least, am not sold on them.

@andrewoffenden

This comment was marked as off-topic.

@folkertdev
Copy link
Contributor

Fair enough. Then the error message should at least be correct though right? (so, there can be a return type, just literally () and ! are allowed).

before we decide we actually want these interrupt ABIs, though, and I, at least, am not sold on them.

The x86 one gets a surprising (to me) amount of use https://github.com/search?q=extern+%22x86-interrupt%22&type=code. But in theory those could now use naked functions and compile on stable.

@workingjubilee
Copy link
Member Author

@andrewoffenden what.

@workingjubilee
Copy link
Member Author

workingjubilee commented Jun 27, 2025

Fair enough. Then the error message should at least be correct though right? (so, there can be a return type, just literally () and ! are allowed).

before we decide we actually want these interrupt ABIs, though, and I, at least, am not sold on them.

The x86 one gets a surprising (to me) amount of use https://github.com/search?q=extern+%22x86-interrupt%22&type=code. But in theory those could now use naked functions and compile on stable.

Yes, Phil Opp espoused using them, so they gained some popularity. But they should not be used by a real OS, because an OS cannot support any debugger APIs if it uses them.

@Freax13
Copy link
Contributor

Freax13 commented Jun 28, 2025

I think there are valid use-cases for the "x86-interrupt" ABI even in a "real OS". Sometimes you don't need to look at the registers of the interrupted thread, and in that case, using "x86-interrupt" is completely fine and can even result in better code than a hand-written interrupt handler. For example, I use it for handling timer interrupts that happen while the kernel is executing in ring 0. That said, there are, of course, also use cases where this absolutely will not work: If you're going to write an int 0x80 handler, you'll very likely need to hand roll your own assembly.

@phil-opp
Copy link
Contributor

Apart from convenience, my main motivation for using the x86-interrupt calling convention was that it generates better code than you can write manually for short interrupt handlers. It backups only the registers that are used by the handler implementation. With manual inline assembly, you have to always backup all caller-saved registers.

That being said, I also agree that the x86-interrupt calling convention is a burden to maintain. The LLVM implementation is not really used by many people, so it's not properly tested and breaks relatively often. It's also quite a complex calling convention because of how the x86 architecture is designed (e.g. for some exceptions, an additional error code is pushed to stack). So I would fully understand if we don't want to have this complexity inside rustc.

Thanks a lot for this PR by the way! I appreciate it that you try to avoid breakage, even though it's a nightly feature!

@phil-opp
Copy link
Contributor

(Not directly related to this PR, but to discussion about whether we want to keep the x86-interrupt calling convention or not:

I think if we had support for LLVM's PreserveAll or PreserveMost calling conventions, we could use that instead of x86-interrupt (in combination with a naked wrapper function). This way, we could keep the complexity of the x86 interrupt stack frame out of rustc, bit still have the performance advantage of saving only the registers that will be overwritten.)

@workingjubilee
Copy link
Member Author

workingjubilee commented Jun 28, 2025

We do support PreserveMost already as extern "rust-cold", but I don't know if it works the way you need it to, @phil-opp. We can add a full-on "rust-save" though.

I agree that we really do need that as a prerequisite to any alternatives. That way any assembly can be a minimal shim sufficient to do whatever operations are needed between the machine and the OS and then jump into the main handler, instead of saving everything in the assembly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-test-infra-minicore Area: `minicore` test auxiliary and `//@ add-core-stubs` S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

x86-interrupt ABI no longer compiles with never result
7 participants