-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Rustc does not warn when .so/.rlib files are not in sync #10786
Comments
@alexcrichton is this still relevant? we don't do this checking, but the metadata isn't pulled from the dylib. |
This has an interesting development:
This issue is supposed to be relevant, but it looks like we're reading metadata of rlibs and dylibs. This is not necessary, and it looks like we could shave ~100ms off compiling hello world if it were taken care of. In short, I'd like to keep this issue open. |
This is an optimization which is quite impactful for compiling small crates. Reading libstd's metadata takes about 50ms, and a hello world before this change took about 100ms (this change halves that time). Recent changes made it such that this optimization wasn't performed, but I think it's a better idea do to this for now. See rust-lang#10786 for tracking this issue.
This is an optimization which is quite impactful for compiling small crates. Reading libstd's metadata takes about 50ms, and a hello world before this change took about 100ms (this change halves that time). Recent changes made it such that this optimization wasn't performed, but I think it's a better idea do to this for now. See rust-lang#10786 for tracking this issue.
Triage: I see that some progress has been made on this issue in the above commits. Does that re-enable this issue? |
just rlibs now, apparently. I'm giving this a close, @alexcrichton , feel free to reopen. |
I think this is still an issue (reopening) In this test case below a blank file is compiled to an rlib/dylib pair, then the rlib is recompiled with a new function. A later executable is then linked against the dylib (using the rlib metadata), succeeding all the way until the linker actually fails.
|
Ahh, I forgot the crate-types. :( |
Triage: the error is slightly different:
|
Triage: This still happens, and seems like a bug with metadata loading and linkage, rather than just a diagnostics issue. The compiler should not load metadata from one file and then link against another while assuming its contents match. |
Would #75594 help here? If we use libcrate_name-svh.rmeta as the metadata filename then we could check if the metadata was up to date without having to read more than the filename in the archive. |
I just got bit by this, except instead of a Rust rlib and dylib, the compiler found a Rust rlib and a .so file of the same name compiled by clang. (The Rust library wraps the C++ one which is why they have the same name.) It then proceeded as if the .so was the dynamic version of the Rust library, leading to a linker error. Definitely confusing. |
…one-from-needless-collect-example, r=Alexendoo Remove unnecessary `clone` from `needless_collect` example The example for [clippy::needless_collect](https://rust-lang.github.io/rust-clippy/master/#needless_collect) is written as follows: ```rust let len = iterator.clone().collect::<Vec<_>>().len(); // should be let len = iterator.count(); ``` With this change, the unnecessary `clone()` is removed and the the standard ### Example ```rust // original ``` Use instead: ```rust // improved ``` structure is followed. Discussion: rust-lang/rust-clippy#10784 (comment) changelog: [`needless_collect`]: Cleaned up the example in the lint documentation.
…nkfelix,petrochenkov Verify that all crate sources are in sync This ensures that rustc will not attempt to link against a cdylib as if it is a rust dylib when an rlib for the same crate is available. Previously rustc didn't actually check if any further formats of a crate which has been loaded are of the same version and if they are actually valid. This caused a cdylib to be interpreted as rust dylib as soon as the corresponding rlib was loaded. As cdylibs don't export any rust symbols, linking would fail if rustc decides to link against the cdylib rather than the rlib. Two crates depended on the previous behavior by separately compiling a test crate as both rlib and dylib. These have been changed to capture their original spirit to the best of my ability while still working when rustc verifies that all crates are in sync. It is unlikely that build systems depend on the current behavior and in any case we are taking a lot of measures to ensure that any change to either the source or the compilation options (including crate type) results in rustc rejecting it as incompatible. We merely didn't do this check here for now obsolete perf reasons. Fixes rust-lang/rust#10786 Fixes rust-lang/rust#82151 Fixes rust-lang/rust#82972 Closes bevy-cheatbook/bevy-cheatbook#114
Right now the logic for reading metadata for a crate is to read it the first time the crate's dylib or rlib file is found. On the second file that's found, the metadata isn't read at all because metadata reading is one of the known slow portions of the compiler.
This implies that if an rlib file is regenerated, but the dylib isn't, then the two are out of sync and rustc will read the old metadata from the dylib and link to the rlib, resulting in linker errors.
I don't want to slow down rustc by reading metadata from all rlibs and dylibs, but this situation is far less than optimal. Perhaps this is a job for a rustpkg-like tool?
The text was updated successfully, but these errors were encountered: