-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Add hash of source files in debug info #69718
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @estebank (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
☔ The latest upstream changes (presumably #69851) made this pull request unmergeable. Please resolve the merge conflicts. |
☔ The latest upstream changes (presumably #70132) made this pull request unmergeable. Please resolve the merge conflicts. |
Seems like this would fix #68980. |
So I opened a quick Zulip topic to discuss. I'd like to see this move forward. One thing I would like to see before we land it though is
|
r? @eddyb |
@arlosi @rylev How would you feel about keeping this MD5-only to start, replacing the existing And we could even turn that flag by default (or not have a flag at all), that part I don't have too many opinions about. EDIT: I suppose we could toggle between MD5 and SHA1 based on the target spec. |
@eddyb storing the hash algorithm in the target spec makes sense to me. I've updated the PR to reflect this change. Because the |
@arlosi Can you make creating a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, r=me if this isn't waiting on anything else.
@bors r+ (assuming @nikomatsakis' "intent to merge" applies) |
📌 Commit 776ebaf02071f99437c0040814904761c90c67f5 has been approved by |
Removing nomination, this was already discussed and it's about to be merged :). |
☔ The latest upstream changes (presumably #70692) made this pull request unmergeable. Please resolve the merge conflicts. |
* Adds either an MD5 or SHA1 hash to the debug info. * Adds new unstable option `-Z src-hash-algorithm` to control the hashing algorithm.
@eddyb could you re-approve? I resolved merge conflicts. |
@bors r+ (feel free to PM me on Zulip or elsewhere when this happens) |
📌 Commit f86b078 has been approved by |
☀️ Test successful - checks-azure |
Fix performance regression in debuginfo file_metadata. Fixes performance regression caused by rust-lang#69718. Finding the `SourceFile` associated with a `FileName` called `get_source_file` on the `SourceMap`, which does a linear search through all files in the `SourceMap`. This resolves the issue by passing the `SourceFile` in from the caller (which already had it available) instead of the `FileName` Fixes rust-lang#70785.
LLVM supports placing the hash of source files inside the debug info.
This information can be used by a debugger to verify that the source code matches
the executable.
This change adds support for both hash algorithms supported by LLVM, MD5 and SHA1, controlled by a target option.
Fixes #68980.
Tracking issue: #70401
rustc dev guide PR with further details: rust-lang/rustc-dev-guide#623