Implement immediate-abort by hooking #[rustc_panic_entrypoint] and lint for functions that look like a panic entrypoint#150497
Implement immediate-abort by hooking #[rustc_panic_entrypoint] and lint for functions that look like a panic entrypoint#150497saethlin wants to merge 2 commits intorust-lang:mainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #150386) made this pull request unmergeable. Please resolve the merge conflicts. |
Should be possible to check |
9907096 to
3dcb309
Compare
Yep. But it turns out |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Implement immediate-abort by hooking #[rustc_panic_entrypoint] and lint for functions that look like a panic entrypoint
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (0de2b26): comparison URL. Overall result: ❌ regressions - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary 0.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 2.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.0%, secondary -0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 480.336s -> 480.598s (0.05%) |
compiler/rustc_lint/src/builtin.rs
Outdated
| } | ||
| } | ||
|
|
||
| declare_lint! { |
There was a problem hiding this comment.
This should be an internal lint
There was a problem hiding this comment.
Should it? Internal lints are enabled on the compiler crates, and I only want this on the standard library.
There was a problem hiding this comment.
I don't understand how this could be an internal lint. Can you explain? I don't want this enabled on the compiler, and that seems to be the definition of internal lints.
b8ef8e7 to
848a6ad
Compare
This comment has been minimized.
This comment has been minimized.
|
I'm lost. How did trying to add an internal lint (which uses completely different macros from adding a builtin lint??) remove another lint? |
|
☔ The latest upstream changes made this pull request unmergeable. Please resolve the merge conflicts. |
|
☔ The latest upstream changes (presumably #150628) made this pull request unmergeable. Please resolve the merge conflicts. |
b500b01 to
f13fc5f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
4893cc0 to
ddb4280
Compare
d3004bf to
64dfd97
Compare
This comment has been minimized.
This comment has been minimized.
64dfd97 to
48494a0
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
48494a0 to
92bd47c
Compare
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Implement immediate-abort by hooking #[rustc_panic_entrypoint] and lint for functions that look like a panic entrypoint
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (e6cf4f6): comparison URL. Overall result: ❌ regressions - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -1.3%, secondary 0.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -8.4%, secondary -1.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.0%, secondary 0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 480.702s -> 486.725s (1.25%) |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Implement immediate-abort by hooking #[rustc_panic_entrypoint] and lint for functions that look like a panic entrypoint
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (d8b0d0e): comparison URL. Overall result: ❌ regressions - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 5.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -7.9%, secondary -5.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.0%, secondary -0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 481.904s -> 483.897s (0.41%) |
There are a handful of functions, mostly in
core, which have a mess of attributes on them which try to ensure that they compile down to justintrinsics::abort()whencfg(panic = "immediate-abort"), and otherwise are outlined. These functions now all get#[rustc_panic_entrypoint]which makes the functions#[cold],#[inline(never)], and#[optimize(size)]. But when immediate-abort is enabled, these functions are#[inline].And since these functions are now all known to the compiler, when immediate-abort is enabled, both their bodies and calls to them are directly replaced
intrinsics::abort(). Replacing both body and calls means that we rely much less on optimization to make immediate-abort do its job, but if for some godforsaken reason you are indirectly calling a panic entrypoint, that will still immediately abort.There is a fair bit of inconsistency in what attributes are on panic entrypoints, only some have
#[optimize(size)]and about half have#[inline(never)]. The single attribute makes sure they are all treated the same. (I am making an informed guess that the inconsistency was accidental, and mostly not impactful)This PR also adds an allow-by-default lint,
missing_panic_entrypointwhich looks for diverging functions that do not have one of#[rustc_panic_entrypoint]or#[inline]. Panic wrappers sometimes have#[inline]because they are theconst fnfor aconst_eval_select, so adding#[inline]ensures that monomorphization doesn't eagerly codegen a useless copy of them.The net effect is that most but not all of the cfgs for immediate-abort are gone, and the hack in cg_ssa that replaced any upstream diverging function call with
abortcan now check specifically for panic entrypoints.