Simplify symbol version mangling

[steveklabnik]

Issue by brson
Thursday Oct 31, 2013 at 23:27 GMT

For earlier discussion, see rust-lang/rust#10208

This issue was labelled with: A-linkage, B-RFC, P-low in the Rust repository

---

In the spirit of stripping down and simplifying our broken versioning scheme (#10188, #10207), this is a suggestion to remove a bunch of complexity from our name mangling.

The description of how we mangle symbols is mostly correct. The most important thing this mangling gets us is that it allows multiple, different crates to define the functions by throwing in some hashes, and that's worth preserving. The other versioning benefits are theoretical and don't work.

Where the current mangling includes a hash of the crate metadata and the type signature and also a version tag, instead we can just add a unique hash that either represents the crate metadata as currently defined or the strict version hash as suggested in #10207.

cc #2166

[nikomatsakis]

There is further reason to change the hashing scheme. In particular, using the entire crate in the name of every symbol is completely incompatible with incremental compilation. To make things work, I think you cannot use the data from other symbols at all (or at least only very, very carefully).

That means we can:

1. use the crate-name and version-number
2. use the signature of the symbol itself
3. if we want to get fancy, use the info from things that the symbol depends on, since if those change, the symbol must be rebuilt anyhow

@alexcrichton and I talked about this at one point but I sort of have forgotten our conclusion. We should settle on something and update the incr. comp. RFC to reflect the consensus.

cc @brson @michaelwoerister

[michaelwoerister]

If we just always built the hashes that we plan to use for incremental compilation (that is, hashes that also include the dependencies of a symbol) that would solve the problem without much additional cost in complexity. And it should shield against name clashes when using two versions of the same item.

Using the crate version number will also solve this problem though (if somewhat less reliably because it depends on really adhering to proper version numbering)

[nikomatsakis]

Just had a conversation with @alexcrichton on IRC. The synopsis:

1. We should hash the symbols with the combination of:
   • the package id supplied by cargo (or, if using command line, just the crate name);
   • the types of its local symbols (which are persistently hashable, since they are based on DefIds)
2. It's not worth hashing the types of all dependent symbols, because:
   • publishing two versions of same crate with different types will still cause conflicts locally
   • in a post-cargo world, it's not clear how realistic this scenario is anyway
3. It occurs to me that if we REALLY wanted the full crate hash, we could use a single dummy symbol.

Based on my read of the prior comments and things, this still seems to address the main use-cases for hashing. But there is a lot of prior commentary, so if something is being overlooked, please do bring it up.

[michaelwoerister]

publishing two versions of same crate with different types will still cause conflicts locally

Will it? Let's say we have the following function and struct in two versions of the same crate:

struct SomeStruct(SomeOtherStruct);

fn foo() -> SomeStruct {
    return bar();
}

The only difference in the two versions of the crate is the definition of SomeOtherStruct, e.g.:

// in version A
struct SomeOtherStruct(u64);

// in version B
struct SomeOtherStruct(u32);

This makes the two versions of foo() incompatible on the ABI level, but their hash would be the same because (solely) going through DefIds will hide the different binary layouts of SomeStruct, since the DefId of SomeOtherStruct is the same for both versions (unless a correct crate version number is part of the DefId, which we cannot rely on in general).

[nikomatsakis]

@michaelwoerister

Will it? Let's say we have the following function and struct in two versions of the same crate:

Yes, sorry. I should have written "will likely cause".

[nikomatsakis]

@alexcrichton points out that we could hash not only the path to the struct's name, but also its contents, which would address this issue. Essentially we create a hash for the struct definition, and when you reference that struct, you include that hash (or something like that).

[nikomatsakis]

Because it seems that @alexcrichton didn't know what I was talking about, I just wanted to elaborate on this "dummy symbol" idea I was tossing around. The idea would be to encode the hash of the crate's AST not on EVERY symbol, but just on ONE symbol we make at the end. So for each crate we'd make a symbol like:

fn crate$HASH() { 
    dep1$HASH1();
    dep2$HASH2();
    dep3$HASH3();
}

where depN$HASHN are the dummy symbols from our dependencies.

The entire purpose of this would be to generate "defensive" link errors where a version is updated in place. You'd still need a hash on every symbol with crate+version-number to allow for multiple versions of a library to co-exist.

[michaelwoerister]

@alexcrichton points out that we could hash not only the path to the struct's name, but also its contents, which would address this issue. Essentially we create a hash for the struct definition, and when you reference that struct, you include that hash (or something like that).

I know, but at that point we would effectively also hash all the dependencies of a type, right?

[michaelwoerister]

The idea would be to encode the hash of the crate's AST not on EVERY symbol, but just on ONE symbol we make at the end

That's quite a neat idea :)

[comex]

Is it a viable alternative to just append some random characters (rather than a hash) every time you generate code for a function, and store the name in the output file's AST (and incremental compilation database)?

[brson]

This has always been a complicated topic. The requirements and implementation have shifted a lot over the years, without I think anybody really understanding them, myself included.

TL;DR I don't think the way we are attempting to encode the entire dag/ast structure into symbols is buying us much and we can do a simpler scheme that is compatible w/ incremental compilation.

This is the current description of symbol hashing. From what I can tell the motivations under 'few issues to handle' are still accurate, except that I don't know what

the hash shown in the filename needs to be predictable and stable for build tooling integration. It also needs to be using a hash function which is easy to use from Python, make, etc.

means, and we're definitely violating it. The 'hash' in the filenames is now just an arbitarary string that the build tooling may optionally pass to rustc via the -C extra-filename flag.

I suspect the technical description under 'here is what we do' is very wrong.

Just some quick notes from reviewing what symbol hashing actually does:

The symbol hashing function is symbol_hash. It hashes a type, along with same crate-global data called LinkMeta, as well as the ambiguously-named crate_metadata, which is just an optional vec of strings passed on the command line via -C metadata.

LinkMeta contains the crate name and the Strict Version Hash.

The SVH is a hash of the crate contents. Note that it also hashes the command-line provided metadata. It also hashes the crate attributes.

The SVH is designed to allow the compile-time crate-loader to detect all upstream changes so it can generate errors and demand a recompile. We did this as a blunt way to fix years of weird errors reading metadata, with the understanding that it prevented any sort of binary upgrading. I believe the primary effect of the SVH today is that, in dependency chain A -> B -> C, if A is rebuilt with modifications, then B is not rebuilt, then C is rebuilt, the build of C will fail because the crate A that B references contains a different SVH than when B was originally built and thus B's deps don't resolve.

I'm surprised that I don't see anywhere where the SVH is transitively hashing the SVH of its dependencies. I would expect such transitive hashing to sort of 'lock' the crates together, ensure that two crates that have the same source but resolve two different versions of an upstream lib are different. As it is it looks like all the hashes within a crate only depend on crate-local information. This can't be right...

Cargo uses -C metadata to, well, do whatever it wants:

• I'm not sure where Cargo's 'metadata' originates from - it's specified in the constructors of most kinds of targets.
• @alexcrichton says it boils down to the crate name, version and the 'source' (to distinguish the exact same source built from e.g. git vs crates.io), and the extra data being mixed in is mostly about changing the filename of test-crates to avoid on-disk conflicts.
• This seems really suspicious that we're relying on the build tool to some non-trivial work to prevent symbol collisions. It would seem more robust to the non-Cargo world for rustc to understand the possible configurations that produce conflicts and require Cargo to specify them semantically, not for Cargo to just throw a blob at rustc to fix the problem. It strikes me as making up for an insufficient definition of the SVH.

NB: mangle_exported_name is doing some weird hacks.

There are at least three paths in trans that define symbol names:

• Names for non-generics are defined in base.rs using mangle_exported_name.
• Names for generics are defined in monomorphize.rs using exported_name. NB: monomorphization is doing it's own symbol hashing which is much different from that defined in link.rs
• mangle_internal_name_* functions are used for a variety of shims.

It's quite an ad-hoc clusterfuck.

From my understanding the current requirements here are actually pretty simple:

• Avoid symbol name collisions globally. Ideally the information that goes into these hashes derives only from the structure of the crate DAG and its ASTS, but we see with the crate_metadata that some of the uniquifying information is provided from without.
• Detect when dependencies of dependencies have changed and thus the metadata is no longer accurate. This is what the SVH does, and actually has little to do with symbol naming, though the SVH is hashed into symbols.

The old system had a notion of symbol versioning as well, for binary upgrades, but we've never used it and no longer mangle a version into the symbol name. Such a system is moot as long as we're using something SVH-like to force non-binary-compatibility when crates change.

I'm very inclined to redesign the whole thing from scratch.

Some quick ideas:

• Accept that having unique symbol names requires having unique crate names and versions. That is, two different (or the same) crates named "foo=1.0.0" may have symbol collisions. We don't need to worry about transitively hashing dependencies of things; we're not trying to hash the structure of the entire upstream dag. In a Cargo-based world, Cargo can ensure that crate-identifying information is unique enough.
• Continue creating an SVH for a crate, but now as the final step of building a crate, encoding its complete structure. Downstream deps will continue put their upstream SVH's in their metadata, solely for the crate loader to detect inconsistencies. Though I didn't understand it fully, this sounds like what niko was proposing. The change here is that SVH's are no longer encoded into symbol hashes, but just passed around in the metadat for the crate loader.
• The symbol hashes only encode the crate version (including whatever cargo-derived metadata is deemed necessary), the monomorphized function type, and whatever other additional crate-local info is needed to make it unique (as seen in the mangle_exported_name hack).

cc @graydon You probably remember better than I the original motivations. I do think we've basically given up on some of them.

[brson]

One thing that today's Rust no longer does is guarantee anything about whether the crate loaded at runtime is the same as the crate seen at compiletime. We used to have stronger guarantees when the filename was derived from the crate version/contents. I don't recall exactly why we changed this behavior, but it was probably because having the filenames change constantly was annoying, or maybe because cargo couldn't predict them (this could have something to do with the cryptic 'tooling' comments in the old description of version hashing).