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

Include rustc version in crate disambiguator #85142

Closed
bjorn3 opened this issue May 10, 2021 · 4 comments · Fixed by #89836
Closed

Include rustc version in crate disambiguator #85142

bjorn3 opened this issue May 10, 2021 · 4 comments · Fixed by #89836
Assignees
Labels
A-linkage Area: linking into static, shared libraries and binaries C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@bjorn3
Copy link
Member

bjorn3 commented May 10, 2021

Otherwise with -Zsymbol-mangling-version=v0 and no -Cmetadata, symbols from the same crate compiled with different versions of rustc are named the same. This makes it possible to replace a rust dylib compiled with one version of rustc with one compiled by another version of rustc even if the ABI doesn't match.

The crate disambiguator is calculated at

pub(crate) fn compute_crate_disambiguator(session: &Session) -> CrateDisambiguator {
use std::hash::Hasher;
// The crate_disambiguator is a 128 bit hash. The disambiguator is fed
// into various other hashes quite a bit (symbol hashes, incr. comp. hashes,
// debuginfo type IDs, etc), so we don't want it to be too wide. 128 bits
// should still be safe enough to avoid collisions in practice.
let mut hasher = StableHasher::new();
let mut metadata = session.opts.cg.metadata.clone();
// We don't want the crate_disambiguator to dependent on the order
// -C metadata arguments, so sort them:
metadata.sort();
// Every distinct -C metadata value is only incorporated once:
metadata.dedup();
hasher.write(b"metadata");
for s in &metadata {
// Also incorporate the length of a metadata string, so that we generate
// different values for `-Cmetadata=ab -Cmetadata=c` and
// `-Cmetadata=a -Cmetadata=bc`
hasher.write_usize(s.len());
hasher.write(s.as_bytes());
}
// Also incorporate crate type, so that we don't get symbol conflicts when
// linking against a library of the same name, if this is an executable.
let is_exe = session.crate_types().contains(&CrateType::Executable);
hasher.write(if is_exe { b"exe" } else { b"lib" });
CrateDisambiguator::from(hasher.finish::<Fingerprint>())
}
This should include something like option_env!("CFG_VERSION").unwrap_or("unknown version") in the hash. This matches what the version info included in the rustc metadata header.

@bjorn3 bjorn3 added A-linkage Area: linking into static, shared libraries and binaries C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 10, 2021
@pierwill
Copy link
Member

@rustbot claim

@pierwill
Copy link
Member

pierwill commented Oct 12, 2021

Looks like this code has moved. Still looking into what happened.

@pierwill
Copy link
Member

pierwill commented Oct 13, 2021

The crate disambiguator was moved to rustc_span::StableCrateId in #85804.

@pierwill
Copy link
Member

This looks like the relevant code:

pub fn new(crate_name: &str, is_exe: bool, mut metadata: Vec<String>) -> StableCrateId {
use std::hash::Hash;
use std::hash::Hasher;
let mut hasher = StableHasher::new();
crate_name.hash(&mut hasher);
// We don't want the stable crate id to dependent on the order
// -C metadata arguments, so sort them:
metadata.sort();
// Every distinct -C metadata value is only incorporated once:
metadata.dedup();
hasher.write(b"metadata");
for s in &metadata {
// Also incorporate the length of a metadata string, so that we generate
// different values for `-Cmetadata=ab -Cmetadata=c` and
// `-Cmetadata=a -Cmetadata=bc`
hasher.write_usize(s.len());
hasher.write(s.as_bytes());
}
// Also incorporate crate type, so that we don't get symbol conflicts when
// linking against a library of the same name, if this is an executable.
hasher.write(if is_exe { b"exe" } else { b"lib" });
StableCrateId(hasher.finish())
}

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 21, 2021
…orn3

Include rustc version in `rustc_span::StableCrateId`

`rustc_span::def_id::StableCrateId` is a hash of various data about a crate during compilation. This PR includes the version of `rustc` in the input when computing this hash. From a cursory reading of [RFC 2603](https://rust-lang.github.io/rfcs/2603-rust-symbol-name-mangling-v0.html), this appears to be acceptable within that design.

In order to pass the `mir-opt` and `ui` test suites, this adds new [normalization for hashes and symbol names in `compiletest`](https://github.com/rust-lang/rust/pull/89836/files#diff-03a0567fa80ca04ed5a55f9ac5c711b4f84659be2d0ac4a984196d581c04f76b). These are enabled by default, but we might prefer it to be configurable.

In the UI tests, I had to truncate a signification amount of error annotations in v0 symbols (and maybe some legacy) in order to get the normalization to work correctly. (See rust-lang#90116.)

Closes rust-lang#85142.
@bors bors closed this as completed in 9e1aff8 Dec 16, 2021
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 C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants