From f58330cbe44598eb2de0cca3b812f67fea0a71ca Mon Sep 17 00:00:00 2001 From: Justin Bogner Date: Fri, 11 Aug 2023 15:44:06 -0700 Subject: [PATCH] [Driver] Make /Zi and /Z7 aliases of -g rather than handling them specially The -g flag has been selecting whether to emit dwarf or codeview based on the target ABI since 2018, so simply aliasing these flags does the right thing for clang-cl. This moves some code from Clang::ConstructJob to renderDebugOptions to make things a little clearer now that we don't need to keep track of whether we're doing codeview or not in multiple places, and also combines the duplicate handling of the cl vs clang handling of jmc flags as a result. This is mostly NFC, but some -cc1 flags may be rendered in a slightly different order because of the code that was moved around. Differential Revision: https://reviews.llvm.org/D157794 --- clang/include/clang/Driver/Options.td | 8 +-- clang/lib/Driver/ToolChains/Clang.cpp | 97 ++++++++++----------------- clang/lib/Driver/ToolChains/Clang.h | 4 +- clang/test/Driver/cl-options.c | 12 +--- clang/test/Driver/working-directory.c | 2 +- 5 files changed, 45 insertions(+), 78 deletions(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 8af5a110605846..86d2bafa6b612e 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -7182,7 +7182,7 @@ 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">, +def _SLASH_Z7 : CLFlag<"Z7">, Alias, 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)">, @@ -7193,7 +7193,7 @@ def _SLASH_ZH_SHA1 : CLFlag<"ZH:SHA1">, def _SLASH_ZH_SHA_256 : CLFlag<"ZH:SHA_256">, HelpText<"Use SHA256 for file checksums in debug info">, Alias, AliasArgs<["sha256"]>; -def _SLASH_Zi : CLFlag<"Zi">, Alias<_SLASH_Z7>, +def _SLASH_Zi : CLFlag<"Zi">, Alias, HelpText<"Like /Z7">; def _SLASH_Zp : CLJoined<"Zp">, HelpText<"Set default maximum struct packing alignment">, @@ -7261,9 +7261,9 @@ def _SLASH_GX_ : CLFlag<"GX-">, def _SLASH_imsvc : CLJoinedOrSeparate<"imsvc">, HelpText<"Add to system include search path, as if in %INCLUDE%">, MetaVarName<"">; -def _SLASH_JMC : CLFlag<"JMC">, +def _SLASH_JMC : CLFlag<"JMC">, Alias, HelpText<"Enable just-my-code debugging">; -def _SLASH_JMC_ : CLFlag<"JMC-">, +def _SLASH_JMC_ : CLFlag<"JMC-">, Alias, HelpText<"Disable just-my-code debugging (default)">; def _SLASH_LD : CLFlag<"LD">, HelpText<"Create DLL">; def _SLASH_LDd : CLFlag<"LDd">, HelpText<"Create debug DLL">; diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 00f2d7140db5d5..be3c92162f5bf0 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -4204,8 +4204,8 @@ static void renderDwarfFormat(const Driver &D, const llvm::Triple &T, static void renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T, - const ArgList &Args, bool EmitCodeView, bool IRInput, - ArgStringList &CmdArgs, + const ArgList &Args, bool IRInput, ArgStringList &CmdArgs, + const InputInfo &Output, llvm::codegenoptions::DebugInfoKind &DebugInfoKind, DwarfFissionKind &DwarfFission) { if (Args.hasFlag(options::OPT_fdebug_info_for_profiling, @@ -4282,6 +4282,7 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T, if (const Arg *A = getDwarfNArg(Args)) EmitDwarf = checkDebugInfoOption(A, Args, D, TC); + bool EmitCodeView = false; if (const Arg *A = Args.getLastArg(options::OPT_gcodeview)) EmitCodeView = checkDebugInfoOption(A, Args, D, TC); @@ -4518,6 +4519,33 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T, renderDwarfFormat(D, T, Args, CmdArgs, EffectiveDWARFVersion); RenderDebugInfoCompressionArgs(Args, CmdArgs, D, TC); + + // This controls whether or not we perform JustMyCode instrumentation. + if (Args.hasFlag(options::OPT_fjmc, options::OPT_fno_jmc, false)) { + if (TC.getTriple().isOSBinFormatELF() || D.IsCLMode()) { + if (DebugInfoKind >= llvm::codegenoptions::DebugInfoConstructor) + CmdArgs.push_back("-fjmc"); + else if (D.IsCLMode()) + D.Diag(clang::diag::warn_drv_jmc_requires_debuginfo) << "/JMC" + << "'/Zi', '/Z7'"; + else + D.Diag(clang::diag::warn_drv_jmc_requires_debuginfo) << "-fjmc" + << "-g"; + } else { + D.Diag(clang::diag::warn_drv_fjmc_for_elf_only); + } + } + + // Add in -fdebug-compilation-dir if necessary. + const char *DebugCompilationDir = + addDebugCompDirArg(Args, CmdArgs, D.getVFS()); + + addDebugPrefixMapArg(D, TC, Args, CmdArgs); + + // Add the output path to the object file for CodeView debug infos. + if (EmitCodeView && Output.isFilename()) + addDebugObjectName(Args, CmdArgs, DebugCompilationDir, + Output.getFilename()); } static void ProcessVSRuntimeLibrary(const ArgList &Args, @@ -5697,33 +5725,16 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, RenderTargetOptions(Triple, Args, KernelOrKext, CmdArgs); - // These two are potentially updated by AddClangCLArgs. - llvm::codegenoptions::DebugInfoKind DebugInfoKind = - llvm::codegenoptions::NoDebugInfo; - bool EmitCodeView = false; - // Add clang-cl arguments. types::ID InputType = Input.getType(); if (D.IsCLMode()) - AddClangCLArgs(Args, InputType, CmdArgs, &DebugInfoKind, &EmitCodeView); + AddClangCLArgs(Args, InputType, CmdArgs); + llvm::codegenoptions::DebugInfoKind DebugInfoKind = + llvm::codegenoptions::NoDebugInfo; DwarfFissionKind DwarfFission = DwarfFissionKind::None; - renderDebugOptions(TC, D, RawTriple, Args, EmitCodeView, - types::isLLVMIR(InputType), CmdArgs, DebugInfoKind, - DwarfFission); - - // This controls whether or not we perform JustMyCode instrumentation. - if (Args.hasFlag(options::OPT_fjmc, options::OPT_fno_jmc, false)) { - if (TC.getTriple().isOSBinFormatELF()) { - if (DebugInfoKind >= llvm::codegenoptions::DebugInfoConstructor) - CmdArgs.push_back("-fjmc"); - else - D.Diag(clang::diag::warn_drv_jmc_requires_debuginfo) << "-fjmc" - << "-g"; - } else { - D.Diag(clang::diag::warn_drv_fjmc_for_elf_only); - } - } + renderDebugOptions(TC, D, RawTriple, Args, types::isLLVMIR(InputType), + CmdArgs, Output, DebugInfoKind, DwarfFission); // Add the split debug info name to the command lines here so we // can propagate it to the backend. @@ -6076,12 +6087,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, if (!ShouldEnableAutolink(Args, TC, JA)) CmdArgs.push_back("-fno-autolink"); - // Add in -fdebug-compilation-dir if necessary. - const char *DebugCompilationDir = - addDebugCompDirArg(Args, CmdArgs, D.getVFS()); - - addDebugPrefixMapArg(D, TC, Args, CmdArgs); - Args.AddLastArg(CmdArgs, options::OPT_ftemplate_depth_EQ); Args.AddLastArg(CmdArgs, options::OPT_foperator_arrow_depth_EQ); Args.AddLastArg(CmdArgs, options::OPT_fconstexpr_depth_EQ); @@ -7518,11 +7523,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, CmdArgs.push_back(Args.MakeArgString(Str)); } - // Add the output path to the object file for CodeView debug infos. - if (EmitCodeView && Output.isFilename()) - addDebugObjectName(Args, CmdArgs, DebugCompilationDir, - Output.getFilename()); - // Add the "-o out -x type src.c" flags last. This is done primarily to make // the -cc1 command easier to edit when reproducing compiler crashes. if (Output.getType() == types::TY_Dependencies) { @@ -7808,9 +7808,7 @@ static EHFlags parseClangCLEHFlags(const Driver &D, const ArgList &Args) { } void Clang::AddClangCLArgs(const ArgList &Args, types::ID InputType, - ArgStringList &CmdArgs, - llvm::codegenoptions::DebugInfoKind *DebugInfoKind, - bool *EmitCodeView) const { + ArgStringList &CmdArgs) const { bool isNVPTX = getToolChain().getTriple().isNVPTX(); ProcessVSRuntimeLibrary(Args, CmdArgs); @@ -7836,31 +7834,8 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID InputType, CmdArgs.push_back(Args.MakeArgString(Twine(LangOptions::SSPStrong))); } - // Emit CodeView if -Z7 or -gline-tables-only are present. - if (Arg *DebugInfoArg = Args.getLastArg(options::OPT__SLASH_Z7, - options::OPT_gline_tables_only)) { - *EmitCodeView = true; - if (DebugInfoArg->getOption().matches(options::OPT__SLASH_Z7)) - *DebugInfoKind = llvm::codegenoptions::DebugInfoConstructor; - else - *DebugInfoKind = llvm::codegenoptions::DebugLineTablesOnly; - } else { - *EmitCodeView = false; - } - const Driver &D = getToolChain().getDriver(); - // This controls whether or not we perform JustMyCode instrumentation. - if (Args.hasFlag(options::OPT__SLASH_JMC, options::OPT__SLASH_JMC_, - /*Default=*/false)) { - if (*EmitCodeView && - *DebugInfoKind >= llvm::codegenoptions::DebugInfoConstructor) - CmdArgs.push_back("-fjmc"); - else - D.Diag(clang::diag::warn_drv_jmc_requires_debuginfo) << "/JMC" - << "'/Zi', '/Z7'"; - } - EHFlags EH = parseClangCLEHFlags(D, Args); if (!isNVPTX && (EH.Synch || EH.Asynch)) { if (types::isCXX(InputType)) diff --git a/clang/lib/Driver/ToolChains/Clang.h b/clang/lib/Driver/ToolChains/Clang.h index 64fc86b6b0a780..0f503c4bd1c4fe 100644 --- a/clang/lib/Driver/ToolChains/Clang.h +++ b/clang/lib/Driver/ToolChains/Clang.h @@ -90,9 +90,7 @@ class LLVM_LIBRARY_VISIBILITY Clang : public Tool { RewriteKind rewrite) const; void AddClangCLArgs(const llvm::opt::ArgList &Args, types::ID InputType, - llvm::opt::ArgStringList &CmdArgs, - llvm::codegenoptions::DebugInfoKind *DebugInfoKind, - bool *EmitCodeView) const; + llvm::opt::ArgStringList &CmdArgs) const; mutable std::unique_ptr CompilationDatabase = nullptr; void DumpCompilationDatabase(Compilation &C, StringRef Filename, diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c index 11096ad55b59ae..6d929b19e7e2ef 100644 --- a/clang/test/Driver/cl-options.c +++ b/clang/test/Driver/cl-options.c @@ -564,16 +564,10 @@ // RUN: %clang_cl /Brepro /Brepro- /c '-###' -- %s 2>&1 | FileCheck -check-prefix=Brepro_ %s // Brepro_: "-mincremental-linker-compatible" -// This test was super sneaky: "/Z7" means "line-tables", but "-gdwarf" occurs -// later on the command line, so it should win. Interestingly the cc1 arguments -// came out right, but had wrong semantics, because an invariant assumed by -// CompilerInvocation was violated: it expects that at most one of {gdwarfN, -// line-tables-only} appear. If you assume that, then you can safely use -// 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. +// If We specify both /Z7 and -gdwarf we should get dwarf, not codeview. // RUN: %clang_cl /Z7 -gdwarf /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7_gdwarf %s -// Z7_gdwarf: "-gcodeview" +// RUN: %clang_cl -gdwarf /Z7 /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7_gdwarf %s +// Z7_gdwarf-NOT: "-gcodeview" // Z7_gdwarf: "-debug-info-kind=constructor" // Z7_gdwarf: "-dwarf-version= diff --git a/clang/test/Driver/working-directory.c b/clang/test/Driver/working-directory.c index 1642feed2ab0b3..d8fc56854cf290 100644 --- a/clang/test/Driver/working-directory.c +++ b/clang/test/Driver/working-directory.c @@ -6,6 +6,6 @@ // CHECK_NO_FILE: no such file or directory: 'no_such_file.cpp' +// CHECK_WORKS: "-fdebug-compilation-dir={{[^"]+}}test{{/|\\\\}}Driver{{/|\\\\}}Inputs" // CHECK_WORKS: "-coverage-notes-file" "{{[^"]+}}test{{/|\\\\}}Driver{{/|\\\\}}Inputs{{/|\\\\}}pchfile.gcno" // CHECK_WORKS: "-working-directory" "{{[^"]+}}test{{/|\\\\}}Driver{{/|\\\\}}Inputs" -// CHECK_WORKS: "-fdebug-compilation-dir={{[^"]+}}test{{/|\\\\}}Driver{{/|\\\\}}Inputs"