[clang-cl] Implement /ZH: flag

[thakis]

Summary

Based on a patch by Arlo Siemsen (D98438)!

[thakis]

tweak

[probinson]

Probably we should reject the -gsrc... option if we're emitting DWARF, which requires MD5 (at least so far).

[thakis]

In D134544#3811867, @probinson wrote:

Probably we should reject the -gsrc... option if we're emitting DWARF, which requires MD5 (at least so far).

It's a cc1 option. As those aren't considered user facing, we don't do a lot of checking for them usually.

[hans]

nice!

clang/include/clang/Basic/CodeGenOptions.def

344

ultra nit: period. But maybe the comment is superfluous?

clang/include/clang/Basic/CodeGenOptions.h

106

what does CSK stand for here?

[thakis]

clang/include/clang/Basic/CodeGenOptions.def

344

Seems superfluous to me too, but this file is very consistent about having superfluous comments for every option. I'll add the trailing period.

clang/include/clang/Basic/CodeGenOptions.h

106

I'm guessing "check sum kind", but I don't know :)

SHA_ looks a bit funny as prefix too...

I also don't love the cc1 flag name, but I figured it's a bit bikesheddy and it's internal only so i kept it as is.

If you have a preference for flag name, enum name, and enum prefix, let me know.

[probinson]

clang/test/CodeGen/debug-info-file-checksum.c

4

I thought -gsrc... was not a driver option, but here it is being passed to the driver?

[thakis]

clang/test/CodeGen/debug-info-file-checksum.c

4

Good catch, thanks. https://reviews.llvm.org/D134544#inline-1297003 does mark it as CC1Option. It's definitely meant to be a driver option, let me look into why this works here. It shouldn't.

[thakis]

clang/test/CodeGen/debug-info-file-checksum.c

4

Aha, I want NoDriverOption too. Update coming in a bit…

[thakis]

don't accidentally add a flag for this to the gcc-style driver

[hans]

lgtm

clang/include/clang/Basic/CodeGenOptions.h

106

How about

enum DebugSrcHashKind { DSH_MD5, ...

("kind" seems a common suffix for these, "debug" is a nice hint about what area this applies to.)

clang/lib/CodeGen/CGDebugInfo.cpp

377

Perhaps it would be simpler to drop the CSKind variable and fold the return into the individual cases?

[thakis]

final tweaks

clang/include/clang/Basic/CodeGenOptions.h

106

DebugSourceHash it is.

clang/lib/CodeGen/CGDebugInfo.cpp

377

Oh, that's much nicer, thanks.

[dblaikie]

clang/include/clang/Driver/Options.td

3220

This is a driver flag, yeah? Might be worth documenting/emitting a warning that it's not compatible with DWARF emission? (looks like the DWARF code in LLVM ignores any hash that's not MD5, because DWARF doesn't have a way to encode other hash algorithms currently - though maybe there's some extension space available to add it, I haven't checked/looked)

[thakis]

clang/include/clang/Driver/Options.td

3220

belated: this is Flags<[CC1Option, NoDriverOption]>, in the line below, so I think this isn't a driver flag. (It's what the CLFlags added below expand to – those are driver flags, but only for clang-cl.)

[dblaikie]

clang/include/clang/Driver/Options.td

3220

Oh, I see - I think I was confused by the driver code referencing gsrc_hash_EQ - but that's because the /ZH are implemented as aliases for gsrc_hash_EQ.

I guess then maybe my comment applies to /ZH - if someone asks for DWARF and uses that flag? But maybe not worth it...

clang/include/clang/Basic/CodeGenOptions.h

enum EmbedBitcodeKind {
     Embed_Marker    // Embed a marker as a placeholder for bitcode.
   };
 
   enum InlineAsmDialectKind {
     IAD_ATT,
     IAD_Intel,
   };
 
+  enum DebugSrcHashKind {
+    DSH_MD5,
+    DSH_SHA1,
+    DSH_SHA256,
+  };
+
   // This field stores one of the allowed values for the option
   // -fbasic-block-sections=.  The allowed values with this option are:
   // {"labels", "all", "list=<file>", "none"}.
   //
   // "labels":      Only generate basic block symbols (labels) for all basic
   //                blocks, do not generate unique sections for basic blocks.
   //                Use the machine basic block id in the symbol name to
   //                associate profile info from virtual address to machine

clang/include/clang/Basic/CodeGenOptions.def

 
 CODEGENOPT(DebugTypeExtRefs, 1, 0) ///< Whether or not debug info should contain
                                    ///< external references to a PCH or module.
 
 CODEGENOPT(DebugExplicitImport, 1, 0)  ///< Whether or not debug info should
                                        ///< contain explicit imports for
                                        ///< anonymous namespaces
 
+/// Set debug info source file hashing algorithm.
+ENUM_CODEGENOPT(DebugSrcHash, DebugSrcHashKind, 2, DSH_MD5)
+
 CODEGENOPT(SplitDwarfInlining, 1, 1) ///< Whether to include inlining info in the
                                      ///< skeleton CU to allow for symbolication
                                      ///< of inline stack frames without .dwo files.
 CODEGENOPT(DebugFwdTemplateParams, 1, 0) ///< Whether to emit complete
                                          ///< template parameter descriptions in
                                          ///< forward declarations (versus just
                                          ///< including them in the name).
 ENUM_CODEGENOPT(DebugSimpleTemplateNames, codegenoptions::DebugTemplateNamesKind, 2, codegenoptions::DebugTemplateNamesKind::Full) ///< Whether to emit template parameters

clang/include/clang/Driver/Options.td

def gsplit_dwarf_EQ : Joined<["-"], "gsplit-dwarf=">, Group<g_flags_Group>,
   Values<"split,single">;
 def gno_split_dwarf : Flag<["-"], "gno-split-dwarf">, Group<g_flags_Group>;
 def gsimple_template_names : Flag<["-"], "gsimple-template-names">, Group<g_flags_Group>;
 def gsimple_template_names_EQ
     : Joined<["-"], "gsimple-template-names=">,
       HelpText<"Use simple template names in DWARF, or include the full "
                "template name with a modified prefix for validation">,
       Values<"simple,mangled">, Flags<[CC1Option, NoDriverOption]>;
+def gsrc_hash_EQ : Joined<["-"], "gsrc-hash=">,
+  Group<g_flags_Group>, Flags<[CC1Option, NoDriverOption]>,
+  Values<"md5,sha1,sha256">,
+  NormalizedValues<["DSH_MD5", "DSH_SHA1", "DSH_SHA256"]>,
+  NormalizedValuesScope<"CodeGenOptions">,
+  MarshallingInfoEnum<CodeGenOpts<"DebugSrcHash">, "DSH_MD5">;
 def gno_simple_template_names : Flag<["-"], "gno-simple-template-names">,
                                 Group<g_flags_Group>;
 def ggnu_pubnames : Flag<["-"], "ggnu-pubnames">, Group<g_flags_Group>, Flags<[CC1Option]>;
 def gno_gnu_pubnames : Flag<["-"], "gno-gnu-pubnames">, Group<g_flags_Group>;
 def gpubnames : Flag<["-"], "gpubnames">, Group<g_flags_Group>, Flags<[CC1Option]>;
 def gno_pubnames : Flag<["-"], "gno-pubnames">, Group<g_flags_Group>;
 def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group<g_flags_Group>;
 def gmodules : Flag <["-"], "gmodules">, Group<gN_Group>,
def _SLASH_Zc_twoPhase_ : CLFlag<"Zc:twoPhase-">,
   HelpText<"Disable two-phase name lookup in templates (default)">,
   Alias<fdelayed_template_parsing>;
 def _SLASH_Zc_wchar_t : CLFlag<"Zc:wchar_t">,
   HelpText<"Enable C++ builtin type wchar_t (default)">;
 def _SLASH_Zc_wchar_t_ : CLFlag<"Zc:wchar_t-">,
   HelpText<"Disable C++ builtin type wchar_t">;
 def _SLASH_Z7 : CLFlag<"Z7">,
   HelpText<"Enable CodeView debug information in object files">;
+def _SLASH_ZH_MD5 : CLFlag<"ZH:MD5">,
+  HelpText<"Use MD5 for file checksums in debug info (default)">,
+  Alias<gsrc_hash_EQ>, AliasArgs<["md5"]>;
+def _SLASH_ZH_SHA1 : CLFlag<"ZH:SHA1">,
+  HelpText<"Use SHA1 for file checksums in debug info">,
+  Alias<gsrc_hash_EQ>, AliasArgs<["sha1"]>;
+def _SLASH_ZH_SHA_256 : CLFlag<"ZH:SHA_256">,
+  HelpText<"Use SHA256 for file checksums in debug info">,
+  Alias<gsrc_hash_EQ>, AliasArgs<["sha256"]>;
 def _SLASH_Zi : CLFlag<"Zi">, Alias<_SLASH_Z7>,
   HelpText<"Like /Z7">;
 def _SLASH_Zp : CLJoined<"Zp">,
   HelpText<"Set default maximum struct packing alignment">,
   Alias<fpack_struct_EQ>;
 def _SLASH_Zp_flag : CLFlag<"Zp">,
   HelpText<"Set default maximum struct packing alignment to 1">,
   Alias<fpack_struct_EQ>, AliasArgs<["1"]>;

 def _SLASH_w : CLIgnoredJoined<"w">;
 def _SLASH_Wv_ : CLIgnoredJoined<"Wv">;
 def _SLASH_Zc___cplusplus : CLIgnoredFlag<"Zc:__cplusplus">;
 def _SLASH_Zc_auto : CLIgnoredFlag<"Zc:auto">;
 def _SLASH_Zc_forScope : CLIgnoredFlag<"Zc:forScope">;
 def _SLASH_Zc_inline : CLIgnoredFlag<"Zc:inline">;
 def _SLASH_Zc_rvalueCast : CLIgnoredFlag<"Zc:rvalueCast">;
 def _SLASH_Zc_ternary : CLIgnoredFlag<"Zc:ternary">;
-def _SLASH_ZH_MD5 : CLIgnoredFlag<"ZH:MD5">;
-def _SLASH_ZH_SHA1 : CLIgnoredFlag<"ZH:SHA1">;
-def _SLASH_ZH_SHA_256 : CLIgnoredFlag<"ZH:SHA_256">;
 def _SLASH_Zm : CLIgnoredJoined<"Zm">;
 def _SLASH_Zo : CLIgnoredFlag<"Zo">;
 def _SLASH_Zo_ : CLIgnoredFlag<"Zo-">;
 
 
 // Unsupported:
 
 def _SLASH_await : CLFlag<"await">;

clang/lib/CodeGen/CGDebugInfo.h

private:
   /// Return current directory name.
   StringRef getCurrentDirname();
 
   /// Create new compile unit.
   void CreateCompileUnit();
 
   /// Compute the file checksum debug info for input file ID.
   Optional<llvm::DIFile::ChecksumKind>
-  computeChecksum(FileID FID, SmallString<32> &Checksum) const;
+  computeChecksum(FileID FID, SmallString<64> &Checksum) const;
 
   /// Get the source of the given file ID.
   Optional<StringRef> getSource(const SourceManager &SM, FileID FID);
 
   /// Convenience function to get the file debug info descriptor for the input
   /// location.
   llvm::DIFile *getOrCreateFile(SourceLocation Loc);
 

clang/lib/CodeGen/CGDebugInfo.cpp

 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/SHA1.h"
+#include "llvm/Support/SHA256.h"
 #include "llvm/Support/TimeProfiler.h"
 using namespace clang;
 using namespace clang::CodeGen;
 
 static uint32_t getTypeAlignIfRequired(const Type *Ty, const ASTContext &Ctx) {
   auto TI = Ctx.getTypeInfo(Ty);
   return TI.isAlignRequired() ? TI.Align : 0;
 }
if (CGM.getLangOpts().CPlusPlus) {
       }
     }
   }
 
   return StringRef();
 }
 
 Optional<llvm::DIFile::ChecksumKind>
-CGDebugInfo::computeChecksum(FileID FID, SmallString<32> &Checksum) const {
+CGDebugInfo::computeChecksum(FileID FID, SmallString<64> &Checksum) const {
   Checksum.clear();
 
   if (!CGM.getCodeGenOpts().EmitCodeView &&
       CGM.getCodeGenOpts().DwarfVersion < 5)
     return None;
 
   SourceManager &SM = CGM.getContext().getSourceManager();
   Optional<llvm::MemoryBufferRef> MemBuffer = SM.getBufferOrNone(FID);
   if (!MemBuffer)
     return None;
 
-  llvm::toHex(
-      llvm::MD5::hash(llvm::arrayRefFromStringRef(MemBuffer->getBuffer())),
-      /*LowerCase*/ true, Checksum);
+  auto Data = llvm::arrayRefFromStringRef(MemBuffer->getBuffer());
+  switch (CGM.getCodeGenOpts().getDebugSrcHash()) {
+  case clang::CodeGenOptions::DSH_MD5:
+    llvm::toHex(llvm::MD5::hash(Data), /*LowerCase=*/true, Checksum);
     return llvm::DIFile::CSK_MD5;
+  case clang::CodeGenOptions::DSH_SHA1:
+    llvm::toHex(llvm::SHA1::hash(Data), /*LowerCase=*/true, Checksum);
+    return llvm::DIFile::CSK_SHA1;
+  case clang::CodeGenOptions::DSH_SHA256:
+    llvm::toHex(llvm::SHA256::hash(Data), /*LowerCase=*/true, Checksum);
+    return llvm::DIFile::CSK_SHA256;
+  }
 }
 
 Optional<StringRef> CGDebugInfo::getSource(const SourceManager &SM,
                                            FileID FID) {
   if (!CGM.getCodeGenOpts().EmbedSource)
     return None;
 
   bool SourceInvalid = false;
   StringRef Source = SM.getBufferData(FID, &SourceInvalid);
 
   if (SourceInvalid)
     return None;
 
llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) {
   // Cache the results.
   auto It = DIFileCache.find(FileName.data());
   if (It != DIFileCache.end()) {
     // Verify that the information still exists.
     if (llvm::Metadata *V = It->second)
       return cast<llvm::DIFile>(V);
   }
 
-  SmallString<32> Checksum;
+  SmallString<64> Checksum;
 
   Optional<llvm::DIFile::ChecksumKind> CSKind = computeChecksum(FID, Checksum);
   Optional<llvm::DIFile::ChecksumInfo<StringRef>> CSInfo;
   if (CSKind)
     CSInfo.emplace(*CSKind, Checksum);
   return createFile(FileName, CSInfo, getSource(SM, SM.getFileID(Loc)));
 }
 
StringRef CGDebugInfo::getCurrentDirname() {
   llvm::ErrorOr<std::string> CWD =
       CGM.getFileSystem()->getCurrentWorkingDirectory();
   if (!CWD)
     return StringRef();
   return CWDName = internString(*CWD);
 }
 
 void CGDebugInfo::CreateCompileUnit() {
-  SmallString<32> Checksum;
+  SmallString<64> Checksum;
   Optional<llvm::DIFile::ChecksumKind> CSKind;
   Optional<llvm::DIFile::ChecksumInfo<StringRef>> CSInfo;
 
   // Should we be asking the SourceManager for the main file name, instead of
   // accepting it as an argument? This just causes the main file name to
   // mismatch with source locations and create extra lexical scopes or
   // mismatched debug info (a CU with a DW_AT_file of "-", because that's what
   // the driver passed, but functions/other things have DW_AT_file of "<stdin>"

clang/lib/Driver/ToolChains/Clang.cpp

if (SimpleTemplateNamesArg &&
       checkDebugInfoOption(SimpleTemplateNamesArg, Args, D, TC)) {
     const auto &Opt = SimpleTemplateNamesArg->getOption();
     if (Opt.matches(options::OPT_gsimple_template_names)) {
       ForwardTemplateParams = true;
       CmdArgs.push_back("-gsimple-template-names=simple");
     }
   }
 
+  if (const Arg *A = Args.getLastArg(options::OPT_gsrc_hash_EQ)) {
+    StringRef v = A->getValue();
+    CmdArgs.push_back(Args.MakeArgString("-gsrc-hash=" + v));
+  }
+
   if (Args.hasFlag(options::OPT_fdebug_ranges_base_address,
                    options::OPT_fno_debug_ranges_base_address, false)) {
     CmdArgs.push_back("-fdebug-ranges-base-address");
   }
 
   // -gdwarf-aranges turns on the emission of the aranges section in the
   // backend.
   // Always enabled for SCE tuning.

clang/test/CodeGen/debug-info-file-checksum.c

+// RUN: %clang -emit-llvm -S -g -gcodeview -x c \
+// RUN:     %S/Inputs/debug-info-file-checksum.c -o - | FileCheck %s
+// RUN: %clang -emit-llvm -S -g -gcodeview -Xclang -gsrc-hash=md5 \
+// RUN:     -x c %S/Inputs/debug-info-file-checksum.c -o - | FileCheck %s
+// RUN: %clang -emit-llvm -S -g -gcodeview -Xclang -gsrc-hash=sha1 \
+// RUN:     -x c %S/Inputs/debug-info-file-checksum.c -o - \
+// RUN:     | FileCheck --check-prefix=SHA1 %s
+// RUN: %clang -emit-llvm -S -g -gcodeview -Xclang -gsrc-hash=sha256 \
+// RUN:     -x c %S/Inputs/debug-info-file-checksum.c -o - \
+// RUN:     | FileCheck --check-prefix=SHA256 %s
 // RUN: %clang -emit-llvm -S -g -gcodeview -x c %S/Inputs/debug-info-file-checksum.c -o - | FileCheck %s
 // RUN: %clang -emit-llvm -S -gdwarf-5 -x c %S/Inputs/debug-info-file-checksum.c -o - | FileCheck %s
 
 // Check that "checksum" is created correctly for the compiled file.
 
 // CHECK: !DIFile(filename:{{.*}}, directory:{{.*}}, checksumkind: CSK_MD5, checksum: "a3b7d27af071accdeccaa933fc603608")
+// SHA1: !DIFile(filename:{{.*}}, directory:{{.*}}, checksumkind: CSK_SHA1, checksum: "6f6eeaba705ad6db6fbb05c2cbcf3cbb3e374bcd")
+// SHA256: !DIFile(filename:{{.*}}, directory:{{.*}}, checksumkind: CSK_SHA256, checksum: "2d49b53859e57898a0f8c16ff1fa4d99306b8ec28d65cf7577109761f0d56197")
 
 // Ensure #line directives (in already pre-processed files) do not emit checksums
 // RUN: %clang -emit-llvm -S -g -gcodeview -x c %S/Inputs/debug-info-file-checksum-pre.cpp -o - | FileCheck %s --check-prefix NOCHECKSUM
 
 // NOCHECKSUM: !DIFile(filename: "{{.*}}code-coverage-filter1.h", directory: "{{[^"]*}}")
 // NOCHECKSUM: !DIFile(filename: "{{.*}}code-coverage-filter2.h", directory: "{{[^"]*}}")
 // NOCHECKSUM: !DIFile(filename: "{{.*}}debug-info-file-checksum.c", directory: "{{[^"]*}}")
 
 // Ensure #line directives without name do emit checksums
 // RUN: %clang -emit-llvm -S -g -gcodeview -x c %S/Inputs/debug-info-file-checksum-line.cpp -o - | FileCheck %s --check-prefix CHECKSUM
 
 // CHECKSUM: !DIFile(filename: "{{.*}}debug-info-file-checksum-line.cpp", directory:{{.*}}, checksumkind: CSK_MD5, checksum: "e13bca9b34ed822d596a519c9ce60995")

clang/test/Driver/cl-options.c

 // RUN:    /source-charset:utf-8 \
 // RUN:    /utf-8 \
 // RUN:    /vmg \
 // RUN:    /volatile:iso \
 // RUN:    /w12345 \
 // RUN:    /wd1234 \
 // RUN:    /Wv \
 // RUN:    /Wv:17 \
-// RUN:    /ZH:MD5 \
-// RUN:    /ZH:SHA1 \
-// RUN:    /ZH:SHA_256 \
 // RUN:    /Zm \
 // RUN:    /Zo \
 // RUN:    /Zo- \
 // RUN:    -### -- %s 2>&1 | FileCheck -check-prefix=IGNORED %s
 // IGNORED-NOT: argument unused during compilation
 // IGNORED-NOT: no such file or directory
 // Don't confuse /openmp- with the /o flag:
 // IGNORED-NOT: "-o" "penmp-.obj"

 // Args.hasArg to test whether a boolean flag is present without caring
 // where it appeared. And for this test, it appeared to the left of -gdwarf
 // which made it "win". This test could not detect that bug.
 // RUN: %clang_cl /Z7 -gdwarf /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7_gdwarf %s
 // Z7_gdwarf: "-gcodeview"
 // Z7_gdwarf: "-debug-info-kind=constructor"
 // Z7_gdwarf: "-dwarf-version=
 
+// RUN: %clang_cl /ZH:MD5 /c -### -- %s 2>&1 | FileCheck -check-prefix=ZH_MD5 %s
+// ZH_MD5: "-gsrc-hash=md5"
+
+// RUN: %clang_cl /ZH:SHA1 /c -### -- %s 2>&1 \
+// RUN:     | FileCheck -check-prefix=ZH_SHA1 %s
+// ZH_SHA1: "-gsrc-hash=sha1"
+
+// RUN: %clang_cl /ZH:SHA_256 /c -### -- %s 2>&1 \
+// RUN:     | FileCheck -check-prefix=ZH_SHA256 %s
+// ZH_SHA256: "-gsrc-hash=sha256"
+
 // RUN: %clang_cl -fmsc-version=1800 -TP -### -- %s 2>&1 | FileCheck -check-prefix=CXX11 %s
 // CXX11: -std=c++11
 
 // RUN: %clang_cl -fmsc-version=1900 -TP -### -- %s 2>&1 | FileCheck -check-prefix=CXX14 %s
 // CXX14: -std=c++14
 
 // RUN: %clang_cl -fmsc-version=1900 -TP -std:c++14 -### -- %s 2>&1 | FileCheck -check-prefix=STDCXX14 %s
 // STDCXX14: -std=c++14