This is an archive of the discontinued LLVM Phabricator instance.

Clang: Allow selecting the hash algorithm for file checksums in debug info.
AbandonedPublic

Authored by arlosi on Mar 11 2021, 10:20 AM.

Details

Summary

Adds clang-cl support for the /ZH: option used to select MD5, SHA1,
or SHA_256 hashing algorithms in debug info.

Previously only the MD5 algorithm was supported.

Diff Detail

Event Timeline

arlosi created this revision.Mar 11 2021, 10:20 AM
arlosi requested review of this revision.Mar 11 2021, 10:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 11 2021, 10:20 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
jansvoboda11 added inline comments.Mar 11 2021, 11:20 PM
clang/include/clang/Driver/Options.td
2775

Can you please use a marshalling annotation here and remove the code in CompilerInvocation.cpp? https://clang.llvm.org/docs/InternalsManual.html#adding-new-command-line-option

arlosi updated this revision to Diff 330386.Mar 12 2021, 3:25 PM
arlosi marked an inline comment as done.

Use automatic marshaling for command line arguments.

hans added a subscriber: hans.Jan 10 2022, 10:14 AM

Sorry for not seeing this sooner.

I think this looks great, just a few comments, and it also needs tests.

clang/include/clang/Basic/CodeGenOptions.def
303

Why does it need 4 bits? Wouldn't 2 be enough?

clang/lib/CodeGen/CGDebugInfo.cpp
368

I'm not sure changing the return type helps that much? I suppose it makes the calling code a little cleaner, but I think it makes the function itself a little more confusing.

398

toHex() from StringExtras.h is much nicer, and MD5 should be using that internally too.

hans added a comment.Jan 21 2022, 5:24 AM

Do you think you'll have time to work on this? Otherwise I'd be happy to take it over.

I wouldn't be able to look at this for a few weeks. If you have the time to
take it over, that works for me. Thanks!

thakis added a subscriber: thakis.

I addressed pending comments, did some other minor tweaks, and uploaded the modified diff to D134544.

Herald added a project: Restricted Project. · View Herald TranscriptSep 23 2022, 8:28 AM
Herald added a subscriber: MaskRay. · View Herald Transcript

So this one can be marked abandoned? You might need to commandeer it first.

arlosi abandoned this revision.Sep 23 2022, 1:08 PM

Closing in favor of D134544