-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Ignore panic functions in backtraces. Print inlined functions on Windows #45637
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
Closed
Closed
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
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 hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,25 +48,38 @@ pub fn panic(expr_file_line_col: &(&'static str, &'static str, u32, u32)) -> ! { | |
// Arguments::new_v1 may allow the compiler to omit Formatter::pad from the | ||
// output binary, saving up to a few kilobytes. | ||
let (expr, file, line, col) = *expr_file_line_col; | ||
panic_fmt(fmt::Arguments::new_v1(&[expr], &[]), &(file, line, col)) | ||
panic_dispatch(fmt::Arguments::new_v1(&[expr], &[]), &(file, line, col), &(panic as usize)) | ||
} | ||
|
||
#[cold] #[inline(never)] | ||
#[lang = "panic_bounds_check"] | ||
fn panic_bounds_check(file_line_col: &(&'static str, u32, u32), | ||
index: usize, len: usize) -> ! { | ||
panic_fmt(format_args!("index out of bounds: the len is {} but the index is {}", | ||
len, index), file_line_col) | ||
index: usize, len: usize) -> ! { | ||
panic_dispatch(format_args!("index out of bounds: the len is {} but the index is {}", | ||
len, index), | ||
file_line_col, | ||
&(panic_bounds_check as usize)) | ||
} | ||
|
||
#[cold] #[inline(never)] | ||
pub fn panic_fmt(fmt: fmt::Arguments, file_line_col: &(&'static str, u32, u32)) -> ! { | ||
panic_dispatch(fmt, file_line_col, &(panic_fmt as usize)); | ||
} | ||
|
||
#[cold] | ||
fn panic_dispatch(fmt: fmt::Arguments, | ||
file_line_col: &(&'static str, u32, u32), | ||
entry_point: &usize) -> ! { | ||
#[allow(improper_ctypes)] | ||
extern { | ||
#[lang = "panic_fmt"] | ||
#[unwind] | ||
fn panic_impl(fmt: fmt::Arguments, file: &'static str, line: u32, col: u32) -> !; | ||
fn panic_impl(fmt: fmt::Arguments, | ||
file: &'static str, | ||
line: u32, | ||
col: u32, | ||
entry_point: &usize) -> !; | ||
} | ||
let (file, line, col) = *file_line_col; | ||
unsafe { panic_impl(fmt, file, line, col) } | ||
} | ||
unsafe { panic_impl(fmt, file, line, col, entry_point) } | ||
} | ||
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. Missing newline |
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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.
How come this is passed as
&usize
? Can't this just be ausize
?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 passing locals by reference also prevents tail call optimization. I'm not really sure that this or the
asm!
trick works for function which never return. Currently these do not get tail call optimization applied, but I don't see a reason for that. Perhaps LLVM skips that optimization on cold functions?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'm a little wary to have this and rely on this, are we sure that this doesn't get optimized away even with LTO? If we leave this in can it at least be documented in the source as to why it's a reference?
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.
My guess for why this isn't optimized is because
fmt::Arguments
is a pretty big struct that's copied around the stack, and LLVM is probably conservative with the tail-call for now. Despite that though this seems like something that's pretty brittle to rely on?