Allow unwinding from OOM hooks #92535

Amanieu · 2022-01-03T21:03:58Z

This is split off from #88098 and contains just the bare minimum to allow specifying a custom OOM hook with set_alloc_error_hook which unwinds instead of aborting.

See #88098 for an actual command-line flag which switches the default OOM behavior to unwind instead of aborting.

Previous perf results show a negligible impact on performance.

rust-highfive · 2022-01-03T21:04:01Z

r? @m-ou-se

(rust-highfive has picked a reviewer for you, use r? to override)

m-ou-se · 2022-01-05T15:26:25Z

From your description I assumed that this program would abort in the current version of rust, and would succeed after the change in this PR:

#![feature(alloc_error_hook)]

fn main() {
    std::alloc::set_alloc_error_hook(|_| {
        panic!("oh no");
    });
    assert!(std::panic::catch_unwind(|| {
        vec![0u8; 12345678901234567890];
    }).is_err());
    println!("i'm ok :)");
}

But it already succeeds on the current version of rust:

thread 'main' panicked at 'oh no', src/main.rs:5:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
i'm ok :)

Am I misunderstanding the purpose of this PR?

bjorn3 · 2022-01-05T15:27:30Z

I think panicking inside the alloc error hook is currently UB.

Amanieu · 2022-01-05T15:33:57Z

Yes, it's currently UB due to extern "C" and #[rustc_allocator_nounwind] which imply nounwind but don't actually enforce it.

Amanieu · 2022-01-05T15:35:03Z

If you try using a type with Drop you will see that the drop gets optimized away because the compiler assumes the allocator doesn't unwind.

m-ou-se · 2022-01-05T15:38:54Z

Ah, interesting. I didn't manage to make it fail on x86_64-unknown-linux-gnu, aarch64-apple-darwin, nor x86_64-pc-windows-gnu, but it does indeed fail on aarch64-unknown-linux-musl and x86_64-unknown-linux-musl. With this patch applied, it succeeds on all of them.

@bors r+

bors · 2022-01-05T15:38:56Z

📌 Commit f4545cc has been approved by m-ou-se

m-ou-se · 2022-01-05T15:43:51Z

If you try using a type with Drop you will see that the drop gets optimized away because the compiler assumes the allocator doesn't unwind.

If I add a let _a = String::new(); in main, it seems to abort without unwinding before this patch. After this patch, it works fine.

Allow unwinding from OOM hooks This is split off from rust-lang#88098 and contains just the bare minimum to allow specifying a custom OOM hook with `set_alloc_error_hook` which unwinds instead of aborting. See rust-lang#88098 for an actual command-line flag which switches the default OOM behavior to unwind instead of aborting. Previous perf results show a negligible impact on performance.

ehuss · 2022-01-12T18:19:51Z

Oh, sorry, I didn't see that.

@bors r-

hkratz · 2022-01-12T18:24:02Z

Not really on Windows much these days, maybe someone from the Windows notification group can help?

@rustbot ping windows

rustbot · 2022-01-12T18:24:04Z

Error: Only Rust team members can ping teams.

Please let @rust-lang/release know if you're having trouble with this bot.

Amanieu · 2022-01-12T18:25:06Z

Just a backtrace at the point where it is hanging would be a good start to figuring out what the problem is.

bjorn3 · 2022-01-12T18:41:46Z

I can see this error in the log when it tries to build rls:
error[E0635]: unknown feature `thread_local_const_init`
  --> C:\Users\runneradmin\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\rustc-ap-rustc_span-722.0.0\src\lib.rs:23:12
   |
23 | #![feature(thread_local_const_init)]
   |            ^^^^^^^^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0635`.
Could this somehow be causing the hang?

Otherwise I have no idea how to go about debugging this.

No, rls fails to build on master too. See #91543.

ehuss · 2022-01-12T22:22:32Z

I'm able to reproduce locally. At a minimum it required llvm assertions, though it may also require parallel-compiler.

The reason it is hanging is because it pops open a helpful dialog box.

In this particular instance, it was hung building tracing-subscriber.

Here is a stack trace:

win32u.dll!NtUserWaitMessage+0x14
USER32.dll!DialogBoxIndirectParamAorW+0x431
USER32.dll!DialogBoxIndirectParamAorW+0x1a1
USER32.dll!SoftModalMessageBox+0x85b
USER32.dll!DrawStateA+0x1e25
USER32.dll!MessageBoxTimeoutW+0x1a7
USER32.dll!MessageBoxW+0x4e
rustc_driver-9acede3d415bc9ac.dll!__acrt_MessageBoxW+0x56
rustc_driver-9acede3d415bc9ac.dll!__acrt_show_wide_message_box+0x7c
rustc_driver-9acede3d415bc9ac.dll!common_assert_to_message_box<wchar_t>+0x74
rustc_driver-9acede3d415bc9ac.dll!llvm::InlineFunction+0x8366
rustc_driver-9acede3d415bc9ac.dll!llvm::InlinerPass::run+0x15d8
rustc_driver-9acede3d415bc9ac.dll!llvm::detail::PassModel<llvm::LazyCallGraph::SCC,llvm::InlinerPass,llvm::PreservedAnalyses,llvm::AnalysisManager<llvm::LazyCallGraph::SCC,llvm::LazyCallGraph & __ptr64>,llvm::LazyCallGraph & __ptr64,llvm::CGSCCUpdateResul
rustc_driver-9acede3d415bc9ac.dll!llvm::PassManager<llvm::LazyCallGraph::SCC,llvm::AnalysisManager<llvm::LazyCallGraph::SCC,llvm::LazyCallGraph & __ptr64>,llvm::LazyCallGraph & __ptr64,llvm::CGSCCUpdateResult & __ptr64>::run+0x30b
rustc_driver-9acede3d415bc9ac.dll!llvm::detail::PassModel<llvm::LazyCallGraph::SCC,llvm::PassManager<llvm::LazyCallGraph::SCC,llvm::AnalysisManager<llvm::LazyCallGraph::SCC,llvm::LazyCallGraph & __ptr64>,llvm::LazyCallGraph & __ptr64,llvm::CGSCCUpdateResu
rustc_driver-9acede3d415bc9ac.dll!llvm::DevirtSCCRepeatedPass::run+0x200
rustc_driver-9acede3d415bc9ac.dll!llvm::detail::PassModel<llvm::LazyCallGraph::SCC,llvm::DevirtSCCRepeatedPass,llvm::PreservedAnalyses,llvm::AnalysisManager<llvm::LazyCallGraph::SCC,llvm::LazyCallGraph & __ptr64>,llvm::LazyCallGraph & __ptr64,llvm::CGSCCU
rustc_driver-9acede3d415bc9ac.dll!llvm::ModuleToPostOrderCGSCCPassAdaptor::run+0x114b
rustc_driver-9acede3d415bc9ac.dll!llvm::detail::PassModel<llvm::Module,llvm::ModuleToPostOrderCGSCCPassAdaptor,llvm::PreservedAnalyses,llvm::AnalysisManager<llvm::Module> >::run+0x11
rustc_driver-9acede3d415bc9ac.dll!llvm::PassManager<llvm::Module,llvm::AnalysisManager<llvm::Module> >::run+0x2a2
rustc_driver-9acede3d415bc9ac.dll!llvm::ModuleInlinerWrapperPass::run+0x24c
rustc_driver-9acede3d415bc9ac.dll!llvm::detail::PassModel<llvm::Module,llvm::ModuleInlinerWrapperPass,llvm::PreservedAnalyses,llvm::AnalysisManager<llvm::Module> >::run+0x11
rustc_driver-9acede3d415bc9ac.dll!llvm::PassManager<llvm::Module,llvm::AnalysisManager<llvm::Module> >::run+0x2a2
rustc_driver-9acede3d415bc9ac.dll!LLVMRustOptimizeWithNewPassManager+0x1f0e
rustc_driver-9acede3d415bc9ac.dll!RNvNtNtCs8Ww9ijeY60a_18rustc_codegen_llvm4back5write35optimize_with_new_llvm_pass_manager+0x400
rustc_driver-9acede3d415bc9ac.dll!RNvNtNtCs8Ww9ijeY60a_18rustc_codegen_llvm4back3lto16run_pass_manager+0x242
rustc_driver-9acede3d415bc9ac.dll!RNvNtNtCs8Ww9ijeY60a_18rustc_codegen_llvm4back3lto20optimize_thin_module+0x2051
rustc_driver-9acede3d415bc9ac.dll!RNvMs_NtNtCs5mXDuW1fxtA_17rustc_codegen_ssa4back3ltoINtB4_16LtoModuleCodegenNtCs8Ww9ijeY60a_18rustc_codegen_llvm18LlvmCodegenBackendE8optimizeB1e_+0x2a
rustc_driver-9acede3d415bc9ac.dll!RINvNtNtCs5mXDuW1fxtA_17rustc_codegen_ssa4back5write17execute_work_itemNtCs8Ww9ijeY60a_18rustc_codegen_llvm18LlvmCodegenBackendEB19_+0x2fc
rustc_driver-9acede3d415bc9ac.dll!RINvNtNtCsbfuXEFdO0KO_3std10sys_common9backtrace28___rust_begin_short_backtraceNCINvXs0_Cs8Ww9ijeY60a_18rustc_codegen_llvmNtB1o_18LlvmCodegenBackendNtNtNtCs5mXDuW1fxtA_17rustc_codegen_ssa6traits7backend19ExtraBackendMetho
rustc_driver-9acede3d415bc9ac.dll!RINvXss_Cs6sEOyLJiBiG_8smallvecINtB6_8SmallVecARNtNtCsg0nmUIWefZS_12rustc_middle2ty3TySj8_EINtNtNtNtCscKNrPtuGzPt_4core4iter6traits7collect6ExtendBJ_E6extendINtNtNtB1y_8adapters5chain5ChainINtNtNtB1y_7sources4once4OnceBJ_
std-fbf1316f493850ac.dll!std::sys::windows::thread::impl$0::new::thread_start+0x4c
KERNEL32.DLL!BaseThreadInitThunk+0x14
ntdll.dll!RtlUserThreadStart+0x21

I'm pretty much out of my depth when it comes to digging into LLVM, though.

nikic · 2022-01-13T08:35:44Z

@ehuss Could you please rerun the asserting rustc command with -C save-temps and zip the results? Hopefully this is reproducible on the bitcode.

ehuss · 2022-01-14T00:23:08Z

Here are the temp files:
libtracing_subscriber-1e4a1d5d5521f0b9.zip

ehuss · 2022-01-14T01:09:57Z

I confirmed the only non-default setting needed to trigger it is llvm.assertions, and building tracing-subscriber 0.3.3 with --release.

nikic · 2022-01-14T11:48:23Z

Best reduction I have for now is https://gist.github.com/nikic/2ff417bd7df48d660f5af0ea47ee3c89 with -passes="cgscc(inline,early-cse<memssa>)".

Edit: https://gist.github.com/nikic/65871cd4c29ded2d1b21cebbdab3f052 asserts under opt -inline.

nikic · 2022-01-14T14:30:02Z

I filed llvm/llvm-project#53206.

nikic · 2022-01-31T10:26:24Z

Fixed upstream by llvm/llvm-project@4810051.

Amanieu · 2022-02-05T21:01:44Z

Thanks @nikic!

Let's try this again.

@bors r=m-ou-se

bors · 2022-02-05T21:01:45Z

📌 Commit 2082842 has been approved by m-ou-se

bors · 2022-02-06T03:12:48Z

⌛ Testing commit 2082842 with merge 719b04c...

bors · 2022-02-06T05:54:00Z

☀️ Test successful - checks-actions
Approved by: m-ou-se
Pushing 719b04c to master...

rust-timer · 2022-02-06T07:25:43Z

Finished benchmarking commit (719b04c): comparison url.

Summary: This benchmark run shows 34 relevant regressions 😿 to instruction counts.

Average relevant regression: 0.8%
Average relevant improvement: -0.3%
Largest regression in instruction counts: 1.9% on incr-patched: println builds of regression-31157 opt

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

rust-highfive assigned m-ou-se Jan 3, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 3, 2022

Amanieu mentioned this pull request Jan 3, 2022

Implement -Z oom=panic #88098

Merged

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 5, 2022

matthiaskrgr mentioned this pull request Jan 5, 2022

Rollup of 7 pull requests #92590

Closed

matthiaskrgr mentioned this pull request Jan 5, 2022

Rollup of 7 pull requests #92593

Closed

matthiaskrgr mentioned this pull request Jan 6, 2022

Rollup of 7 pull requests #92616

Closed

matthiaskrgr mentioned this pull request Jan 9, 2022

Rollup of 7 pull requests #92694

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 12, 2022

hkratz mentioned this pull request Jan 12, 2022

LLVM assertion failures hang with a modal dialog on Windows #92829

Closed

Amanieu added 3 commits February 5, 2022 20:58

Allow handle_alloc_error to unwind

b1b8810

Mark __rgl_oom and __rd_oom as "C-unwind"

915a160

Update llvm-project submodule

2082842

Amanieu force-pushed the oom_hook_unwind branch from f4545cc to 2082842 Compare February 5, 2022 21:00

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 5, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 6, 2022

bors merged commit 719b04c into rust-lang:master Feb 6, 2022

rustbot added this to the 1.60.0 milestone Feb 6, 2022

rustbot added the perf-regression Performance regression. label Feb 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow unwinding from OOM hooks #92535

Allow unwinding from OOM hooks #92535

Amanieu commented Jan 3, 2022

rust-highfive commented Jan 3, 2022

m-ou-se commented Jan 5, 2022 •

edited

Loading

bjorn3 commented Jan 5, 2022

Amanieu commented Jan 5, 2022

Amanieu commented Jan 5, 2022

m-ou-se commented Jan 5, 2022

bors commented Jan 5, 2022

m-ou-se commented Jan 5, 2022

ehuss commented Jan 12, 2022

hkratz commented Jan 12, 2022

rustbot commented Jan 12, 2022

Amanieu commented Jan 12, 2022

bjorn3 commented Jan 12, 2022

ehuss commented Jan 12, 2022

nikic commented Jan 13, 2022

ehuss commented Jan 14, 2022

ehuss commented Jan 14, 2022

nikic commented Jan 14, 2022 •

edited

Loading

nikic commented Jan 14, 2022

nikic commented Jan 31, 2022

Amanieu commented Feb 5, 2022

bors commented Feb 5, 2022

bors commented Feb 6, 2022

bors commented Feb 6, 2022

rust-timer commented Feb 6, 2022

Allow unwinding from OOM hooks #92535

Allow unwinding from OOM hooks #92535

Conversation

Amanieu commented Jan 3, 2022

rust-highfive commented Jan 3, 2022

m-ou-se commented Jan 5, 2022 • edited Loading

bjorn3 commented Jan 5, 2022

Amanieu commented Jan 5, 2022

Amanieu commented Jan 5, 2022

m-ou-se commented Jan 5, 2022

bors commented Jan 5, 2022

m-ou-se commented Jan 5, 2022

ehuss commented Jan 12, 2022

hkratz commented Jan 12, 2022

rustbot commented Jan 12, 2022

Amanieu commented Jan 12, 2022

bjorn3 commented Jan 12, 2022

ehuss commented Jan 12, 2022

nikic commented Jan 13, 2022

ehuss commented Jan 14, 2022

ehuss commented Jan 14, 2022

nikic commented Jan 14, 2022 • edited Loading

nikic commented Jan 14, 2022

nikic commented Jan 31, 2022

Amanieu commented Feb 5, 2022

bors commented Feb 5, 2022

bors commented Feb 6, 2022

bors commented Feb 6, 2022

rust-timer commented Feb 6, 2022

m-ou-se commented Jan 5, 2022 •

edited

Loading

nikic commented Jan 14, 2022 •

edited

Loading