- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 49
Fix illumos .init_array section creation #84
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
Conversation
Under illumos the #[used] tag emits an `llvm.used` flag, this gets converted into a SHF_SUNW_NODISCARD Solaris flag that illumos doesn't understand. The linker then doesn't merge the two .init_array sections since they have different flags. Because we end up with two .init_array sections and the one pointed by default by DT_INIT_ARRAY is the first ""incorrect"" one, inventory doesn't work under illumos when using the illumos linker (the default). This patch changes the default behaviour under illumos to not using #[used]. Some references: + https://reviews.llvm.org/D107955#:~:text=One,them + https://reviews.llvm.org/D97448 + My own writeup of investigating this issue: https://system-illumination.org/01-rustler.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jclulow, since you worked on illumos support for ctor in mmastrac/rust-ctor#135, would you be able to say whether this looks like the best solution to you?
| I feel like the best solution is probably to fix LLVM not to emit random flags on platforms that don't understand them, rather than just try to work around it? | 
| @jclulow Yeah, that's probably a better idea. I'll keep the fork so I can unblock my own needs. And will look into an LLVM fix in the near future. | 
| The rustup-distributed Rust compiler builds use a Rust-specific fork of LLVM (https://github.com/rust-lang/llvm-project). So as soon as you find a fix, it can be backported there and make it into the next nightly with a turnaround time of 1-2 days, instead of waiting months for the fix to end up in an LLVM release and major upgrade of Rust's LLVM dependency. Since this workaround in inventory would be nightly-only anyway, I think I will not be merging it unless it turns out there is no reasonable LLVM fix somehow, or the backport is rejected. This workaround would only make a difference to someone who is building inventory for illumos while unable to upgrade from an old nightly. | 
| Makes a lot of sense @dtolnay, will do that. Thanks for the tip, I didn't know. Feel free to close the PR! I can reopen it if it comes to that. | 
| Coming here from your blog post. For reference I'm the one who changed  Does Illumos have an equivalent to SHF_SUNW_NODISCARD/SHF_GNU_RETAIN? | 
| Let's move this to a Rust issue: rust-lang/rust#146169 | 
Hey there! I've been debugging this issue for the some days and did a write up with more details that I link
to at the end of the PR.
I've tried to keep the disturbance to a minimum but sadly it seems that we need an unstable feature, and thus nightly
for illumos. I don't have a lot of rust experience so I may have done something wrong but I think all other platforms
should still work under stable.
Under illumos the #[used] tag emits an
llvm.usedflag, this gets converted into a SHF_SUNW_NODISCARD Solaris flag that illumos doesn't understand. The linker then doesn't merge the two .init_array sections since they have different flags.Because we end up with two .init_array sections and the one pointed by default by DT_INIT_ARRAY is the first ""incorrect"" one, inventory doesn't work under illumos when using the illumos linker (the default).
This patch changes the default behaviour under illumos to not using #[used].
Some references: