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

LF_BUILDINFO introduces non-determinism to Windows rlib builds #128842

Open
capickett opened this issue Aug 8, 2024 · 2 comments
Open

LF_BUILDINFO introduces non-determinism to Windows rlib builds #128842

capickett opened this issue Aug 8, 2024 · 2 comments
Labels
A-reproducibility Area: Reproducible / deterministic builds C-bug Category: This is a bug. O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@capickett
Copy link
Contributor

The changes from #113492 have introduced a source of non-determinism in our rustc invocations, which causes cache invalidations for us in Buck since the output often changes hashes.

The root issue here seems to be that LF_BUILDINFO now contains an absolute path to rustc.exe, which is not consistent for our use-case in Meta (our build machines use https://github.com/facebook/dotslash in order to download rustc to a temporary directory which is not stable across hosts). The LF_BUILDINFO entry is present in every rlib artifact we produce.

Details

  1. Build an rlib crate on windows
  2. ar x foo.rlib to look at the rcgu.o files
  3. llvm-pdbutil dump -all foo-<hash>.foo.<hash>-cgu.0.rcgu.o

Contents for our Buck built rlib look like:

0x100C | LF_STRING_ID [size = 152] ID: <no type>, String: C:\remote_execution\dotslash_cache\keyed_blake3\dc\ef\<hash>\data\bin\rustc.exe

The problematic string ends up embedded in the object files within the rlib archive, introducing non-determinism in our build outputs.

I'd like to adjust this by proposing one of a few ideas:

  1. Amending LF_BUILDINFO to accept a relative path (or user-defined path) to rustc.exe. E.g. -Zbuildinfo_exe_path=rustc.exe or similar.
  2. Omit LF_BUILDINFO's command & command line args entirely, reverting back to pre- Add CL and CMD into to pdb debug info #113492 behavior. E.g. -Zomit_buildinfo_args or similar.
  3. Something else?

Would any of these options make sense to pursue as a PR?

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 8, 2024
@MolecularMatters
Copy link

@capickett Assuming that you still use C++ with Buck, I'm wondering what the produced PDB files contain in these cases?
As far as I am aware, both MSVC and Clang will store absolute paths in the compiland environments for C++ as well.

If possible, I think it would be great if the PDB information produced by the Rust compiler matched that of other language toolchains.

@bjorn3
Copy link
Member

bjorn3 commented Aug 9, 2024

How do msvc and clang handle this? My understanding is that both should also put their own invocation in the pdb file. #113492 was supposed to reproduce the behavior of msvc and clang for tools that need this.

@saethlin saethlin added O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. A-reproducibility Area: Reproducible / deterministic builds and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-reproducibility Area: Reproducible / deterministic builds C-bug Category: This is a bug. O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants