-
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
Protect crate metadata from corruption via SHA-256 hash #87896
Conversation
r? @cjgillot (rust-highfive has picked a reviewer for you, use r? to override) |
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 297e23fc026279541fea4940689d2d4c041b215a with merge af0cbac7c33f68cd7cc4b0c0d627193cdb60b5f8... |
☀️ Try build successful - checks-actions |
Queued af0cbac7c33f68cd7cc4b0c0d627193cdb60b5f8 with parent ae90dcf, future comparison URL. |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking try commit (af0cbac7c33f68cd7cc4b0c0d627193cdb60b5f8): comparison url. Summary: This change led to significant regressions 😿 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf. Next Steps: If you can justify the regressions found in this perf run, please indicate this with @bors rollup=never |
We now compute a SHA-256 of the raw (encoded) crate metadata, and append it to the final crate metadata that we store on disk. When we load the metadata, we compute the hash from the metadata blob, and verify that matches the hash stored at the end of the blob. This allows us to detect on-disk corruption of the metadata file, which might later cause a build failure much later in compilation. If anyone is manually editing crate metadata on-disk, they will need to re-compute and modify the hash at the end of the blob. This will allow us to determine whether or not crate metadata corruption is causing some of the unusual incr-comp failures we've been seeing. The incremental compilation data itself will be hashed in a follow-up PR.
297e23f
to
7ee8972
Compare
I am not really convinced why we need to do that, and in which cases this feature would be useful. |
☔ The latest upstream changes (presumably #82183) made this pull request unmergeable. Please resolve the merge conflicts. |
Maybe use a faster non-cryptographically secure hash or maybe even just a crc or other kind of checksum?
Hash checking mostly. Probably because previously the crate metadata was lazily loaded. Now it has to be eagerly loaded for the hash check. |
https://github.com/BLAKE3-team/BLAKE3 might be interesting for something like this. |
Triage: |
@Aaron1011 @rustbot label: +S-inactive |
We now compute a SHA-256 of the raw (encoded) crate metadata,
and append it to the final crate metadata that we store on disk.
When we load the metadata, we compute the hash from the metadata
blob, and verify that matches the hash stored at the end of the
blob.
This allows us to detect on-disk corruption of the metadata file,
which might later cause a build failure much later in compilation.
If anyone is manually editing crate metadata on-disk,
they will need to re-compute and modify the hash at
the end of the blob.
This will allow us to determine whether or not crate metadata
corruption is causing some of the unusual incr-comp failures
we've been seeing. The incremental compilation data itself
will be hashed in a follow-up PR.