Skip to content

Use a linker script for libstdc++. #1134

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

Merged
merged 1 commit into from
Nov 28, 2022
Merged

Conversation

Alexhuszagh
Copy link
Contributor

Use a linker script for libstdc++.so since we remove the dylib, to ensure a static build is always used to avoid segfaults at runtime. However, the archive can reference missing symbols present in libc or libgcc, and itself depends on symbols in libm. To ensure these libraries are properly linked when forcing a build against the static libstdc++, create a linker script for libstdc++.so that links to the static archive, and as needed, all dependencies.

Related to #1124.

@Alexhuszagh Alexhuszagh requested a review from a team as a code owner November 13, 2022 23:51
@Alexhuszagh

This comment was marked as outdated.

bors bot added a commit that referenced this pull request Nov 13, 2022
@Alexhuszagh
Copy link
Contributor Author

Alexhuszagh commented Nov 13, 2022

So this is a much better solution than what I was doing before: to fix #902, we removed the shared libstdc++.so, however, this has issues since the static version reference symbols resolved in libc, libgcc, and itself depends on libm. To avoid any issues with detecting missing symbols in compiler-builtins, and to avoid using libgcc whenever it is not required, we only want to link to libgcc when needed, and linking another time to libc shouldn't be required. Finally, manually detecting the compiler-builtins path and linking to it uses that library for something it was never intended for: using it as libgcc so we don't have to link to libgcc.

The solution is actually quite simple: use a linker script (which I should have though of, a long time ago):

/* cross-rs linker script
 * this allows us to statically link libstdc++ to avoid segfaults
 * https://github.com/cross-rs/cross/issues/902
 */
GROUP ( libstdc++.a AS_NEEDED( -lgcc -lc -lm ) )

This way, we always link to our static C++ library, and then resolve the missing symbols if required. This works when built against a crate using re2 as a CMake dependency as seen in #1112.

@bors

This comment was marked as outdated.

@Alexhuszagh

This comment was marked as outdated.

bors bot added a commit that referenced this pull request Nov 14, 2022
@bors

This comment was marked as outdated.

@Alexhuszagh
Copy link
Contributor Author

bors try --target musl

bors bot added a commit that referenced this pull request Nov 14, 2022
@bors
Copy link
Contributor

bors bot commented Nov 14, 2022

try

Build succeeded:

Use a linker script for `libstdc++.so` since we remove the dylib, to ensure a static build is always used to avoid segfaults at runtime. However, the archive can reference missing symbols present in `libc` or `libgcc`, and itself depends on symbols in `libm`. To ensure these libraries are properly linked when forcing a build against the static `libstdc++`, create a linker script for `libstdc++.so` that links to the static archive, and as needed, all dependencies.
@Alexhuszagh
Copy link
Contributor Author

Just a ping for a review on this: does this look ready?

Copy link
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

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

oops my bad!

yes this looks great

bors r+

@bors
Copy link
Contributor

bors bot commented Nov 28, 2022

Build succeeded:

@bors bors bot merged commit d4b2141 into cross-rs:main Nov 28, 2022
@Alexhuszagh Alexhuszagh deleted the musl_ld_script branch November 28, 2022 15:46
@Emilgardis Emilgardis added this to the v0.3.0 milestone Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants