rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.04k stars 12.68k forks source link

rustc can produce non-deterministic crates that can't be mixed together #89904

Closed Fuuzetsu closed 3 years ago

Fuuzetsu commented 3 years ago

We're using nix to build rust crates and benefit from binary caches which are usually populated by our CI. I'll assume a slightly familiarity but I can elaborate when necessary. The details are not too important but it means that the following scenario can happen:

Consider a crate C that depends on crate B that depends on crate A (that depends on std). Now, the following sequence of events happens.

Does the build succeed? Most of the time, yes. But sometimes, it fails if rustc produced two incompatible results for A: that is, if A-local and A-cache differ, we're in trouble. The developer can't use A-local with B-cache: only A-cache is usable.

An error message might look like:

error[E0460]: found possibly newer version of crate `parse_display` which `trader` depends on
  --> src/c/foo.rs:21:5
   |
21 | use b::something;
   |     ^^^^^^
   |
   = note: perhaps that crate needs to be recompiled?
   = note: the following crate versions were found:
           crate `a`: /nix/store/a-local-input-hash/lib/liba-local-38cc57346a.rlib
           crate `b`: /nix/store/b-local-input-hash/lib/libb-local-f69511fec1.rlib

error: aborting due to previous error

In my real case, A is parse_display create, B is our local crate called trader and C was another crate in the workspace that depended on trader. As B (trader) came from binary cache, it is binary-identical and we only have to focus on A.

Investigation

First, let's see what rustc says:

 INFO rustc_metadata::creader resolving dep crate parse_display hash: `5963747a8c8f099e` extra filename: `-38cc57346a`
 INFO rustc_metadata::creader resolving crate `parse_display`
 INFO rustc_metadata::creader falling back to a load
 INFO rustc_metadata::locator lib candidate: target/deps/libparse_display-38cc57346a.rlib
 INFO rustc_metadata::locator rlib reading metadata from: /nix/store/pfy5c0x8jbx44xfnsrqb5yp5sz5ahpxx-rust_parse-display-0.5.2-lib/lib/libparse_display-38cc57346a.rlib
 INFO rustc_metadata::locator Rejecting via hash: expected 5963747a8c8f099e got 5f612fa7f4240092

Looks like the hash it expects is not what it gets. Back to this later. Let's try to look for something obvious in the .rlib.

$ sha256sum libparse_display-38cc57346a.rlib /nix/store/pfy5c0x8jbx44xfnsrqb5yp5sz5ahpxx-rust_parse-display-0.5.2-lib/lib/libparse_display-38cc57346a.rlib
c26bf883248419284608aa1595043e67a9a8a137a62e345172cd87fc112e32a2  libparse_display-38cc57346a.rlib
05af37f3ed3bd95c001f0754e5102589e2d03640de4c65b33093dfcac63b7f2f  /nix/store/pfy5c0x8jbx44xfnsrqb5yp5sz5ahpxx-rust_parse-display-0.5.2-lib/lib/libparse_display-38cc57346a.rlib
$ diff <(nm ./libparse_display-38cc57346a.rlib) <(nm /nix/store/pfy5c0x8jbx44xfnsrqb5yp5sz5ahpxx-rust_parse-display-0.5.2-lib/lib/libparse_display-38cc57346a.rlib)
nm: lib.rmeta: no symbols
nm: lib.rmeta: no symbols

[shana@aya:/tmp/foo]$ diff <(objdump -x ./libparse_display-38cc57346a.rlib) <(objdump -x /nix/store/pfy5c0x8jbx44xfnsrqb5yp5sz5ahpxx-rust_parse-display-0.5.2-lib/lib/libparse_display-38cc57346a.rlib)
1c1
< In archive ./libparse_display-38cc57346a.rlib:
---
> In archive /nix/store/pfy5c0x8jbx44xfnsrqb5yp5sz5ahpxx-rust_parse-display-0.5.2-lib/lib/libparse_display-38cc57346a.rlib:
155c155
<   0 .rmeta        000096d9  0000000000000000  0000000000000000  00000040  2**0
---
>   0 .rmeta        000096da  0000000000000000  0000000000000000  00000040  2**0

[shana@aya:/tmp/foo]$ diff <(strings ./libparse_display-38cc57346a.rlib) <(strings /nix/store/pfy5c0x8jbx44xfnsrqb5yp5sz5ahpxx-rust_parse-display-0.5.2-lib/lib/libparse_display-38cc57346a.rlib)

If I compare binaries side-by-side, it looks like the two files are 1 byte misaligned in first section and have minor differences later on. I will include both files for your own inspection too.

 228   ā”‚ 000038c0: 635f 756e 7769 6e64 94d9 ef8b cdb7 abd5 5600 0111 2d30 3666 3031 6163 3235 3738 6264 6139 3409 6f6e 6365 5f63 656c 6cd2 8bbc ec86 a5dc ab8b 0100 020b 2d35 6636  c_unwind........V...-06f01ac2578bda94.once_cell.............-5f6             000038c0: 635f 756e 7769 6e64 94d9 ef8b cdb7 abd5 5600 0111 2d30 3666 3031 6163 3235 3738 6264 6139 3409 6f6e 6365 5f63 656c 6cd2 8bbc ec86 a5dc ab8b 0100 020b 2d35 6636  c_unwind........V...-06f01ac2578bda94.once_cell.............-5f6
 229   ā”‚ 00003900: 3138 6161 3539 6605 7265 6765 78a2 84b8 b4ed bbff 9332 0002 0b2d 6230 3132 3039 6331 6531 0c72 6567 6578 5f73 796e 7461 788b a4cc c6c0 ddf3 a7de 0100 020b 2d35  18aa59f.regex........2...-b01209c1e1.regex_syntax.............-5             00003900: 3138 6161 3539 6605 7265 6765 78a2 84b8 b4ed bbff 9332 0002 0b2d 6230 3132 3039 6331 6531 0c72 6567 6578 5f73 796e 7461 788b a4cc c6c0 ddf3 a7de 0100 020b 2d35  18aa59f.regex........2...-b01209c1e1.regex_syntax.............-5
 230   ā”‚ 00003940: 3238 3233 6265 3866 380c 6168 6f5f 636f 7261 7369 636b 9aca c888 a2e8 82ca 2c00 020b 2d35 3438 6335 3563 3461 3606 6d65 6d63 6872 c7f1 a28d f4a2 84c0 0a00 020b  2823be8f8.aho_corasick........,...-548c55c4a6.memchr............             00003940: 3238 3233 6265 3866 380c 6168 6f5f 636f 7261 7369 636b 9aca c888 a2e8 82ca 2c00 020b 2d35 3438 6335 3563 3461 3606 6d65 6d63 6872 c7f1 a28d f4a2 84c0 0a00 020b  2823be8f8.aho_corasick........,...-548c55c4a6.memchr............
 231   ā”‚ 00003980: 2d31 3331 6563 3830 6434 3114 7061 7273 655f 6469 7370 6c61 795f 6465 7269 7665 ce9a f3ad b9c0 86a2 8b01 0000 0b2d 6335 6132 6166 3538 3438 0000 001d 2f90 b8af  -131ec80d41.parse_display_derive.............-c5a2af5848..../...           | 00003980: 2d31 3331 6563 3830 6434 3114 7061 7273 655f 6469 7370 6c61 795f 6465 7269 7665 9fda dfa4 e3d3 f580 1700 000b 2d63 3561 3261 6635 3834 3800 0000 1d2f 90b8 af64  -131ec80d41.parse_display_derive............-c5a2af5848..../...d
 232   ā”‚ 000039c0: 6448 ea7a e3a5 752d 3bbf 2d01 0001 001d 2f90 b8af 6448 eaa4 cc8c bfdb f1b9 2301 0003 0373 7464 001d 2f90 b8af 6448 eaee aad1 d9c3 701f 6101 0001 011d 2f90 b8af  dH.z..u-;.-...../...dH........#....std../...dH......p.a...../...           | 000039c0: 48ea 7ae3 a575 2d3b bf2d 0100 0100 1d2f 90b8 af64 48ea a4cc 8cbf dbf1 b923 0100 0303 7374 6400 1d2f 90b8 af64 48ea eeaa d1d9 c370 1f61 0100 0101 1d2f 90b8 af64  H.z..u-;.-...../...dH........#....std../...dH......p.a...../...d
 233   ā”‚ 00003a00: 6448 ea6f ff54 d9ae a053 9f01 0001 021d 2f90 b8af 6448 eaa0 3fcf ee88 19b8 c901 0001 031d 2f90 b8af 6448 eae6 0668 5fbb d337 7901 0001 041d 2f90 b8af 6448 ea21  dH.o.T...S....../...dH..?.........../...dH...h_..7y...../...dH.!           | 00003a00: 48ea 6fff 54d9 aea0 539f 0100 0102 1d2f 90b8 af64 48ea a03f cfee 8819 b8c9 0100 0103 1d2f 90b8 af64 48ea e606 685f bbd3 3779 0100 0104 1d2f 90b8 af64 48ea 2185  H.o.T...S....../...dH..?.........../...dH...h_..7y...../...dH.!.

Of course, it's very difficult to tell anything by just looking at raw binary. I tried to add extra information to rustc itself so that I could see during the hashing what values it's getting: if hash inputs were different, we'd see which ones and be able to go backwards from there. Looking around, I think crate_hash function is the point to change

diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs
index 392372fad53..dafd15b5a46 100644
--- a/compiler/rustc_middle/src/hir/map/mod.rs
+++ b/compiler/rustc_middle/src/hir/map/mod.rs
@@ -968,6 +968,7 @@ pub(super) fn index_hir<'tcx>(tcx: TyCtxt<'tcx>, (): ()) -> &'tcx IndexedHir<'tc
 pub(super) fn crate_hash(tcx: TyCtxt<'_>, crate_num: CrateNum) -> Svh {
     assert_eq!(crate_num, LOCAL_CRATE);

+    tracing::info!("Starting to hash {:?}", crate_num);
     // We can access untracked state since we are an eval_always query.
     let mut hcx = tcx.create_stable_hashing_context();

@@ -984,7 +985,9 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, crate_num: CrateNum) -> Svh {
             Some((def_path_hash, hasher.finish()))
         })
         .collect();
+    tracing::info!("pre-sort {:?}: {:?}", crate_num, hir_body_nodes);
     hir_body_nodes.sort_unstable_by_key(|bn| bn.0);
+    tracing::info!("post-sort {:?}: {:?}", crate_num, hir_body_nodes);

     let node_hashes = hir_body_nodes.iter().fold(
         Fingerprint::ZERO,
@@ -992,8 +995,11 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, crate_num: CrateNum) -> Svh {
             combined_fingerprint.combine(def_path_hash.0.combine(fingerprint))
         },
     );
+    tracing::info!("node_hashes {:?}: {:?}", crate_num, node_hashes);
+

     let upstream_crates = upstream_crates(tcx);
+    tracing::info!("upstream_crates {:?}: {:?}", crate_num, upstream_crates);

     // We hash the final, remapped names of all local source files so we
     // don't have to include the path prefix remapping commandline args.
@@ -1009,17 +1015,26 @@ pub(super) fn crate_hash(tcx: TyCtxt<'_>, crate_num: CrateNum) -> Svh {
         .map(|source_file| source_file.name_hash)
         .collect();

+    tracing::info!("source_file_names before {:?}: {:?}", crate_num, source_file_names);
     source_file_names.sort_unstable();
+    tracing::info!("source_file_names after {:?}: {:?}", crate_num, source_file_names);

     let mut stable_hasher = StableHasher::new();
+    tracing::info!("node_hashes {:?}: {:?}", crate_num, node_hashes);
     node_hashes.hash_stable(&mut hcx, &mut stable_hasher);
+    tracing::info!("upstream_crates {:?}: {:?}", crate_num, upstream_crates);
     upstream_crates.hash_stable(&mut hcx, &mut stable_hasher);
+    tracing::info!("source_file_names {:?}: {:?}", crate_num, source_file_names);
     source_file_names.hash_stable(&mut hcx, &mut stable_hasher);
+    tracing::info!("opts {:?}: {:?}", crate_num, tcx.sess.opts.dep_tracking_hash(true));
     tcx.sess.opts.dep_tracking_hash(true).hash_stable(&mut hcx, &mut stable_hasher);
+    tracing::info!("local_stable_crate_id {:?}: {:?}", crate_num, tcx.sess.local_stable_crate_id());
     tcx.sess.local_stable_crate_id().hash_stable(&mut hcx, &mut stable_hasher);
+    tracing::info!("non_exported_macro_attrs {:?}: {:?}", crate_num, tcx.untracked_crate.non_exported_macro_attrs);
     tcx.untracked_crate.non_exported_macro_attrs.hash_stable(&mut hcx, &mut stable_hasher);

     let crate_hash: Fingerprint = stable_hasher.finish();
+    tracing::info!("crate_hash {:?}: {:?}", crate_num, crate_hash);
     Svh::new(crate_hash.to_smaller_hash())
 }
# Includes one of the default files in src/bootstrap/defaults
profile = "compiler"
changelog-seen = 2
[rust]
channel = "stable"

Sadly, this doesn't work for ironically similar reason: building this compiler locally gives a new std hash which means I can't re-invoke original rustc command with the original .rlibs as those depend on std which has the wrong hash...

At this point I decided to open this ticket: how can I debug this further? At this point I am more or less getting ready to try to gut rustc to the point that I can call crate_hash myself or the rlibs directly but it seems like a lot of work and I'm not confident it's even going to work. I didn't find any tool to read rlib files either.

If we can find what the difference between the two files it, presumably we can work backwards and find why rustc produced two different results.

Here are the files: different_rlibs.tar.gz. local was one I built locally and cache is from the binary cache, built by CI machine.

Meta

rustc --version --verbose:

rustc 1.55.0 (c8dfcfe04 2021-09-06)

I originally suspected that codegen-units may be the culprit: some concurrent resource getting non-deterministically modified, such as to hand out some Ids or something that made it to the code. But no, the issue occurred even with codegen-units=1.

It is difficult to verify or replicate the bug because it happens fairly rarely so it seems like looking at the rlibs I attached above is probably the fastest way to figure something out.

Fuuzetsu commented 3 years ago

Aha! Of course after posting an issue, I get further. I found that I can shove some code into create_resolver and gut other places a bit too to load rlibs during compilation:

pub fn create_resolver(
    sess: Lrc<Session>,
    metadata_loader: Box<MetadataLoaderDyn>,
    krate: &ast::Crate,
    crate_name: &str,
) -> BoxedResolver {
    tracing::trace!("create_resolver for {}: {:?}", crate_name, krate);
    if crate_name == "cloudsim" {
        let dbg = |path: &'static str| {
            let data = rustc_metadata::locator::get_metadata_section(&sess.target, rustc_metadata::locator::CrateFlavor::Rlib, &std::path::Path::new(path), &*metadata_loader).unwrap();
            let mut buf: Vec<u8> = format!("XXX META FOR {}:\n", path).as_bytes().to_vec();
            data.list_crate_metadata(&mut buf).unwrap();
            let buf = String::from_utf8(buf).unwrap();
            tracing::trace!("{}", buf);
        };
        dbg("/nix/store/pfy5c0x8jbx44xfnsrqb5yp5sz5ahpxx-rust_parse-display-0.5.2-lib/lib/libparse_display-38cc57346a.rlib");
        dbg("/tmp/foo/libparse_display-38cc57346a.rlib");
    }

This gives us:

 TRACE rustc_interface::passes XXX META FOR /nix/store/pfy5c0x8jbx44xfnsrqb5yp5sz5ahpxx-rust_parse-display-0.5.2-lib/lib/libparse_display-38cc57346a.rlib:
Crate info:
name parse_display-38cc57346a
hash 5f612fa7f4240092 stable_crate_id StableCrateId(16881853909076815645)
proc_macro false
=External Dependencies=
1 std-008055cc7d873802 hash a2dafebbcfe30fb2 host_hash None kind Explicit
2 core-4beb03d03503c439 hash f808ea40ffa97e9f host_hash None kind Explicit
3 compiler_builtins-dd7db1bec6909f24 hash d1ef612ea1c41b0b host_hash None kind Explicit
4 rustc_std_workspace_core-557ba8776e04d182 hash 5b57ea45fda7dad2 host_hash None kind Explicit
5 alloc-ac23a75f6f42004e hash a7f45db4bd465ca7 host_hash None kind Explicit
6 libc-8480e85e0be96197 hash d2acc6a19075b83f host_hash None kind Explicit
7 unwind-74be3a703f788ba2 hash f299ac5ad662548e host_hash None kind Explicit
8 cfg_if-8f2c5b445c28b2e3 hash 32ab094aa7b255b0 host_hash None kind Explicit
9 hashbrown-6b148909d375a785 hash cae2243f7ad96800 host_hash None kind Explicit
10 rustc_std_workspace_alloc-cd15fa647f4775d1 hash 2f5ca7c7bd62d0ed host_hash None kind Explicit
11 rustc_demangle-e530649c9a06e3c6 hash 78f9818ae04e0955 host_hash None kind Explicit
12 std_detect-0c35b278736219a2 hash 5b8a3aec28451004 host_hash None kind Explicit
13 addr2line-d489f0ca872880cc hash db8132e99d16ec5e host_hash None kind Explicit
14 gimli-75f07df0b18fea39 hash a7acca3f4fd97b7b host_hash None kind Explicit
15 object-95c14e1c1e3ebcc4 hash 22a1c9e36fdb00d2 host_hash None kind Explicit
16 miniz_oxide-f9a3c3274a1835e0 hash e41cb10aab3e61cc host_hash None kind Explicit
17 adler-d4cbb754ee9f4daa hash 0f4ca401300c8a8e host_hash None kind Explicit
18 panic_unwind-06f01ac2578bda94 hash 56aaadbcd17bec94 host_hash None kind Implicit
19 once_cell-5f618aa59f hash 8b5771286d8f05d2 host_hash None kind Explicit
20 regex-b01209c1e1 hash 3227fdded68e0222 host_hash None kind Explicit
21 regex_syntax-52823be8f8 hash de4fceec08d3120b host_hash None kind Explicit
22 aho_corasick-548c55c4a6 hash 2c940b422112251a host_hash None kind Explicit
23 memchr-131ec80d41 hash 0a80111741a8b8c7 host_hash None kind Explicit
24 parse_display_derive-c5a2af5848 hash 8b441a0395bccd4e host_hash None kind MacrosOnly

TRACE rustc_interface::passes XXX META FOR /tmp/foo/libparse_display-38cc57346a.rlib:
Crate info:
name parse_display-38cc57346a
hash 5963747a8c8f099e stable_crate_id StableCrateId(16881853909076815645)
proc_macro false
=External Dependencies=
1 std-008055cc7d873802 hash a2dafebbcfe30fb2 host_hash None kind Explicit
2 core-4beb03d03503c439 hash f808ea40ffa97e9f host_hash None kind Explicit
3 compiler_builtins-dd7db1bec6909f24 hash d1ef612ea1c41b0b host_hash None kind Explicit
4 rustc_std_workspace_core-557ba8776e04d182 hash 5b57ea45fda7dad2 host_hash None kind Explicit
5 alloc-ac23a75f6f42004e hash a7f45db4bd465ca7 host_hash None kind Explicit
6 libc-8480e85e0be96197 hash d2acc6a19075b83f host_hash None kind Explicit
7 unwind-74be3a703f788ba2 hash f299ac5ad662548e host_hash None kind Explicit
8 cfg_if-8f2c5b445c28b2e3 hash 32ab094aa7b255b0 host_hash None kind Explicit
9 hashbrown-6b148909d375a785 hash cae2243f7ad96800 host_hash None kind Explicit
10 rustc_std_workspace_alloc-cd15fa647f4775d1 hash 2f5ca7c7bd62d0ed host_hash None kind Explicit
11 rustc_demangle-e530649c9a06e3c6 hash 78f9818ae04e0955 host_hash None kind Explicit
12 std_detect-0c35b278736219a2 hash 5b8a3aec28451004 host_hash None kind Explicit
13 addr2line-d489f0ca872880cc hash db8132e99d16ec5e host_hash None kind Explicit
14 gimli-75f07df0b18fea39 hash a7acca3f4fd97b7b host_hash None kind Explicit
15 object-95c14e1c1e3ebcc4 hash 22a1c9e36fdb00d2 host_hash None kind Explicit
16 miniz_oxide-f9a3c3274a1835e0 hash e41cb10aab3e61cc host_hash None kind Explicit
17 adler-d4cbb754ee9f4daa hash 0f4ca401300c8a8e host_hash None kind Explicit
18 panic_unwind-06f01ac2578bda94 hash 56aaadbcd17bec94 host_hash None kind Implicit
19 once_cell-5f618aa59f hash 8b5771286d8f05d2 host_hash None kind Explicit
20 regex-b01209c1e1 hash 3227fdded68e0222 host_hash None kind Explicit
21 regex_syntax-52823be8f8 hash de4fceec08d3120b host_hash None kind Explicit
22 aho_corasick-548c55c4a6 hash 2c940b422112251a host_hash None kind Explicit
23 memchr-131ec80d41 hash 0a80111741a8b8c7 host_hash None kind Explicit
24 parse_display_derive-c5a2af5848 hash 1701d69e3497ed1f host_hash None kind MacrosOnly

The line 24 is different: so the problem is the same just one level down. I will now check rlib for that and keep going until I find the one package that has a difference somewhere else. Though, macro package looks promising honestly and it might come up right there.

Fuuzetsu commented 3 years ago

parse_display_derive are just shared libraries so we can use objdump and similar tools as usual! They do seem a bit different.

The metadata seems to be a byte different but I have yet to look closer. I will attach the files in case someone wants to look directly.

dyn_diffs.tar.gz

TRACE rustc_interface::passes XXX META FOR /nix/store/vpi369bxzqgvh3hvvnwrsg51bir435kp-rust_parse-display-derive-0.5.2-lib/lib/libparse_display_derive-c5a2af5848.so:
Crate info:
name parse_display_derive-c5a2af5848
hash 8b441a0395bccd4e stable_crate_id StableCrateId(1156960631929199340)
proc_macro true
=External Dependencies=

DEBUG rustc_metadata::locator checking 8 bytes of metadata-version stamp
DEBUG rustc_metadata::locator inflating 3084 bytes of compressed metadata
TRACE rustc_interface::passes XXX META FOR /tmp/foo/libparse_display_derive-c5a2af5848.so:
Crate info:
name parse_display_derive-c5a2af5848
hash 1701d69e3497ed1f stable_crate_id StableCrateId(1156960631929199340)
proc_macro true
=External Dependencies=
Fuuzetsu commented 3 years ago

I think crate_hash function is the point to change

I think I understand now that this is created at create build time and we only read the information back afterwards. So we have to dump out the metadata itself and inspect closely.

Fuuzetsu commented 3 years ago

I think crate_hash function is the point to change

I think I understand now that this is created at create build time and we only read the information back afterwards. So we have to dump out the metadata itself and inspect closely.

So far I was able to confirm all but the commented out fields (and the hash, obvs) are identical:

        f.debug_struct("CrateRoot")
            .field("name", &root.name)
            .field("triple", &root.triple)
            .field("extra_filename", &root.extra_filename)
            .field("hash", &root.hash)
            .field("stable_crate_id", &root.stable_crate_id)
            .field("panic_strategy", &root.panic_strategy)
            .field("edition", &root.edition)
            .field("has_global_allocator", &root.has_global_allocator)
            .field("has_panic_handler", &root.has_panic_handler)
            .field("crate_deps", &root.crate_deps.decode(meta).collect::<Vec<_>>())

            .field("dylib_dependency_formats", &root.dylib_dependency_formats.decode(meta).collect::<Vec<_>>())
            .field("lib_features", &root.lib_features.decode(meta).collect::<Vec<_>>())
            .field("lang_items", &root.lang_items.decode(meta).collect::<Vec<_>>())
            .field("lang_items_missing", &root.lang_items_missing.decode(meta).collect::<Vec<_>>())
            .field("diagnostic_items", &root.diagnostic_items.decode(meta).collect::<Vec<_>>())
            .field("native_libraries", &root.native_libraries.decode(meta).collect::<Vec<_>>())
            .field("foreign_modules", &root.foreign_modules.decode(meta).collect::<Vec<_>>())
            .field("interpret_alloc_index", &root.interpret_alloc_index.decode(meta).collect::<Vec<_>>())
            .field("exported_symbols", &root.exported_symbols.decode(meta).collect::<Vec<_>>())

            .field("proc_macro_data.proc_macro_decls_static", &root.proc_macro_data.as_ref().map(|x| x.proc_macro_decls_static))
            .field("proc_macro_data.stability", &root.proc_macro_data.as_ref().map(|x| x.stability))
            .field("proc_macro_data.macros", &root.proc_macro_data.as_ref().map(|x| x.macros.decode(meta).collect::<Vec<_>>()))

            // .field("impls", &root.impls.decode(meta).collect::<Vec<_>>())
            // tables: LazyTables<'tcx>,
            // .field("syntax_contexts", &root.syntax_contexts)
            // .field("expn_data", &root.expn_data)
            // .field("expn_hashes", &root.expn_hashes)
            // .field("source_map", &root.source_map.decode(meta).collect::<Vec<_>>())

            .field("has_default_lib_allocator", &root.has_default_lib_allocator)
            .field("compiler_builtins", &root.compiler_builtins)
            .field("needs_allocator", &root.needs_allocator)
            .field("needs_panic_runtime", &root.needs_panic_runtime)
            .field("no_builtins", &root.no_builtins)
            .field("panic_runtime", &root.panic_runtime)
            .field("profiler_runtime", &root.profiler_runtime)
            .field("symbol_mangling_version", &root.symbol_mangling_version)
            .finish()
TRACE rustc_interface::passes XXX META FOR /tmp/foo/libparse_display_derive-c5a2af5848.so:
Crate info:
name parse_display_derive-c5a2af5848
hash 1701d69e3497ed1f stable_crate_id StableCrateId(1156960631929199340)
proc_macro true
=External Dependencies=

TRACE rustc_interface::passes CrateRoot {
    name: "parse_display_derive",
    triple: TargetTriple(
        "x86_64-unknown-linux-gnu",
    ),
    extra_filename: "-c5a2af5848",
    hash: Svh {
        hash: 1657842112824601887,
    },
    stable_crate_id: StableCrateId(
        1156960631929199340,
    ),
    panic_strategy: Unwind,
    edition: Edition2018,
    has_global_allocator: false,
    has_panic_handler: false,
    crate_deps: [],
    dylib_dependency_formats: [],
    lib_features: [],
    lang_items: [],
    lang_items_missing: [],
    diagnostic_items: [],
    native_libraries: [],
    foreign_modules: [],
    interpret_alloc_index: [],
    exported_symbols: [],
    proc_macro_data.proc_macro_decls_static: Some(
        DefIndex(803),
    ),
    proc_macro_data.stability: Some(
        None,
    ),
    proc_macro_data.macros: Some(
        [
            DefIndex(347),
            DefIndex(353),
        ],
    ),
    has_default_lib_allocator: false,
    compiler_builtins: false,
    needs_allocator: false,
    needs_panic_runtime: false,
    no_builtins: false,
    panic_runtime: false,
    profiler_runtime: false,
    symbol_mangling_version: Legacy,
}

I'll continue on Monday or whenever I'm on next.

klensy commented 3 years ago

Rust doesn't have stable abi, so you can't directly cache builded crates, but you can try to use sccache.

workingjubilee commented 3 years ago

More precisely: You cannot assume crate caches are stable between versions, at the moment, thus

Sadly, this doesn't work for ironically similar reason: building this compiler locally gives a new std hash which means I can't re-invoke original rustc command with the original .rlibs as those depend on std which has the wrong hash...

This is on purpose, yes.

You mention it is rare: Does it happen more when trying to reuse the cache between different dev envs, such as different directory layouts? This is a detail that comes up with https://github.com/rust-lang/rust/labels/A-reproducibility issues often.

Fuuzetsu commented 3 years ago

Rust doesn't have stable abi, so you can't directly cache builded crates, but you can try to use sccache.

Given exact same set of sources and all dependencies (system and rust ones), I don't expect the compiler to roll a dice and give different ABI sometimes.

We rely on reproducible builds and also reproducible runtime behaviour for our use-case so sccache isn't really an option.

More precisely: You cannot assume crate caches are stable between versions, at the moment, thus

Right. That's fine. This ticket is only about "I'm giving same inputs to the compiler but get two incompatible outputs sometimes" and presumably this is why the reproducability label even exists.

Does it happen more when trying to reuse the cache between different dev envs, such as different directory layouts? This is a detail that comes up with https://github.com/rust-lang/rust/labels/A-reproducibility issues often.

I think that when nix performs the build, all the sources are in some chroot unpacked in some /build/randomthing directory but I have to verify whether this can actually vary. I think it might. I do also suspect that it could somehow impact the resulting hash: after all in crate_hash it seems to be hashing some filepaths and if those are absolute, it could make a difference.

I will keep going with dumping the metadata that rustc sees about the resulting differing .so files as I have been doing. Sadly, I can't print out source_map as the info is not available retroactively it seems and that may be when the interesting stuff is. But let's just eliminate everything else. If there's nothing obviously mis-matching and only hash differs, I will try to verify the build directory theory somehow.

Fuuzetsu commented 3 years ago

Debug printing more and more contents of the so file didn't get me far and interesting sections would usually panic the compiler so I decided to do different approach. I simply cloned the parse-display repo, set PATH to point at my custom rustc with debug info and did cargo build --verbose --release -pparse-display-derive followed by cargo clean -pparse-display-derive && find target -name '*parse_display_derive*' -exec rm {} \;.

Subsequent builds of parse_display_derive like so, (command lifted out of cargo) given different crate hash each time.

USTC_LOG=rustc_interface=trace,info /home/shana/programming/rust/build/x86_64-unknown-linux-gnu/stage1/bin/rustc --crate-name parse_display_derive --edition=2018 parse-display-derive/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type proc-macro --emit=dep-info,link -C prefer-dynamic -C opt-level=3 -C embed-bitcode=no -C metadata=b84aec42a06fb464 -C extra-filename=-b84aec42a06fb464 --out-dir /tmp/parse-display/target/release/deps -L dependency=/tmp/parse-display/target/release/deps --extern once_cell=/tmp/parse-display/target/release/deps/libonce_cell-f85256786e437989.rlib --extern proc_macro2=/tmp/parse-display/target/release/deps/libproc_macro2-c8c830d884b73582.rlib --extern quote=/tmp/parse-display/target/release/deps/libquote-36957c67d00968b9.rlib --extern regex=/tmp/parse-display/target/release/deps/libregex-18b6f9d4e4577b8b.rlib --extern regex_syntax=/tmp/parse-display/target/release/deps/libregex_syntax-696c6c87ad015395.rlib --extern structmeta=/tmp/parse-display/target/release/deps/libstructmeta-9b38df7e221d1bda.rlib --extern syn=/tmp/parse-display/target/release/deps/libsyn-b4b89df1fc010a55.rlib --extern proc_macro

I see that in crate_hash function, hir_body_nodes is different between the two runs, even after sorting. I will debug now. I did briefly go through the functions invoked in there to see if there's some random place with some unstable data structure like hashmaps being used but I didn't see anything immediately stand out.

Once I had the crate_hash output (and adding extra info), I found a line that gave differing result:

@@ -5309,7 +5309,7 @@ kind:
 Lit(Spanned
 {
 node:
-Str("dump",
+Str("default",
 Cooked),
 span:
 parse-display-derive/src/lib.rs:678:10:
@@ -5337,7 +5337,7 @@ kind:
 Lit(Spanned
 {
 node:
-Str("default",
+Str("dump",
 Cooked),
 span:
 parse-display-derive/src/lib.rs:678:10:
@@ -5430,7 +5430,7 @@ kind:
 Lit(Spanned
 {
 node:
-Str("regex",
+Str("new",
 Cooked),
 span:
 parse-display-derive/src/lib.rs:678:10:
@@ -5458,7 +5458,7 @@ kind:
 Lit(Spanned
 {
 node:
-Str("new",
+Str("regex",
 Cooked),
 span:
 parse-display-derive/src/lib.rs:678:10:
@@ -6432,8 +6432,8 @@ local_id:
 178
 },
 span:

If we look in parse-display-derive source, we see this https://github.com/frozenlib/parse-display/blob/e105b49e715549c8c40e73a980587deb12e49983/parse-display-derive/src/lib.rs#L678

Possibly this derive produces code with non-deterministic ordering of identifiers? I will dive into structmeta crate.

If yes, this would explain a lot:

I'm not sure if there's much rustc itself can do if this is the case but I will reconfirm once I look into structmeta crate.

Maybe rustc could do something like sort identifiers with same spans but it seems janky. Anyway, off to check structmeta.

EDIT: It may still be parse-display, mis-using structmeta of course...

Fuuzetsu commented 3 years ago

I updated my rust-analyzer and was able to expand the derive macro. Definitely suspicious, especially look at those _value3, _value1 etc orderings. I didn't show this initially but they were actually large part of the diff. I think it supports my theory that it's some macro traversing an unstable structure. Indulge me a little bit more until I verify and then I can hopefully just close the ticket ā€“ it'll be great if it's not a rustc problem!

diff --git a/tmp/one_lines b/tmp/two_lines
index 2e1be23..6041042 100644
--- a/tmp/one_lines
+++ b/tmp/two_lines
@@ -6,8 +6,8 @@ parse_display_derive[c310]::{impl#21}::parse),
 Some(OwnerNodes
 {
 hash:
-Fingerprint(11937788502066574632,
-3118914414890280417),
+Fingerprint(1511079356857906158,
+15621007578138819097),
 nodes:
 [Some(ParentedNode
 {
@@ -633,7 +633,7 @@ parse_display_derive[c310]::{impl#21}::parse),
 local_id:
 6
 },
-_value_0#1612,
+_value_4#1612,
 None),
 span:
 parse-display-derive/src/lib.rs:678:10:
@@ -745,7 +745,7 @@ parse_display_derive[c310]::{impl#21}::parse),
 local_id:
 6
 },
-_value_0#1612,
+_value_4#1612,
 None),
 span:
 parse-display-derive/src/lib.rs:678:10:
<snip>
// Recursive expansion of StructMeta! macro
// =========================================

#[automatically_derived]
impl::syn::parse::Parse for DisplayArgs {
  fn parse(input: ::syn::parse::ParseStream< '_>) ->  ::syn::Result<Self>{
    let mut _value_0:Option<LitStr>  = None;
    let mut _value_1 = None;
    let mut _value_2 = None;
    let mut _value_3 = None;
    let mut is_next = false;
    let mut unnamed_index = 0;
    let mut named_used = false;
    while!input.is_empty(){
      if is_next {
        input.parse:: < ::syn::Token![,]>()? ;
        if input.is_empty(){
          break;

        }
      }is_next = true;
      if let Some((index,span)) =  ::structmeta::helpers::try_parse_name(input, &["dump",],false, &["style",],false, &["bound",],false,false)?{
        named_used = true;
        match index {
          ::structmeta::helpers::NameIndex::Flag(Ok(0usize)) => {
            if _value_3.is_some(){
              return Err(::syn::Error::new(span,"parameter `dump` speficied more than once"));

            }_value_3 = Some(span);

          }
          ::structmeta::helpers::NameIndex::NameValue(Ok(0usize)) => {
            if _value_1.is_some(){
              return Err(::syn::Error::new(span,"parameter `style` speficied more than once"));

            }_value_1 = Some(input.parse:: <LitStr>()?);

          }
          ::structmeta::helpers::NameIndex::NameArgs(Ok(0usize)) => {
            if _value_2.is_some(){
              return Err(::syn::Error::new(span,"parameter `bound` speficied more than once"));

            }_value_2 = Some({
              let content;
              ::syn::parenthesized!(content in input);
              ::syn::punctuated::Punctuated:: <Quotable<Bound> , ::syn::Token![,]> ::parse_terminated(&content)? .into_iter().collect()
            });

          }
          _ => unreachable!()

        }
      }else {
        if named_used {
          return Err(input.error("cannot use unnamed parameter after named parameter"));

        }match unnamed_index {
          0usize => {
            _value_0 = Some(input.parse:: <LitStr>()?);

          }
          _ => {
            return Err(input.error("too many unnamed parameter"));

          }

        }unnamed_index+=1;

      }
    }Ok(Self {
      format:_value_0,style:_value_1,bound:_value_2,dump:_value_3.is_some(),
    })
  }

}
Fuuzetsu commented 3 years ago

OK, I found it:

named: HashMap<String, NamedParam<'a>>,
    fn named_ps(&self, f: impl Fn(&NamedParamType<'a>) -> bool) -> (Vec<&NamedParam<'a>>, bool) {
        (
            self.named.values().filter(|p| f(&p.ty)).collect(),
            if let Some(p) = &self.rest {
                f(&p.ty)
            } else {
                false
            },
        )
    }

this then feeds into

        let (flag_ps, flag_rest) = self.named_ps(|p| p.is_flag());
        let (name_value_ps, name_value_rest) = self.named_ps(|p| p.is_name_value());
        let (name_args_ps, name_args_rest) = self.named_ps(|p| p.is_name_args());

        let mut arms_named = Vec::new();
        for (index, p) in flag_ps.iter().enumerate() {
            arms_named.push(p.build_arm_parse(index, ArgKind::Flag));
        }

and build_arm_parse stuff has

    fn build_arm_parse(&self, index: usize, kind: ArgKind) -> TokenStream {
        let temp_ident = &self.info.temp_ident;
        let msg = format!("parameter `{}` speficied more than once", self.name);
        let span = self.info.field.span();
        let expr = self.ty.build_parse_expr(kind, span);
        let var = kind.to_helper_name_index_variant();
        quote_spanned! { span=>
            ::structmeta::helpers::NameIndex::#var(Ok(#index)) => {
                if #temp_ident.is_some() {
                    return Err(::syn::Error::new(span, #msg));
                }
                #temp_ident = Some(#expr);
            }
        }
    }

So this crate is producing unstable output and stomping all over the crate hash!

Thank you for indulging me. I'll close the ticket; I'll re-open if necessary though I doubt it.

Fuuzetsu commented 3 years ago

Upstream fix at https://github.com/frozenlib/structmeta/pull/1

I even get the same binary now whereas before it'd change hash every time.

Thanks for your time.