Don't output a PDB unless /DEBUG is specified

[zturner]

Summary

Without this, someone can write lld-link.exe /OUT:foo.exe /OUT:foo.pdb foo.obj and it will generate a PDB. But it's not a very useful PDB, because .debug$S and .debug$T information is not put into the PDB unless /DEBUG is also specified from the command line. So, if /DEBUG is not specified, don't output anything.

[ruiu]

Is this behavior the same as the MSVC linker? If it doesn't make sense to pass the /pdb option without /debug, we might want to emit an error (or warning) in the driver.

[zturner]

Yes, MSVC ignores /pdb option if you don't pass /DEBUG.

D:\src\llvmbuild\pdbtest>cl /c /Z7 hello.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.10.25019 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

hello.cpp
C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\VC\Tools\MSVC\14.10.25017\include\xlocale(314): warning C4530: C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc
C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\VC\Tools\MSVC\14.10.25017\include\exception(366): warning C4577: 'noexcept' used with no exception handling mode specified; termination on exception is not guaranteed. Specify /EHsc

D:\src\llvmbuild\pdbtest>link /out:hello.exe /pdb:hello.pdb hello.obj
Microsoft (R) Incremental Linker Version 14.10.25019.0
Copyright (C) Microsoft Corporation.  All rights reserved.


D:\src\llvmbuild\pdbtest>dir hello*
 Volume in drive D is New Volume
 Volume Serial Number is 2EC5-6F01

 Directory of D:\src\llvmbuild\pdbtest

05/17/2017  04:05 PM                92 hello.cpp
05/17/2017  04:06 PM           183,296 hello.exe
05/17/2017  04:05 PM           334,693 hello.obj
               3 File(s)        518,081 bytes
               0 Dir(s)  129,495,351,296 bytes free

D:\src\llvmbuild\pdbtest>link /debug /out:hello.exe /pdb:hello.pdb hello.obj
Microsoft (R) Incremental Linker Version 14.10.25019.0
Copyright (C) Microsoft Corporation.  All rights reserved.


D:\src\llvmbuild\pdbtest>dir hello*
 Volume in drive D is New Volume
 Volume Serial Number is 2EC5-6F01

 Directory of D:\src\llvmbuild\pdbtest

05/17/2017  04:05 PM                92 hello.cpp
05/17/2017  04:06 PM           756,224 hello.exe
05/17/2017  04:06 PM         3,284,272 hello.ilk
05/17/2017  04:05 PM           334,693 hello.obj
05/17/2017  04:06 PM         8,048,640 hello.pdb
               5 File(s)     12,423,921 bytes
               0 Dir(s)  129,483,444,224 bytes free

[ruiu]

Thank you for verifying! LGTM.

[zturner]

Removed the /DEBUGPDB option since it is unreferenced in the code. If the intent is to force writing a PDB, then all you have to do is specify /DEBUG, and you are guaranteed to get a PDB.

Also added some tests confirming the interactions between /DEBUG and /PDB.

BTW, this page indicates that it's not just an accident that MSVC linker behaves this way.

Use /PDB:filename to specify the name of the PDB file. If /DEBUG is not specified, the /PDB option is ignored.

[ruiu]

LGTM

lld/test/COFF/pdb-options.test

21 ↗

(On Diff #99377)

Please fix.

lld/trunk/COFF/Config.h

struct Configuration {
   bool NxCompat = true;
   bool AllowIsolation = true;
   bool TerminalServerAware = true;
   bool LargeAddressAware = false;
   bool HighEntropyVA = false;
   bool AppContainer = false;
 
   // This is for debugging.
-  bool DebugPdb = false;
   bool DumpPdb = false;
 };
 
 extern Configuration *Config;
 
 } // namespace coff
 } // namespace lld
 
 #endif

lld/trunk/COFF/Driver.cpp

if (Args.hasArg(OPT_dynamicbase_no))
     Config->DynamicBase = false;
   if (Args.hasArg(OPT_nxcompat_no))
     Config->NxCompat = false;
   if (Args.hasArg(OPT_tsaware_no))
     Config->TerminalServerAware = false;
   if (Args.hasArg(OPT_nosymtab))
     Config->WriteSymtab = false;
   Config->DumpPdb = Args.hasArg(OPT_dumppdb);
-  Config->DebugPdb = Args.hasArg(OPT_debugpdb);
 
   Config->MapFile = getMapFile(Args);
 
   if (ErrorCount)
     return;
 
   // Create a list of input files. Files can be given as arguments
   // for /defaultlib option.

lld/trunk/COFF/Options.td

 def help_q : Flag<["/?", "-?"], "">, Alias<help>;
 
 // LLD extensions
 def nopdb : F<"nopdb">, HelpText<"Disable PDB generation for DWARF users">;
 def nosymtab : F<"nosymtab">;
 def msvclto : F<"msvclto">;
 
 // Flags for debugging
-def debugpdb : F<"debugpdb">;
 def dumppdb : Joined<["/", "-"], "dumppdb">;
 def lldmap : F<"lldmap">;
 def lldmap_file : Joined<["/", "-"], "lldmap:">;
 
 //==============================================================================
 // The flags below do nothing. They are defined only for link.exe compatibility.
 //==============================================================================
 

lld/trunk/COFF/Writer.cpp

void Writer::run() {
   } else {
     writeHeader<pe32_header>();
   }
   fixSafeSEHSymbols();
   writeSections();
   sortExceptionTable();
   writeBuildId();
 
-  if (!Config->PDBPath.empty()) {
+  if (!Config->PDBPath.empty() && Config->Debug) {
     const llvm::codeview::DebugInfo *DI = nullptr;
     if (Config->DebugTypes & static_cast<unsigned>(coff::DebugType::CV))
       DI = BuildId->DI;
     createPDB(Config->PDBPath, Symtab, SectionTable, DI);
   }
 
   writeMapFile(OutputSections);
 

lld/trunk/test/COFF/invalid-debug-type.test

 # RUN: yaml2obj < %p/Inputs/pdb1.yaml > %t1.obj
 # RUN: yaml2obj < %p/Inputs/pdb2.yaml > %t2.obj
 # RUN: lld-link /debug /debugtype:invalid /pdb:%t.pdb /dll /out:%t.dll /entry:main /nodefaultlib \
-# RUN:   /debugpdb %t1.obj %t2.obj
+# RUN:   %t1.obj %t2.obj
 

lld/trunk/test/COFF/pdb-none.test

 # RUN: yaml2obj < %p/Inputs/pdb1.yaml > %t1.obj
 # RUN: yaml2obj < %p/Inputs/pdb2.yaml > %t2.obj
 # RUN: lld-link /debug /debugtype:pdata /pdb:%t.pdb /dll /out:%t.dll /entry:main /nodefaultlib \
-# RUN:   /debugpdb %t1.obj %t2.obj
+# RUN:   %t1.obj %t2.obj
 
 # RUN: llvm-pdbdump pdb2yaml -pdb-stream %t.pdb | FileCheck %s
 
 # CHECK:      PdbStream:
 # CHECK-NEXT:   Age:             0
 # CHECK-NEXT:   Guid:            '{00000000-0000-0000-0000-000000000000}'
 # CHECK-NEXT:   Signature:       0
 # CHECK-NEXT:   Version:         VC70
 

lld/trunk/test/COFF/pdb-options.test

+# RUN: yaml2obj < %p/Inputs/pdb1.yaml > %t1.obj
+# RUN: yaml2obj < %p/Inputs/pdb2.yaml > %t2.obj
+
+; If /DEBUG is not specified, /pdb is ignored.
+# RUN: lld-link /pdb:%t.pdb /entry:main /nodefaultlib %t1.obj %t2.obj
+# RUN: not ls %t.pdb
+
+; If /DEBUG and /pdb are specified, it uses the specified name.
+# RUN: lld-link /DEBUG /pdb:%t.pdb /entry:main /nodefaultlib %t1.obj %t2.obj
+# RUN: ls %t.pdb
+# RUN: rm %t.pdb
+
+; If /DEBUG is specified but not /pdb, it uses a default name in the current
+; directory.  This is a bit hacky since but we need to be IN our test specific
+; temporary directory when we run this command or we can't test this
+# RUN: cd %T
+# RUN: lld-link /DEBUG /entry:main /nodefaultlib %t1.obj %t2.obj
+# RUN: ls %t1.pdb
+# RUN: rm %t*
+# RUN: cd %T/..

lld/trunk/test/COFF/pdb.test

 # RUN: yaml2obj < %p/Inputs/pdb1.yaml > %t1.obj
 # RUN: yaml2obj < %p/Inputs/pdb2.yaml > %t2.obj
 # RUN: lld-link /debug /pdb:%t.pdb /dll /out:%t.dll /entry:main /nodefaultlib \
-# RUN:   /debugpdb %t1.obj %t2.obj
+# RUN:   %t1.obj %t2.obj
 
 # RUN: llvm-pdbdump pdb2yaml -stream-metadata -stream-directory -pdb-stream \
 # RUN:   -dbi-stream -ipi-stream -tpi-stream %t.pdb | FileCheck %s
 
 # RUN: llvm-pdbdump raw -modules -section-map -section-headers -section-contribs \
 # RUN:   -tpi-records %t.pdb | FileCheck -check-prefix RAW %s
 
 # CHECK:      MSF: