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.
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
base: master
Are you sure you want to change the base?
RFC: Interrupt calling conventions #3246
Changes from all commits
3421407
8442e71
f54ce99
5ae3d65
5e8cb56
d72e615
92bf2fe
a2f8c29
fe789a8
c584ec1
b2ffad4
719b02c
38a2a9f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Maybe use usize as error code type instead?
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 think it makes sense being explicit here.
usize
isn't necessarily defined to be the width of a register.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.