Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pnkfelix (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

I like it! There was a lot of reverse engineering to get interrupt handlers to compile correctly for Redox, it would be nice if the compiler did more work

While I appreciate that naked_fns exist, this would be way, way nicer. 👍

/cc @nikomatsakis who IIRC had some small concerns about naked fn, as well as @nagisa I think?

oh and @hawkw

This is definitely going to make my life much easier, thanks @phil-opp!

I can confirm the difficulties you mention with naked functions; it's not always obvious when using Rust code in a function marked as naked will cause a prologue to be inserted. This often leads to rather hard to diagnose double faults, and so on. Naked functions are certainly a useful tool, but having the interrupt calling convention is much nicer.

I'll be awaiting this PR eagerly.

So my opinion on interrupt CCs haven't changed at all since the last interrupt convention PR, so I won't bother with arguing about it yet again.

I want to call out some points though:

• No inline assembly needed: if you're writing a kernel you will use assembly one way or another. In the end you have to understand how the CPU works in order to write a kernel. You cannot avoid it. It is a fair point that using naked functions is a pain and I agree with it.
• Higher performance: I already stated in my comment at the PR linked above that this point is mostly moot with the preserve_all and preserve_most calling conventions, where the callee saves all registers it uses itself. These preserve conventions may even have less problems than interrupt CCs (do not save xmm registers unnecessarily for example, and saves are properly aligned).

At this point there's just about two things I'm worried about. How hard is it to support interrupt CCs for less common platforms such as ppc, MIPS etc. And how willing will we be to remove current buggy impl of naked functions.

Seems like the RFC should be revisited (if only with a "we'll call it experimental with no plans to stabilize" outcome) if this is going to be supported. I'm happy to update it if there's demand.

@nagisa

if you're writing a kernel you will use assembly one way or another.

I didn't want to imply that assembly-free kernel are possible with this. My point was that you don't need inline assembly for interrupt handlers (compared to the naked function approach).

I already stated in my comment at the PR linked above that this point is mostly moot with the preserve_all and preserve_most calling conventions, where the callee saves all registers it uses itself.

AFAIK, the x86-interrupt cc is very similar to preserve_all. It just adds some additional magic such as checking for a valid function signature, passing the exception stack frame pointer/error code, popping the error code off the stack before returning, and using iret instead of ret for returning. Another difference is that it also preserves r11 and supports 32-bit x86 systems (unlike preserve_all). But apart from that, they seem to use the same mechanisms.

So the x86-interrupt cc does not save all registers, but only the registers used by the callee. For example, consider this code:

extern "x86-interrupt" fn handler(stack_frame: &ExceptionStackFrame) {
    use core::sync::atomic::{AtomicUsize, Ordering};
    static C: AtomicUsize = AtomicUsize::new(0);

    C.fetch_add(1, Ordering::Relaxed);
}

The function only increases an atomic counter. The compiler generates the following code with optimizations:

0000000000103390 <blog_os::interrupts::breakpoint_handler::hfa8af4eac17d93e9>:
  103390:	fc                   	cld    
  103391:	f0 48 ff 05 67 bc 00 	lock incq 0xbc67(%rip)
  103398:	00 
  103399:	48 cf                	iretq  
  10339b:	0f 1f 44 00 00       	nopl   0x0(%rax,%rax,1)

The function uses no registers, so there's no need to save/restore them.

These preserve conventions may even have less problems than interrupt CCs (do not save xmm registers unnecessarily for example, and saves are properly aligned).

The alignment problem is caused by the error code that is pushed by the CPU for certain exceptions. Thus the stack does not have the 16-byte alignment that the x86_64 ABI requires on function entry. So using preserve_all would have the same problem.

I'm not sure about the cause of the xmm issue, though. It might be because x86_64 requires SSE support, so that nobody thought of the "x86-64 without sse" case. I suspect that the preserve_all calling convention has the same issue.

AFAIK, the x86-interrupt cc is very similar to preserve_all. It just adds some additional magic such as checking for a valid function signature,

The alignment problem is caused by the error code that is pushed by the CPU for certain exceptions. Thus the stack does not have the 16-byte alignment that the x86_64 ABI requires on function entry.

You're missing my point entirely I think. I'm not proposing use of of preserveall as a direct interrupt handler at all. Instead what I was suggesting is use of a naked function trampoline into such preserve all function, which avoids all the pushing and popping cost described in the PR description. Such trampoline will realign stack pointer just fine avoiding the misaligned push problem as well.

If anything, all you managed to do is convincing me that generic interrupt CC with automatically generated trampolines to a preserveall function is a good idea.

Ah, now I get it :). I agree that "naked wrapper trampoline to preserve_all function" is a good and more general solution. However, the performance will be a bit worse than with the x86-interrupt cc, because you have to backup/restore r11 (scratch register in preserve_all cc), rdi, and rsi (used for passing the arguments) and to issue a function call.

Both x86-interrupt and preserve_all have direct LLVM support and only require minimal changes, so I would propose to add support for both.

Both x86-interrupt and preserve_all have direct LLVM support and only require minimal changes, so I would propose to add support for both.

My two cents is that this is the right way to go. I can see use-cases for both, and if the implementation isn't too difficult, why not provide multiple options? The performance benefit from x86-interrupt is nice...

I'm not sure about the cause of the xmm issue, though. It might be because x86_64 requires SSE support, so that nobody thought of the "x86-64 without sse" case. I suspect that the preserve_all calling convention has the same issue.

I can confirm I wasn't considering that case when I wrote the code implementing the interrupt CC. Couldn't say about preserve_all without looking into it.

nominating because I don't really know what to do with this PR. Main points to bring up:

• is it ready for review?
• who should review it; i do not feel like best team member to do so?
• does it need an RFC amendment (or its own RFC) before we potentially commit it?

It is a good PR to discuss during the mtg, indeed.

@pnkfelix do not forget to also tag the team when nominating an issue.

r? @nagisa

(update: manually assigning; nagisa pointed out that he is not on bors' list, and I cannot quickly determine the syntax, if any, to tell bors to assign the PR to nagisa.)

During yesterday’s meeting we’ve decided to proceed with landing this for now, however this will not be considered for stabilisation without a RFC. I, myself, will be investigating a general "interrupt" ABI wherein the compiler does the right thing depending on the target platform (falling back to generated trampolines) and maybe write an RFC if the results are favourable.

The implementation looks fine to me. I could importune a test of some sort, but it seems to me like one cannot really compose anything other than a codegen test or a test that calls the interrupt-abi function from rust to rust. Both of these are kind of useless, so, with or without a test…

r=me

I added a simple codegen test and squashed the commits.

@bors r=nagisa

📌 Commit 03fad81 has been approved by nagisa

Added min-llvm-version 3.8 to the codegen test. The failed travis check is green now.

@bors r=nagisa

📌 Commit ba66d92 has been approved by nagisa

⌛ Testing commit ba66d92 with merge 4ea6605...

💔 Test failed - status-travis

@bors retry

• OSX linker segfaulting on Travis #38878

☔ The latest upstream changes (presumably #40206) made this pull request unmergeable. Please resolve the merge conflicts.

Rebased

@bors: r=nagisa

📌 Commit b448058 has been approved by nagisa

⌛ Testing commit b448058 with merge c0b7112...

Don't other architectures also have specialized ABIs for interrupts? Wouldn't it be more reasonable to have a generalized extern "interrupt" that works on whatever architecture you're compiling for? Even if we don't implement them all for all of the architectures Rust supports, it seems like a better practice to encourage.

The RFC touches on that, but as I see it the most important points:

• Some architectures have more than one interrupt CC ("fast" and regular interrupts on ARM, for example)
• Interrupt handlers often need to read system-specific context from the stack- it may be easier or clearer to have visibly distinct CCs rather than a bunch of different #[cfg]'d implementations for portable software.

Some architectures have more than one interrupt CC ("fast" and regular interrupts on ARM, for example)

Some platforms have more than one C ABIs and yet we still have extern "C" defaulting to one of them.
extern "interrupt" can do this to, e.g. to default to IRQ on ARM.

Hey,

since the compiler knows which registers are clobbered by the interrupt handler

Sometimes, this has nothing to do with the registers actually used by the interrupt handler.
Am i the only one thinking that some interrupt handlers do have to read/write saved registers. For example, to give a return value to a system call, handling userland signal trampolines ... ?

Is there any way to force saving all registers using this convention, and access them in the interrupt handler (ie. extended ExceptionStackFrame) ?

If not, then the use of naked function will be the only option ...

For example, to give a return value to a system call, handling userland signal trampolines ... ?

Each of those sounds to me like distinct calling conventions. To use the system call example, handling the x86 syscall instruction will usually involve preserving R11 and RCX but ultimately has implementation-defined semantics.

It might be reasonable to define a different calling convention that acts like save-all but makes the machine's entry state available, but I'm not convinced it would be worthwhile since exposing a struct would inherently require spilling all registers (which you can easily do yourself) and making each register a unique parameter is both cumbersome (you end up with a large number of parameters, many of which may go unused) and requires user code be aware of what resources exist (which is what you need to do when writing a custom calling convention with naked functions).

Naked functions are the ultimate escape hatch for these situations, so the argument to be made is that there is an efficient way to expose the machine state on function entry while restoring it on exit which is sufficiently general to be worth the maintenance burden.