Skip to content

fix: Ensure mod unwind_unimplemented works without nightly feature enabled #150

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 23, 2024
Merged

fix: Ensure mod unwind_unimplemented works without nightly feature enabled #150

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
079441d
fix: Ensure `nightly` feature doesn't force `unwinding`
polarathene Oct 21, 2024
e0db774
chore: `nightly` feature should still enable `unwinding` as a feature
polarathene Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,12 @@ pub(crate) mod naked;
#[cfg(all(feature = "unwinding", not(target_arch = "arm")))]
#[allow(unused_extern_crates)]
extern crate unwinding;
#[cfg(all(feature = "unwinding", target_arch = "arm"))]
#[cfg(any(not(feature = "unwinding"), target_arch = "arm"))]
Copy link
Contributor Author

@polarathene polarathene Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reverted this back to what it was, as I think it's valid to enable for either condition vs only unwinding enabled on arm?

Errors without this change

For x86_64 this change allowed the hello-world-small Eyra example to build successfully (if the nightly feature of origin was not enabled by c-scape):

  • This was with eyra => c-gull => c-scape => origin crates all cloned locally where I removed the nightly feature from c-scape's origin feature list.
  • I tried to reproduce with just origin but no luck. Presumably something eyra/c-scape was doing with origin and it's unwind modules here?

Without the change, these are the two kinds of failures:

With -C target-feature=+crt-static:

  = note: rust-lld: error: undefined symbol: _dl_find_object
          >>> referenced by unwind-dw2-fde-dip.o:(_Unwind_Find_FDE) in archive /usr/lib/gcc/x86_64-redhat-linux/14/libgcc_eh.a
          >>> referenced by unwind-dw2-fde-dip.o:(_Unwind_Find_FDE) in archive /usr/lib/gcc/x86_64-redhat-linux/14/libgcc_eh.a
          collect2: error: ld returned 1 exit status

Without -C target-feature=+crt-static, all the symbols that the module provides are listed as errors:

  = note: rust-lld: error: undefined symbol: _Unwind_Resume

...

RUSTFLAGS='-C panic=abort' vs panic = "abort" (Cargo.toml)

If building with -C panic=abort (regardless of panic setting in Cargo.toml profile), with the nightly feature you'll get:

error[E0463]: can't find crate for `unwinding`
  --> /origin/src/lib.rs:39:1
   |
39 | extern crate unwinding;
   | ^^^^^^^^^^^^^^^^^^^^^^^ can't find crate

That could be prevented by matching the unwinding dep cfg requirement for panic = "unwind", but you'd then also need the opposite for the mod unwind_unimplemented condition.

Not possible to use cfg with features yet: rust-lang/cargo#1197

mod unwind_unimplemented;
// If we don't have "unwinding", provide stub functions for unwinding and
// panicking.
#[cfg(not(feature = "unwinding"))]
mod stubs;
Comment on lines +42 to +45
Copy link
Contributor Author

@polarathene polarathene Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured given the context it was better to co-locate with the other unwinding related logic?

mod stubs was previously toggled by the nightly feature but has since lost that context with the switch to unwinding feature. Thus technically unwind_unimplemented and stubs are similar beyond the arm conditional.

Not sure how you want to approach this.


#[cfg_attr(target_arch = "aarch64", path = "arch/aarch64.rs")]
#[cfg_attr(target_arch = "x86_64", path = "arch/x86_64.rs")]
Expand Down Expand Up @@ -66,11 +70,6 @@ pub mod signal;
#[cfg_attr(not(feature = "take-charge"), path = "thread/libc.rs")]
pub mod thread;

// If we don't have "unwinding", provide stub functions for unwinding and
// panicking.
#[cfg(not(feature = "unwinding"))]
mod stubs;

// Include definitions of `memcpy` and other functions called from LLVM
// Codegen. Normally, these would be defined by the platform libc, however with
// origin with `take-charge`, there is no libc. Otherwise normally, these
Expand Down