-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
RFC: Interrupt calling conventions #3246
Open
phil-opp
wants to merge
13
commits into
rust-lang:master
Choose a base branch
from
phil-opp:interrupt-calling-conventions
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
3421407
Start the interrupt calling convention RFC
phil-opp 8442e71
First draft on motivation and guide-level explanation sections
phil-opp f54ce99
Write reference-level explanation, drawbacks, and alternatives section
phil-opp 5ae3d65
Prior art, open questions, and future possibilities
phil-opp 5e8cb56
Fix typos
phil-opp d72e615
Various improvements after proof-reading
phil-opp 92bf2fe
Stabilizations for tier 3 should only be done with caution
phil-opp a2f8c29
Clarify the stability and platform support of interrupt calling conve…
phil-opp fe789a8
Some architectures might have multiple different interrupt calling co…
phil-opp c584ec1
Fix typo
phil-opp b2ffad4
Mention `rust_codegen_gcc`
phil-opp 719b02c
Add more details on x86-interrupt calling convention
phil-opp 38a2a9f
Extend paragraph about register backup/restore in motivation
phil-opp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
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.
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.
This needs to say that interrupt routines are implicitly unsafe. Even if the interrupt routine only calls safe Rust functions, it can easily observe intermediate CPU states that are beyond the Rust safety model, or corrupt the state so that undefined behavior occurs after the interrupt routine returns.
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.
Could you clarify what you mean by "implictly unsafe"? That they should be treated like an
unsafe fn
so that nounsafe {}
blocks are required in it?I agree this question is something that we should specify in the RFC in more detail. My opinion on this is that we should keep the function body safe, but make all dangerous operations unsafe:
x86_64
crate requires anunsafe
block to modify the passedInterruptStackFrame
. Ideally, thisunsafe
block should be required directly by the compiler, for example by requiring raw pointer arguments instead of references for thex86-interrupt
calling convention.I'm happy to discuss this further!
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.
What I meant is that even if the function is not marked as unsafe, the usual rules about Rust memory safety do not apply at all to these functions. There are a lot of restrictions that have to be followed, depending on the rest of the system, and I'm not sure if Rust is actually up for that. (With C or C++, separate compilation with different compiler flags is sometimes used.)
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.
That seems like a case by case basis as to how safe each ABI is going to be. The interrupt handler function ABI could (for example) have additional pre and post code inserted by rustc automatically so that the user's source code always runs in a safe environment.
Like we can literally make up these ABIs to do whatever, since normal code is never going to interface with the interrupt handler code.
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.
@fweimer can you elaborate a bit more about what rules re: Rust memory safety are inapplicable? The function doesn't get access to anything but static data, right?
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.
Ah, I see your point. Well then at least the situations I was talking about are fine enough if the compiler is handling it. Though, perhaps fweimer had something else in mind.
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.
@nikomatsakis Some mutex implementations have fast paths for single-threaded operation that do not do anything. I think this would enable an interrupt routine to access data behind a mutex while the interrupted program uses it. (We change
pthread_mutex_lock
to always perform the mutex update only in EDIT glibc 2.34—starting with that version, single-threaded processes should self-deadlock if the mutex is acquired twice.)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.
Once you introduce interrupts you are effectively no longer a single-threaded process as the easiest way to model interrupts in the rust abstract machine is as running on a separate thread which could synchronize with the non-interrupt thread using regular concurrency methods. It just so happens that while the interrupt runs, the non-interrupt thread doesn't make any progress and thus you may deadlock if you wait for 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.
I don't think it can be treated the same as a separate thread precisely because it blocks the non-interrupt code while running and is identified as the same thread.
I've equated it to a signal handler before, which is far more limited in what it can do.
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.
@bjorn3 The single-thread optimization is an internal C runtime thing that gets disabled if you create another thread using a C runtime facility. The C runtime won't know about the interrupt code and therefore cannot treat the application as multi-threaded.