Page MenuHomePhabricator

Use sha256 for source file debug info checksums with MSVC compat >= 1930
Needs ReviewPublic

Authored by hans on Jan 7 2022, 12:00 PM.

Details

Reviewers
thakis
rnk
arlosi
Summary

From the VS2022 release notes, it sounds like newer MSVC versions are using SHA256 for these checksums: (search for "SHA-256" in https://docs.microsoft.com/en-us/visualstudio/releases/2022/release-notes#17.0.0)

Since D75785 laid the groundwork, let's hook it up.

While here, I noticed llvm::SHA256 doesn't have a method to get the hash as a hex string, like llvm::MD5. But we can use llvm::toHex() and actually that could be made more efficient and llvm::MD5 could use that too.

Diff Detail

Event Timeline

hans created this revision.Jan 7 2022, 12:00 PM
hans requested review of this revision.Jan 7 2022, 12:00 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJan 7 2022, 12:00 PM
hans added a comment.Jan 7 2022, 12:06 PM

After sending this, I realized we should probably have a test to make sure it gets all the way into the .o and .pdb files correctly...

And, maybe we should hook up the /ZH flag? (https://docs.microsoft.com/en-us/cpp/build/reference/zh?view=msvc-170&preserve-view=true still says MD5 is default, but I guess that's just out of date)

I submitted a change last year to hook up the /ZH option, but it was never completed. https://reviews.llvm.org/D98438

rnk added a comment.Jan 7 2022, 3:23 PM

I think it's worth taking the time to hook up /ZH. Sorry we missed the patch last March.

hans added a comment.Jan 10 2022, 10:15 AM

I submitted a change last year to hook up the /ZH option, but it was never completed. https://reviews.llvm.org/D98438

I just put some comments on that. I'm happy to provide review if you think you have time to complete it?

(I'll break out the toHex() related changes here and try to get those landed separately.)