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

Linking a static C library does not include all symbols #78827

Open
Firstyear opened this issue Nov 7, 2020 · 14 comments
Open

Linking a static C library does not include all symbols #78827

Firstyear opened this issue Nov 7, 2020 · 14 comments
Labels
A-linkage Area: linking into static, shared libraries and binaries P-low Low priority T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Firstyear
Copy link
Contributor

Firstyear commented Nov 7, 2020

This relates to #15460. On 1.47 stable and 1.48 nightly, when using cc/cbindgen, if a symbol in the C file is not used by the .rs file, then it is not in the resulting cdylib. It is possible to force the symbol to be included by "calling" it as show in #19321 .

The linked repository is able to reproduce this. Testing on OpenSUSE tumbleweed (fully patched with gcc10). After cargo build then the resulting libdemo.so does not contain the symbol wrapper_add.

This appears to be a regression since #15460 which was intended to resolve this behaviour. I appreciate your advice on this, thanks,

https://github.com/Firstyear/demo_r_link

EDIT: Worth noting in wrapper.c that it does not need to link to any R libs, and that SEXP is a typdef to typedef struct SEXPREC *SEXP;

@camelid camelid added A-linkage Area: linking into static, shared libraries and binaries regression-untriaged Untriaged performance or correctness regression. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 10, 2020
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Nov 10, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 10, 2020

@Firstyear can you confirm whether this worked in Rust 1.0 (or any later release)? 1ae1461 is present in every stable release, it would be helpful to know when this regressed.

@jyn514 jyn514 added the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Nov 10, 2020
@Firstyear
Copy link
Contributor Author

I can probably make a smaller reproducer and do a rough bisect of some older releases, but I honestly only just found this problem the other day :(

@Firstyear
Copy link
Contributor Author

@jyn514 I've just checked back through to 1.34.0-x86_64-unknown-linux-gnu, where releases prior can't be used without lowering the version of cc. Every version from 1.34 to nightly exhibits this issue with the example repository. I have pushed an update that simplifies the example code so that anyone should be able to reproduce it.

@jyn514
Copy link
Member

jyn514 commented Nov 12, 2020

@Firstyear I don't understand how to run your example. Do I need R installed or something?

@Firstyear
Copy link
Contributor Author

Sorry to make it a bit clearer.

git clone https://github.com/Firstyear/demo_r_link.git
cd demo_r_link
cargo build
readelf -Ws target/debug/libdemo.so | grep wrapper_add

The function wrapper_add as defined https://github.com/Firstyear/demo_r_link/blob/main/src/wrapper.c which is built with https://github.com/Firstyear/demo_r_link/blob/main/build.rs should be present in the resulting .so, but it is not.

@mati865
Copy link
Contributor

mati865 commented Nov 13, 2020

You are not linking your C library anywhere so you cannot expect it be included.

Re-exporting C symbols in cdylibs currently doesn't work: rust-lang/rfcs#2771. If that's what you are trying to achieve this issue can be closed as a duplicate.

@Firstyear
Copy link
Contributor Author

The cc crate emits the metadata to link the C lib into the output.

This is not a duplicate as it is the C symbols that are not available after compilation.

@mati865
Copy link
Contributor

mati865 commented Nov 14, 2020

Rust won't link that C library if it doesn't find anything useful there and your code doesn't pull any symbol.
You can see this example how to import symbol from C library: https://doc.rust-lang.org/cargo/reference/build-script-examples.html#building-a-native-library

Re-exporting them is more complicated though, you'd have to do something like this: #62287

@Firstyear
Copy link
Contributor Author

This is still not sufficient.

I have updated the example such that rust has a call into the C component now. From the symbol outputs:

# readelf -Ws ./target/debug/libdemo.so | grep -i wrapper_add
# readelf -Ws ./target/debug/libdemo.so | grep -i reverse
    75: 0000000000001141    30 FUNC    LOCAL  DEFAULT   12 reverse_call

You can find the updates in https://github.com/Firstyear/demo_r_link/tree/reverse-call . In this lib.rs calls from f64mul to reverse_call in wrapper.c, which does correctly compile in. However, the symble for "wrapper_add" that calls back to f64sum is still not present. This indicates that wrapper.c is compiled and linked, but not all symbols are included.

@mati865
Copy link
Contributor

mati865 commented Nov 18, 2020

You can find the updates in https://github.com/Firstyear/demo_r_link/tree/reverse-call . In this lib.rs calls from f64mul to reverse_call in wrapper.c, which does correctly compile in.

It does import the symbol but it's still not exported:

$ nm target/debug/libdemo.so | grep wrapping
                 U wrapping_add

rust-lang/rfcs#2771

However, the symble for "wrapper_add" that calls back to f64sum is still not present. This indicates that wrapper.c is compiled and linked, but not all symbols are included.

AFAIK this is expected because you do not import wrapper_add anywhere and cdylib only exports symbols whose names are explicitly passed by rustc to the linker.

@Firstyear
Copy link
Contributor Author

What would be the way to fix the provided example code so that it does work in the way that I expect it to then?

@apiraino apiraino added P-low Low priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Nov 19, 2020
@apiraino
Copy link
Contributor

Assigning P-low as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

@mati865
Copy link
Contributor

mati865 commented Nov 19, 2020

What would be the way to fix the provided example code so that it does work in the way that I expect it to then?

For re-exporting somebody has to fix rust-lang/rfcs#2771.
For importing wrapper_add you have to use similar code as you have used for importing reverse_call.

@jyn514 jyn514 added T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-untriaged Untriaged performance or correctness regression. E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc labels Nov 19, 2020
@Firstyear
Copy link
Contributor Author

I think it's that a symbol defined in my code's .c files is not exported which seems like a surprising behaviour. Especially since calling wrapper_add would be recursive, and the point is that wrapped_add should be a landing point for incoming calls from C applications.

The fact that a linked library in my own source tree has it's symbols not exported is a very surprising (and incorrect) behaviour IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linkage Area: linking into static, shared libraries and binaries P-low Low priority T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants