Default to standalone debug info on Windows

[rnk]

Summary

PDB-based debuggers do not support looking up type names across type
stream boundaries. There are two ways where users end up being unable to
look into variables with types that were required to be complete:

1. The type was provided by another DLL, but was not marked dllimport. In this case, the type stream is in the PDB, and it will contain all types that had complete definitions within the DLL. If the type had a vtable that was only emitted in another DLL, we will not have type information for it.

2. The user is using /DEBUG:FASTLINK. This flag tells the linker to leave debug information in object files, similar to the default build flow on MacOS. When /DEBUG:FASTLINK is used with /Z7, each individual object file needs to have complete definitions of all types required to be complete in that TU. This isn't so bad for MSVC because it supports using type servers with /Zi, so they still get some amount of type deduplication.

I did an experiment some time ago on Chrome and found that changing the
default here increased the object file sizes in a Chrome debug build by
17%[1], which is quite significant. If those savings are important to
users and they aren't using /DEBUG:FASTLINK, they can opt into limited
debug info by passing -flimit-debug-info.

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=642812#c15

[thakis]

Did you measure what this does to link times?

[rnk]

In D27858#625367, @thakis wrote:

Did you measure what this does to link times?

No, but I plan to recover it in Chromium by doing something like:

if (is_win && is_clang && !is_win_fastlink) {
  cflags += ["-flimit-debug-info"]
}

[rnk]

In D27858#625367, @thakis wrote:

Did you measure what this does to link times?

I've measured now, and it's pretty bad. Times to link blink_core increase by 39% with -fstandalone-debug (211s -> 294s). I still think it's what we should do by default.

There's other things we can do with PCH to have less duplicate debug info when not using /debug:fastlink.

lib/Driver/MSVCToolChain.cpp

 bool MSVCToolChain::isPIEDefault() const {
   return false;
 }
 
 bool MSVCToolChain::isPICDefaultForced() const {
   return getArch() == llvm::Triple::x86_64;
 }
 
+bool MSVCToolChain::GetDefaultStandaloneDebug() const {
+  // Microsoft debuggers can't lookup type names across type stream boundaries.
+  // This means that types implemented in b.dll used by a.dll sometimes can't be
+  // found when debugging a.dll. Clang will emit types marked with dllimport to
+  // try to cope with this case, but is not a complete heuristic. Non-standalone
+  // type info is also incompatble with /DEBUG:FASTLINK, which leaves all debug
+  // info, including types, in object files. In this case, the type stream is
+  // limited to the TU, and every TU needs to have all the types required to be
+  // complete by that TU for debugging.
+  return true;
+}
+
 #ifdef USE_WIN32
 static bool readFullStringValue(HKEY hkey, const char *valueName,
                                 std::string &value) {
   std::wstring WideValueName;
   if (!llvm::ConvertUTF8toWide(valueName, WideValueName))
     return false;
 
   DWORD result = 0;

lib/Driver/ToolChains.h

public:
   TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch,
                 Action::OffloadKind DeviceOffloadKind) const override;
 
   bool IsIntegratedAssemblerDefault() const override;
   bool IsUnwindTablesDefault() const override;
   bool isPICDefault() const override;
   bool isPIEDefault() const override;
   bool isPICDefaultForced() const override;
+  bool GetDefaultStandaloneDebug() const override;
 
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
                             llvm::opt::ArgStringList &CC1Args) const override;
   void AddClangCXXStdlibIncludeArgs(
       const llvm::opt::ArgList &DriverArgs,
       llvm::opt::ArgStringList &CC1Args) const override;
 

test/Driver/cl-options.c

 // RUN: %clang_cl /Zc:threadSafeInit /Zc:threadSafeInit- /c -### -- %s 2>&1 | FileCheck -check-prefix=NoThreadSafeStatics %s
 // NoThreadSafeStatics: "-fno-threadsafe-statics"
 
 // RUN: %clang_cl /Zc:threadSafeInit /c -### -- %s 2>&1 | FileCheck -check-prefix=ThreadSafeStatics %s
 // ThreadSafeStatics-NOT: "-fno-threadsafe-statics"
 
 // RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
 // Zi: "-gcodeview"
-// Zi: "-debug-info-kind=limited"
+// Zi: "-debug-info-kind=standalone"
 
 // RUN: %clang_cl /Z7 /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7 %s
 // Z7: "-gcodeview"
-// Z7: "-debug-info-kind=limited"
+// Z7: "-debug-info-kind=standalone"
 
 // RUN: %clang_cl /Zd /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7GMLT %s
 // Z7GMLT: "-gcodeview"
 // Z7GMLT: "-debug-info-kind=line-tables-only"
 
 // RUN: %clang_cl -gline-tables-only /c -### -- %s 2>&1 | FileCheck -check-prefix=ZGMLT %s
 // ZGMLT: "-gcodeview"
 // ZGMLT: "-debug-info-kind=line-tables-only"
 
+// RUN: %clang_cl /Z7 -flimit-debug-info /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7LIMITED %s
+// Z7LIMITED: "-gcodeview"
+// Z7LIMITED: "-debug-info-kind=limited"
+
 // RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck -check-prefix=BreproDefault %s
 // BreproDefault: "-mincremental-linker-compatible"
 
 // RUN: %clang_cl /Brepro- /Brepro /c '-###' -- %s 2>&1 | FileCheck -check-prefix=Brepro %s
 // Brepro-NOT: "-mincremental-linker-compatible"
 
 // 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.
 // RUN: %clang_cl /Z7 -gdwarf /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7_gdwarf %s
 // Z7_gdwarf: "-gcodeview"
-// Z7_gdwarf: "-debug-info-kind=limited"
+// Z7_gdwarf: "-debug-info-kind=standalone"
 // Z7_gdwarf: "-dwarf-version=4"
 
 // 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