Remove -Zoom=panic #147725
Remove -Zoom=panic #147725
Conversation
| The Miri subtree was changed cc @rust-lang/miri Some changes occurred in compiler/rustc_codegen_ssa Some changes occurred in compiler/rustc_codegen_gcc |
rustbot added A-LLVM rustbot added T-compiler 
This comment has been minimized.
This comment has been minimized.
bjorn3 force-pushed the remove_oom_panic branch from 0bdf920 to c159b2b Compare This comment has been minimized.
This comment has been minimized.
bjorn3 force-pushed the remove_oom_panic branch from c159b2b to 8800809 Compare 
| Cc @rust-lang/wg-allocators |
| I've been waiting for At Cloudflare we were waiting for ability to use it in servers. It was part of the RFC https://github.com/rust-lang/rfcs/blob/master/text/2116-alloc-me-maybe.md#user-profile-server. We currently aren't only because we use stable Rust, and have a ton of hacks in place already put in due to suffering from lack of this feature. In some places we do rely on panics from allocators not being totally UB any more. Is removal of |
| You can still panic in the alloc error hook you register with |
In fact I changed the existing |
| ☔ The latest upstream changes (presumably #147745) made this pull request unmergeable. Please resolve the merge conflicts. |
| r? @Amanieu maybe |
There are major questions remaining about the reentrancy that this allows. It doesn't have any users on github outside of a single project that uses it in a panic=abort project to show backtraces. It can still be emulated through #[alloc_error_handler] or set_alloc_error_hook depending on if you use the standard library or not. And finally it makes it harder to do various improvements to the allocator shim.
bjorn3 force-pushed the remove_oom_panic branch from 05a4290 to 0c3cdac Compare | This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
| The job Click to see the possible cause of the failure (guessed by this bot) |
| Reducing the number of symbols used by the global allocation interface is a valid argument, and I do agree that it should be over But the reentrancy concerns aren't a reason to remove oom panic support, because the same exact concerns still exist without oom panic, both panics within the allocator (that don't escape) and unwinds from the alloc (error) handler. Personally I think handling (non-pre-main) OOM through the same panic machinery as standard code panics is the correct thing to do for global general purpose "infallible" allocations, since it's the same general style of "allegedly impossible" error, but I can see logic in saying it's notable enough to use its own specialized machinery, instead of the heavily generic and indirected panic machinery. Consider this a neutral vote from me, I suppose. |
| By the way the awkward thing with |
9 participants
There are major questions remaining about the reentrancy that this allows. It doesn't have any users on github outside of a single project that uses it in a panic=abort project to show backtraces. It can still be emulated through
#[alloc_error_handler]orset_alloc_error_hookdepending on if you use the standard library or not. And finally it makes it harder to do various improvements to the allocator shim.This also adds support for showing backtraces on the first call of the libstd alloc error handler to satisfy the needs of this single project on github that uses it and because it seems generally useful to have. If an allocation while printing the backtrace fails, we don't try to print another backtrace as that will never succeed.
With this PR the sole remaining symbol in the allocator shim that is not effectively emulating weak symbols is the symbol that prevents skipping the allocator shim on stable even when it would otherwise be empty because libstd +
#[global_allocator]is used.Closes #43596
Fixes #126683