Skip to content

Commit

Permalink
[Driver] Make /Zi and /Z7 aliases of -g rather than handling them spe…
Browse files Browse the repository at this point in the history 
…cially

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
  • Loading branch information
@bogner
bogner committed Aug 14, 2023
1 parent c4ada13 commit f58330c
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 78 deletions.
8 changes: 4 additions & 4 deletions clang/include/clang/Driver/Options.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<g_Flag>,
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)">,
Expand All @@ -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<gsrc_hash_EQ>, AliasArgs<["sha256"]>;
def _SLASH_Zi : CLFlag<"Zi">, Alias<_SLASH_Z7>,
def _SLASH_Zi : CLFlag<"Zi">, Alias<g_Flag>,
HelpText<"Like /Z7">;
def _SLASH_Zp : CLJoined<"Zp">,
HelpText<"Set default maximum struct packing alignment">,
Expand Down Expand Up @@ -7261,9 +7261,9 @@ def _SLASH_GX_ : CLFlag<"GX-">,
def _SLASH_imsvc : CLJoinedOrSeparate<"imsvc">,
HelpText<"Add <dir> to system include search path, as if in %INCLUDE%">,
MetaVarName<"<dir>">;
def _SLASH_JMC : CLFlag<"JMC">,
def _SLASH_JMC : CLFlag<"JMC">, Alias<fjmc>,
HelpText<"Enable just-my-code debugging">;
def _SLASH_JMC_ : CLFlag<"JMC-">,
def _SLASH_JMC_ : CLFlag<"JMC-">, Alias<fno_jmc>,
HelpText<"Disable just-my-code debugging (default)">;
def _SLASH_LD : CLFlag<"LD">, HelpText<"Create DLL">;
def _SLASH_LDd : CLFlag<"LDd">, HelpText<"Create debug DLL">;
Expand Down
97 changes: 36 additions & 61 deletions clang/lib/Driver/ToolChains/Clang.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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))
Expand Down
4 changes: 1 addition & 3 deletions clang/lib/Driver/ToolChains/Clang.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<llvm::raw_fd_ostream> CompilationDatabase = nullptr;
void DumpCompilationDatabase(Compilation &C, StringRef Filename,
Expand Down
12 changes: 3 additions & 9 deletions clang/test/Driver/cl-options.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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=

Expand Down
2 changes: 1 addition & 1 deletion clang/test/Driver/working-directory.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

2 comments on commit f58330c

@mstorsjo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CC @MaskRay and @bogner

This change had a surprising, and seemingly unintended effect: If compiling an assembly (.s) file with clang-cl /Z7, the output object file now contains DWARF debug info, even if /Z7 is supposed to map to codeview. See #101710 (comment) for reference for a full case where this breaks a LLDB test.

Does anyone know right off the bat where to start looking into that, or should I open a specific directed issue about it?

@MaskRay
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CC @MaskRay and @bogner

This change had a surprising, and seemingly unintended effect: If compiling an assembly (.s) file with clang-cl /Z7, the output object file now contains DWARF debug info, even if /Z7 is supposed to map to codeview. See #101710 (comment) for reference for a full case where this breaks a LLDB test.

Does anyone know right off the bat where to start looking into that, or should I open a specific directed issue about it?

I'm starting a 3-week vacation today and will have limited availability. I don't know where to look at it off my head :)

Please sign in to comment.