[LLD] [MinGW] Allow requesting PDB output without giving a file name

[mstorsjo]

Summary

Opinions on naming options wanted!

When integrating PDB output in mingw targeting build systems, it might be a lot of extra work to specify unique file names for the pdb output. Therefore allow omitting the actual file name and let it implicitly be the same name as the linker output, with a pdb extension.

As the current form of the pdb option takes a separate parameter value, e.g. "-pdb out.pdb", it is impractical to leave out the parameter value.

Therefore, introduce a second syntax for the option, with an equals sign, like -pdb=out.pdb, where the value easily can be omitted.

The form -pdb= for requesting pdb files with an implicit name should work fine, even though it looks a bit unconventional in that form.

Alternatively, a flag with a different name could be introduced (e.g. --default-pdb), or we could consider changing the flag name altogether (having -pdb be the default with an implicit name, and "--out-pdb out.pdb" for the case when naming the output file, just like the existing --out-implib). The current number of users of the lld specific pdb option is probably low, so in case renaming the option altogether is deemed the best path forward, that could also be doable.

[ruiu]

I think I'm fine with the --pdb= option, although it looks a bit odd. Does this work both for --pdb= and --pdb ''? I think logically they should be identical.

[mstorsjo]

In D62004#1504666, @ruiu wrote:

I think I'm fine with the --pdb= option, although it looks a bit odd. Does this work both for --pdb= and --pdb ''? I think logically they should be identical.

Yes, the latter should also work, although I didn't make the testcase try it (in case there's issues with quoting in various environments).

[dmajor]

MinGW/Driver.cpp

168–170

Is the StringRef on the RHS necessary?

MinGW/Options.td

61

Should the new behavior be advertised in the HelpText or a doc somewhere?

[mstorsjo]

MinGW/Driver.cpp

168–170

Ah right, no it isn't.

MinGW/Options.td

61

Hmm, yes, ideally. I don't think there's much other general usage docs for the MinGW driver, so I guess it has to be squeezed into the HelpText here. I'd document it on my page for my MinGW distribution at least, for general usage of PDBs in a MinGW context (for the combination of passing -gcodeview to the compiler, and this option to the linker).

Maybe HelpText<"Specify output PDB debug information file. Defaults to the output filename, with a pdb suffix, if given an empty argument.">? Or can you come up with a more concise way of describing it?

[ruiu]

MinGW/Options.td

61

Other help messages don't end with a full stop, so I'd try to explain it in a single sentence rather than two sentences.

[mstorsjo]

MinGW/Options.td

61

What about Output PDB debug info file, chosen implicitly if the arguent is empty? That's pretty short, no full stops, and still conveys the main point.

[ruiu]

LGTM

MinGW/Options.td

61

Sounds good!

MinGW/Driver.cpp

if (auto *A = Args.getLastArg(OPT_o))
     Add("-out:" + StringRef(A->getValue()));
   else if (Args.hasArg(OPT_shared))
     Add("-out:a.dll");
   else
     Add("-out:a.exe");
 
   if (auto *A = Args.getLastArg(OPT_pdb)) {
     Add("-debug");
-    Add("-pdb:" + StringRef(A->getValue()));
+    StringRef V = A->getValue();
+    if (!V.empty())
+      Add("-pdb:" + V);
   } else if (Args.hasArg(OPT_strip_debug)) {
     Add("-debug:symtab");
   } else if (!Args.hasArg(OPT_strip_all)) {
     Add("-debug:dwarf");
   }
 
   if (Args.hasArg(OPT_shared))
     Add("-dll");

MinGW/Options.td

def require_defined: S<"require-defined">,
     HelpText<"Force symbol to be added to symbol table as an undefined one">;
 def require_defined_eq: J<"require-defined=">, Alias<require_defined>;
 
 // LLD specific options
 def _HASH_HASH_HASH : Flag<["-"], "###">,
     HelpText<"Print (but do not run) the commands to run for this compilation">;
 def appcontainer: F<"appcontainer">, HelpText<"Set the appcontainer flag in the executable">;
 def mllvm: S<"mllvm">;
-def pdb: S<"pdb">, HelpText<"Specify output PDB debug information file">;
+def pdb: S<"pdb">, HelpText<"Specify output PDB debug information file. "
+                            "Defaults to the output filename, with a pdb suffix, if given an empty argument">;
+def pdb_eq: J<"pdb=">, Alias<pdb>;
 def Xlink : J<"Xlink=">, MetaVarName<"<arg>">,
     HelpText<"Pass <arg> to the COFF linker">;
 
 // Alias
 def alias_entry_e: JoinedOrSeparate<["-"], "e">, Alias<entry>;
 def alias_strip_s: Flag<["-"], "s">, Alias<strip_all>;
 def alias_strip_S: Flag<["-"], "S">, Alias<strip_debug>;
 

test/MinGW/driver.test

 STRIP-NOT: -debug:dwarf
 
 RUN: ld.lld -### -m i386pep foo.o -S | FileCheck -check-prefix STRIP-DEBUG %s
 RUN: ld.lld -### -m i386pep foo.o --strip-debug | FileCheck -check-prefix STRIP-DEBUG %s
 STRIP-DEBUG: -debug:symtab
 STRIP-DEBUG-NOT: -debug:dwarf
 
 RUN: ld.lld -### -m i386pep foo.o -pdb out.pdb | FileCheck -check-prefix PDB %s
+RUN: ld.lld -### -m i386pep foo.o -pdb=out.pdb | FileCheck -check-prefix PDB %s
 PDB: -debug -pdb:out.pdb
 PDB-NOT: -debug:dwarf
 
+RUN: ld.lld -### -m i386pep foo.o -pdb= | FileCheck -check-prefix PDB-DEFAULT %s
+PDB-DEFAULT: -debug
+PDB-DEFAULT-NOT: -pdb:{{.*}}
+
 RUN: ld.lld -### -m i386pep foo.o --large-address-aware | FileCheck -check-prefix LARGE-ADDRESS-AWARE %s
 LARGE-ADDRESS-AWARE: -largeaddressaware
 
 RUN: ld.lld -### -m i386pe foo.o | FileCheck -check-prefix DEFAULT-DISABLE-FLAGS %s
 RUN: ld.lld -### -m i386pep foo.o | FileCheck -check-prefix DEFAULT-DISABLE-FLAGS %s
 DEFAULT-DISABLE-FLAGS: -dynamicbase:no
 RUN: ld.lld -### -m i386pe --dynamicbase foo.o | FileCheck -check-prefix NO-DEFAULT-DISABLE-FLAGS %s
 RUN: ld.lld -### -m i386pep -dynamicbase foo.o | FileCheck -check-prefix NO-DEFAULT-DISABLE-FLAGS %s