-
Notifications
You must be signed in to change notification settings - Fork 13.7k
deduplicate abort implementations #139103
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
Currently, the code for process aborts is duplicated across `panic_abort` and `std`. This PR uses `#[rustc_std_internal_symbol]` to make the `std` implementation available to `panic_abort` via the linker, thereby deduplicating the code.
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -328,8 +328,13 @@ pub fn dur2timeout(dur: Duration) -> u32 { | |||||||||||||||||||||
|
||||||||||||||||||||||
/// Use `__fastfail` to abort the process | ||||||||||||||||||||||
/// | ||||||||||||||||||||||
/// This is the same implementation as in libpanic_abort's `__rust_start_panic`. See | ||||||||||||||||||||||
/// that function for more information on `__fastfail` | ||||||||||||||||||||||
/// In Windows 8 and later, this will terminate the process immediately without | ||||||||||||||||||||||
/// running any in-process exception handlers. In earlier versions of Windows, | ||||||||||||||||||||||
/// this sequence of instructions will be treated as an access violation, | ||||||||||||||||||||||
/// terminating the process but without necessarily bypassing all exception | ||||||||||||||||||||||
/// handlers. | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Let's use the same verb both times, otherwise this is unnecessarily confusing since it uses "without" twice to actually make statements that are opposites of each other. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've rephrased the comment a bit to make it even clearer. |
||||||||||||||||||||||
/// | ||||||||||||||||||||||
/// https://docs.microsoft.com/en-us/cpp/intrinsics/fastfail | ||||||||||||||||||||||
#[cfg(not(miri))] // inline assembly does not work in Miri | ||||||||||||||||||||||
pub fn abort_internal() -> ! { | ||||||||||||||||||||||
unsafe { | ||||||||||||||||||||||
|
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 they get different symbol names? Having the same extern signature reused but distinguished by attributes is a bit tricky.
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.
__rust_abort
is the obvious name for the symbol shared betweenpanic_abort
andstd
, so I'm a bit hesitant about changing that. Changing the SGX symbol seems better, but it's a lot of effort as it requires changing libunwind as well; I don't really think it's worth 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.
Symbols marked as
#[rustc_std_internal_symbol]
don't really need the__rust_
prefix anymore now that we do symbol mangling for them too.