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

Dead links when unstable-static-class enabled #667

Open
Berrysoft opened this issue Oct 29, 2024 · 7 comments
Open

Dead links when unstable-static-class enabled #667

Berrysoft opened this issue Oct 29, 2024 · 7 comments
Labels
A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates bug Something isn't working

Comments

@Berrysoft
Copy link

I've tried both unstable-static-class and unstable-static-class-inlined with LTO enabled. It seems that there are some dead links to the classes. For example, when I enable the feature NSAttributedString in objc2-app-kit, NSPresentationIntent is also linked to the final binary even if I don't use it. Is there some way to optimize these links out?

@madsmtm madsmtm added bug Something isn't working A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates labels Oct 29, 2024
@madsmtm
Copy link
Owner

madsmtm commented Oct 29, 2024

Hmm, we explicitly add no_dead_strip for these because Clang does that as well, and I seem to have intentionally added this in 61213ad.

But looking at it again, I think perhaps I was mistaken back then, and thought that this code meant that lld when parsing expected these to be available (which isn't what that piece of code is doing, it's outputting)?

In any case, I don't see a reason to keep no_dead_strip today, apart from Clang having it.

@madsmtm
Copy link
Owner

madsmtm commented Oct 29, 2024

Hmm, actually, from my testing, it seems like no_dead_strip there actually only applies to the section itself (?), and not the symbol (which can be stripped unless there's a .no_dead_strip directive, corresponding to #[used] in Rust).

@madsmtm
Copy link
Owner

madsmtm commented Oct 29, 2024

Just to confirm: You're testing with --release, and [profile.release] codegen-units = 1? And could you try with lto = "full"?

@Berrysoft
Copy link
Author

Berrysoft commented Oct 29, 2024

I ran IDA to find the unwanted symbol. You can also try a min macos version lower than 12.0 to see a link error when NSAttributedString is enabled.

I tried with --release, codegen-units = 1 and lto = true.

UPDATE: rust version: rustc 1.84.0-nightly (da935398d 2024-10-19)

@Berrysoft
Copy link
Author

I cloned this objc2 repo and checked out to objc-0.5.2 tag. This is my change:

diff --git a/crates/objc2/src/macros/mod.rs b/crates/objc2/src/macros/mod.rs
index 815f1025..9b8ce381 100644
--- a/crates/objc2/src/macros/mod.rs
+++ b/crates/objc2/src/macros/mod.rs
@@ -505,7 +505,7 @@ macro_rules! __statics_class {
         }

         /// SAFETY: Same as `REF` above in `__statics_sel!`.
-        #[link_section = "__DATA,__objc_classrefs,regular,no_dead_strip"]
+        #[link_section = "__DATA,__objc_classrefs,regular"]
         #[export_name = $crate::__macro_helpers::concat!(
             "\x01L_OBJC_CLASSLIST_REFERENCES_$_",
             $hash,

And now there are no dead links.

@Berrysoft
Copy link
Author

Berrysoft commented Oct 29, 2024

Now I have tried this one:

diff --git a/crates/objc2/src/macros/mod.rs b/crates/objc2/src/macros/mod.rs
index 815f1025..25fd2542 100644
--- a/crates/objc2/src/macros/mod.rs
+++ b/crates/objc2/src/macros/mod.rs
@@ -443,11 +443,11 @@ macro_rules! __statics_sel {
             // Clang uses `no_dead_strip` in the link section for some reason,
             // which other tools (notably some LLVM tools) now assume is
             // present, so we have to add it as well.
-            link_section = "__DATA,__objc_selrefs,literal_pointers,no_dead_strip",
+            link_section = "__DATA,__objc_selrefs,literal_pointers",
         )]
         #[cfg_attr(
             all(target_os = "macos", target_arch = "x86"),
-            link_section = "__OBJC,__message_refs,literal_pointers,no_dead_strip",
+            link_section = "__OBJC,__message_refs,literal_pointers",
         )]
         #[export_name = $crate::__macro_helpers::concat!("\x01L_OBJC_SELECTOR_REFERENCES_", $hash)]
         static mut REF: $crate::__macro_helpers::UnsafeCell<$crate::runtime::Sel> = unsafe {
@@ -505,7 +505,7 @@ macro_rules! __statics_class {
         }
 
         /// SAFETY: Same as `REF` above in `__statics_sel!`.
-        #[link_section = "__DATA,__objc_classrefs,regular,no_dead_strip"]
+        #[link_section = "__DATA,__objc_classrefs,regular"]
         #[export_name = $crate::__macro_helpers::concat!(
             "\x01L_OBJC_CLASSLIST_REFERENCES_$_",
             $hash,
@@ -537,7 +537,7 @@ macro_rules! __statics_class {
         static NAME_DATA: [$crate::__macro_helpers::u8; X.len()] = $crate::__statics_string_to_known_length_bytes!(X);
 
         /// SAFETY: Same as `REF` above in `__statics_sel!`.
-        #[link_section = "__OBJC,__cls_refs,literal_pointers,no_dead_strip"]
+        #[link_section = "__OBJC,__cls_refs,literal_pointers"]
         #[export_name = $crate::__macro_helpers::concat!(
             "\x01L_OBJC_CLASS_REFERENCES_",
             $hash,

And it at least works on my machine:)

The system info: 13.7 (22H123), x86_64

@madsmtm
Copy link
Owner

madsmtm commented Oct 29, 2024

Hmm, I clearly don't understand enough about this, have opened llvm/llvm-project#114111 to get some clarification from the Clang developers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants