Skip to content
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

Add UWTable attr to functions with a personality function #125844

Closed
wants to merge 1 commit into from

Conversation

Reflexe
Copy link

@Reflexe Reflexe commented Jun 1, 2024

Adding a personality function forces LLVM to generate unwinding info that might be incorrect. To solve it, always apply the UWTable attribute when setting a personality function.

Thanks @Amanieu for doing most of the research work.

Fixes #123733

@rustbot
Copy link
Collaborator

rustbot commented Jun 1, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pnkfelix (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 1, 2024
@rust-log-analyzer

This comment has been minimized.

@Reflexe
Copy link
Author

Reflexe commented Jun 1, 2024

Looks like this solution somehow is causing a crash with SIGILL on:

#0  0x00007fffeecedb68 in llvm::Attribute::getAsString[abi:cxx11](bool) const ()
   from /home/reflexe/rust/rust/build/x86_64-unknown-linux-gnu/stage1/bin/../lib/../lib/libLLVM.so.18.1-rust-1.80.0-nightly
#1  0x00007fffeecdd2c6 in (anonymous namespace)::AssemblyWriter::printFunction(llvm::Function const*) ()
   from /home/reflexe/rust/rust/build/x86_64-unknown-linux-gnu/stage1/bin/../lib/../lib/libLLVM.so.18.1-rust-1.80.0-nightly
#2  0x00007ffff21e3f3e in (anonymous namespace)::AssemblyWriter::printModule(llvm::Module const*) [clone .cold] ()
   from /home/reflexe/rust/rust/build/x86_64-unknown-linux-gnu/stage1/bin/../lib/../lib/libLLVM.so.18.1-rust-1.80.0-nightly
#3  0x00007ffff173a29f in llvm::Module::print(llvm::raw_ostream&, llvm::AssemblyAnnotationWriter*, bool, bool) const ()
   from /home/reflexe/rust/rust/build/x86_64-unknown-linux-gnu/stage1/bin/../lib/../lib/libLLVM.so.18.1-rust-1.80.0-nightly
#4  0x00007ffff3e3982a in LLVMRustPrintModule () from /home/reflexe/rust/rust/build/x86_64-unknown-linux-gnu/stage1/bin/../lib/librustc_driver-2fdfd11538a7490f.so
#5  0x00007ffff3e0d47a in codegen () at compiler/rustc_codegen_llvm/src/back/write.rs:799
#6  0x00007ffff3dfb57e in <rustc_codegen_llvm::LlvmCodegenBackend as rustc_codegen_ssa::traits::write::WriteBackendMethods>::codegen ()
    at compiler/rustc_codegen_llvm/src/lib.rs:238
#7  0x00007ffff3dc0feb in finish_intra_module_work<rustc_codegen_llvm::LlvmCodegenBackend> () at compiler/rustc_codegen_ssa/src/back/write.rs:1001
#8  0x00007ffff3dc1255 in execute_optimize_work_item<rustc_codegen_llvm::LlvmCodegenBackend> () at compiler/rustc_codegen_ssa/src/back/write.rs:893
#9  0x00007ffff3e7eafe in {closure#0}<rustc_codegen_llvm::LlvmCodegenBackend> () at compiler/rustc_codegen_ssa/src/back/write.rs:1806
#10 {closure#0}<rustc_codegen_ssa::back::write::spawn_work::{closure_env#0}<rustc_codegen_llvm::LlvmCodegenBackend>, ()> ()
    at compiler/rustc_codegen_llvm/src/lib.rs:152
#11 __rust_begin_short_backtrace<rustc_codegen_llvm::{impl#2}::spawn_named_thread::{closure_env#0}<rustc_codegen_ssa::back::write::spawn_work::{closure_env#0}<rustc_codegen_llvm::LlvmCodegenBackend>, ()>, ()> () at library/std/src/sys_common/backtrace.rs:155
#12 0x00007ffff3ddcc2b in {closure#0}<rustc_codegen_llvm::{impl#2}::spawn_named_thread::{closure_env#0}<rustc_codegen_ssa::back::write::spawn_work::{closure_env#0}<rustc_codegen_llvm::LlvmCodegenBackend>, ()>, ()> () at library/std/src/thread/mod.rs:542
#13 call_once<(), std::thread::{impl#0}::spawn_unchecked_::{closure#2}::{closure_env#0}<rustc_codegen_llvm::{impl#2}::spawn_named_thread::{closure_env#0}<rustc_codegen_ssa::back::write::spawn_work::{closure_env#0}<rustc_codegen_llvm::LlvmCodegenBackend>, ()>, ()>> () at library/core/src/panic/unwind_safe.rs:272
#14 do_call<core::panic::unwind_safe::AssertUnwindSafe<std::thread::{impl#0}::spawn_unchecked_::{closure#2}::{closure_env#0}<rustc_codegen_llvm::{impl#2}::spawn_named_thread::{closure_env#0}<rustc_codegen_ssa::back::write::spawn_work::{closure_env#0}<rustc_codegen_llvm::LlvmCodegenBackend>, ()>, ()>>, ()> ()
    at library/std/src/panicking.rs:559
#15 try<(), core::panic::unwind_safe::AssertUnwindSafe<std::thread::{impl#0}::spawn_unchecked_::{closure#2}::{closure_env#0}<rustc_codegen_llvm::{impl#2}::spawn_named_thread::{closure_env#0}<rustc_codegen_ssa::back::write::spawn_work::{closure_env#0}<rustc_codegen_llvm::LlvmCodegenBackend>, ()>, ()>>> ()
    at library/std/src/panicking.rs:523
#16 0x00007ffff3e4cea2 in catch_unwind<core::panic::unwind_safe::AssertUnwindSafe<std::thread::{impl#0}::spawn_unchecked_::{closure#2}::{closure_env#0}<rustc_codegen_llvm::{impl#2}::spawn_named_thread::{closure_env#0}<rustc_codegen_ssa::back::write::spawn_work::{closure_env#0}<rustc_codegen_llvm::LlvmCodegenBackend>, ()>, ()>>, ()>
    () at library/std/src/panic.rs:149
#17 {closure#2}<rustc_codegen_llvm::{impl#2}::spawn_named_thread::{closure_env#0}<rustc_codegen_ssa::back::write::spawn_work::{closure_env#0}<rustc_codegen_llvm::LlvmCodegenBackend>, ()>, ()> () at library/std/src/thread/mod.rs:541
#18 call_once<std::thread::{impl#0}::spawn_unchecked_::{closure_env#2}<rustc_codegen_llvm::{impl#2}::spawn_named_thread::{closure_env#0}<rustc_codegen_ssa::back::write::spawn_work::{closure_env#0}<rustc_codegen_llvm::LlvmCodegenBackend>, ()>, ()>, ()> () at library/core/src/ops/function.rs:250
#19 0x00007ffff2f079fd in call_once<(), dyn core::ops::function::FnOnce<(), Output=()>, alloc::alloc::Global> () at library/alloc/src/boxed.rs:2063
#20 call_once<(), alloc::boxed::Box<dyn core::ops::function::FnOnce<(), Output=()>, alloc::alloc::Global>, alloc::alloc::Global> () at library/alloc/src/boxed.rs:2063
#21 0x00007ffff2f5008a in std::sys::pal::unix::thread::Thread::new::thread_start () at library/std/src/sys/pal/unix/thread.rs:108
#22 0x00007ffff2d48ded in start_thread (arg=<optimized out>) at pthread_create.c:447

:\
Maybe we trigger an LLVM bug by adding this attribute. I will research it later.

@Amanieu
Copy link
Member

Amanieu commented Jun 1, 2024

You can try debugging this by enabling LLVM assertions: https://rustc-dev-guide.rust-lang.org/backend/debugging.html#enable-llvm-internal-checks

@Reflexe Reflexe marked this pull request as ready for review June 1, 2024 17:50
@rust-log-analyzer

This comment has been minimized.

Adding a personality function forces LLVM to generate unwinding info that might be incorrect.
To solve it, always apply the UWTable attribute when setting a personality function.

Fixes rust-lang#123733
@pnkfelix
Copy link
Member

can we get some kind of regression test added here? In the absence of a test, I have no idea how to evaluate whether the current state of this PR even represents a net win with respect to issue #123733 (apart from building it myself locally)

@rustbot author

@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Jun 11, 2024
@Dylan-DPC
Copy link
Member

@Reflexe any updates on this? thanks

@Reflexe
Copy link
Author

Reflexe commented Aug 18, 2024

@pnkfelix , @Dylan-DPC , unfortunately I have yet to find the time to research how to properly test this specific case. Do we have anything similar test in rust already? (E.g. a regression test for a specific platform, in which you need to run a sample binary with qemu or another emulator)

@JohnCSimon
Copy link
Member

@Reflexe
Ping from triage:

I'm closing this due to inactivity because this PR hasn't been touched in a few months.
If want to continue on this PR, please reopen before committing to the branch. Thank you.

@rustbot label: +S-inactive

@JohnCSimon JohnCSimon closed this Oct 6, 2024
@rustbot rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Oct 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RUST_BACKTRACE=full loop with -Cpanic=abort on aarch64-unknown-linux-gnu
7 participants