[llvm-symbolizer] Switch to using native symbolizer by default on Windows

[akhuang]

Summary

llvm-symbolizer used to use the DIA SDK for symbolization on
Windows; this patch switches to using native symbolization, which was
implemented recently.

Users can still make the symbolizer use DIA by adding the -dia flag
in the LLVM_SYMBOLIZER_OPTS environment variable.

[amccarth]

I added two suggested edits for your consideration.

Other than those, this looks good.

(It seems there's some old Cmake cruft around LLVM_ENABLE_DIA_SDK that could probably be cleaned up in a separate patch.)

llvm/tools/llvm-symbolizer/Opts.td

44

I think this help string might be a smidge more useful to people who don't already know what DIA is.

llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp

291–298

--use-native-pdb-reader was always possible, but --dia isn't, so let's let the user know if we cannot satisfy their request.

[jhenderson]

llvm/tools/llvm-symbolizer/Opts.td

44

Could you please also add this option to the llvm-symbolizer Command Guide (llvm/docs/CommadnGuide/llvm-symbolizer.rst)? You might want to add a new subsection to the existing document for Windows/COFF/PDB specific options (like there is already for Mach-O).

[akhuang]

address comments

[akhuang]

fix typo

[amccarth]

LGTM

[jhenderson]

LGTM, with the suggestion.

llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp

294

Use WithColor::defaultWarningHandler instead and don't end the message with '.' (see the coding guidelines).

(I'm aware that llvm-symbolizer isn't consistent in this area, but we can work on improving it gradually)

[akhuang]

Use WithColor::warning() to print the warning

lld/test/COFF/symbolizer-inline.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj %s -o %t.obj -triple x86_64-windows-msvc
 # RUN: lld-link -entry:main -nodefaultlib %t.obj -out:%t.exe -pdb:%t.pdb -debug
-# RUN: llvm-symbolizer --obj=%t.exe --use-native-pdb-reader --relative-address \
+# RUN: llvm-symbolizer --obj=%t.exe --relative-address \
 # RUN:   0x1014 0x1018 0x101c 0x1023 0x1024 | FileCheck %s
 
 # Compiled from this cpp code, with modifications to add extra inline line and 
 # file changes:
 # clang -cc1 -triple x86_64-windows-msvc -gcodeview -S test.cpp
 #
 # __attribute__((always_inline)) int inlinee_2(int x) {
 #   return x + 1;

llvm/docs/CommandGuide/llvm-symbolizer.rst

 .. option:: --version, -v
 
   Print version information for the tool.
 
 .. option:: @<FILE>
 
   Read command-line options from response file `<FILE>`.
 
+WINDOWS/PDB SPECIFIC OPTIONS
+-----------------------------
+
+.. option:: --dia
+
+  Use the Windows DIA SDK for symbolization. If the DIA SDK is not found,
+  llvm-symbolizer will fall back to the native implementation.
+
 MACH-O SPECIFIC OPTIONS
 -----------------------
 
 .. option:: --default-arch <arch>
 
   If a binary contains object files for multiple architectures (e.g. it is a
   Mach-O universal binary), symbolize the object file for a given architecture.
   You can also specify the architecture by writing ``binary_name:arch_name`` in

llvm/include/llvm/DebugInfo/Symbolize/Symbolize.h

 public:
   struct Options {
     FunctionNameKind PrintFunctions = FunctionNameKind::LinkageName;
     FileLineInfoKind PathStyle = FileLineInfoKind::AbsoluteFilePath;
     bool UseSymbolTable = true;
     bool Demangle = true;
     bool RelativeAddresses = false;
     bool UntagAddresses = false;
-    bool UseNativePDBReader = false;
+    bool UseDIA = false;
     std::string DefaultArch;
     std::vector<std::string> DsymHints;
     std::string FallbackDebugPath;
     std::string DWPName;
     std::vector<std::string> DebugFileDirectory;
   };
 
   LLVMSymbolizer() = default;

llvm/lib/DebugInfo/Symbolize/Symbolize.cpp

LLVMSymbolizer::getOrCreateModuleInfo(const std::string &ModuleName) {
   if (auto CoffObject = dyn_cast<COFFObjectFile>(Objects.first)) {
     const codeview::DebugInfo *DebugInfo;
     StringRef PDBFileName;
     auto EC = CoffObject->getDebugPDBInfo(DebugInfo, PDBFileName);
     if (!EC && DebugInfo != nullptr && !PDBFileName.empty()) {
       using namespace pdb;
       std::unique_ptr<IPDBSession> Session;
 
-      PDB_ReaderType ReaderType = PDB_ReaderType::Native;
-#if LLVM_ENABLE_DIA_SDK
-      if (!Opts.UseNativePDBReader)
-        ReaderType = PDB_ReaderType::DIA;
-#endif
+      PDB_ReaderType ReaderType =
+          Opts.UseDIA ? PDB_ReaderType::DIA : PDB_ReaderType::Native;
       if (auto Err = loadDataForEXE(ReaderType, Objects.first->getFileName(),
                                     Session)) {
         Modules.emplace(ModuleName, std::unique_ptr<SymbolizableModule>());
         // Return along the PDB filename to provide more context
         return createFileError(PDBFileName, std::move(Err));
       }
       Context.reset(new PDBContext(*CoffObject, std::move(Session)));
     }

llvm/test/tools/llvm-symbolizer/pdb/pdb-columns.test

 RUN: echo 0x140006BA0 > %t.input
 RUN: echo 0x140006C00 >> %t.input
 RUN: echo 0x140006BB0 >> %t.input
 RUN: echo 0x140006C10 >> %t.input
 RUN: echo 0x140006C20 >> %t.input
 RUN: echo 0x140006C30 >> %t.input
 RUN: echo 0x140006C40 >> %t.input
 RUN: echo 0x140006C70 >> %t.input
 RUN:    llvm-symbolizer -obj="%p/Inputs/test-columns.exe" < %t.input \
 RUN:    | FileCheck %s
-RUN:    llvm-symbolizer -obj="%p/Inputs/test-columns.exe" -use-native-pdb-reader < %t.input \
-RUN:    | FileCheck %s
 
 This tests that the symbolizer outputs column info when it is present in the pdb.
 
 CHECK: foo(void)
 CHECK-NEXT: test.cpp:11:1
 CHECK: {{^private_symbol$}}
 CHECK-NEXT: test.cpp:14:1
 CHECK: {{^main}}

llvm/test/tools/llvm-symbolizer/pdb/pdb.test

 RUN: echo 0x401380 > %t.input
 RUN: echo 0x401390 >> %t.input
 RUN: echo 0x4013A0 >> %t.input
 RUN: echo 0x4013C0 >> %t.input
 RUN: echo 0x4013D0 >> %t.input
 RUN: echo 0x4013E0 >> %t.input
 RUN: echo 0x4013F0 >> %t.input
 RUN: echo 0x401420 >> %t.input
-RUN: llvm-symbolizer -obj="%p/Inputs/test.exe" < %t.input \
+RUN: llvm-symbolizer --obj="%p/Inputs/test.exe" < %t.input \
 RUN:    | FileCheck %s
 RUN: llvm-symbolizer --obj="%p/Inputs/test.exe" --no-demangle < %t.input \
 RUN:    | FileCheck %s --check-prefix=CHECK-NO-DEMANGLE
 
-Test with native pdb reader.
-RUN: llvm-symbolizer -use-native-pdb-reader -obj="%p/Inputs/test.exe" < %t.input \
+; Check that -dia works
+RUN: llvm-symbolizer --dia --obj="%p/Inputs/test.exe" < %t.input \
 RUN:    | FileCheck %s
-RUN: llvm-symbolizer --use-native-pdb-reader --obj="%p/Inputs/test.exe" --no-demangle < %t.input \
+RUN: llvm-symbolizer --dia --obj="%p/Inputs/test.exe" --no-demangle < %t.input \
 RUN:    | FileCheck %s --check-prefix=CHECK-NO-DEMANGLE
 
+
 Subtract ImageBase from all the offsets and run the test again with
 --relative-address.
 
 RUN: %python -c 'import sys;print("\n".join([hex(int(x, 16) - 0x400000) for x in sys.stdin]))' < %t.input \
 RUN:    | llvm-symbolizer --obj="%p/Inputs/test.exe" --no-demangle --relative-address \
 RUN:    | FileCheck %s --check-prefix=CHECK-NO-DEMANGLE
 
-RUN: %python -c 'import sys;print("\n".join([hex(int(x, 16) - 0x400000) for x in sys.stdin]))' < %t.input \
-RUN:    | llvm-symbolizer --use-native-pdb-reader --obj="%p/Inputs/test.exe" --no-demangle --relative-address \
-RUN:    | FileCheck %s --check-prefix=CHECK-NO-DEMANGLE
-
 CHECK: foo(void)
 CHECK-NEXT: test.cpp:10
 CHECK: {{^private_symbol$}}
 CHECK-NEXT: test.cpp:13:0
 CHECK: {{^main}}
 CHECK-NEXT: test.cpp:16:0
 CHECK: {{^foo_cdecl$}}
 CHECK: {{^foo_stdcall$}}

llvm/tools/llvm-symbolizer/Opts.td

defm output_style
     : Eq<"output-style", "Specify print style. Supported styles: LLVM, GNU">,
       MetaVarName<"style">,
       Values<"LLVM,GNU">;
 def pretty_print : F<"pretty-print", "Make the output more human friendly">;
 defm print_source_context_lines : Eq<"print-source-context-lines", "Print N lines of source file context">;
 def relative_address : F<"relative-address", "Interpret addresses as addresses relative to the image base">;
 def relativenames : F<"relativenames", "Strip the compilation directory from paths">;
 defm untag_addresses : B<"untag-addresses", "", "Remove memory tags from addresses before symbolization">;
-def use_native_pdb_reader : F<"use-native-pdb-reader", "Use native PDB functionality">;
+def use_dia: F<"dia", "Use the DIA library to access symbols (Windows only)">;
 def verbose : F<"verbose", "Print verbose line info">;
 def version : F<"version", "Display the version">;
 
 def : Flag<["-"], "a">, Alias<addresses>, HelpText<"Alias for --addresses">;
 def : F<"print-address", "Alias for --addresses">, Alias<addresses>;
 def : Flag<["-"], "C">, Alias<demangle>, HelpText<"Alias for --demangle">;
 def : Joined<["--"], "exe=">, Alias<obj_EQ>, HelpText<"Alias for --obj">, MetaVarName<"<file>">;
 def : Separate<["--"], "exe">, Alias<obj_EQ>, HelpText<"Alias for --obj">, MetaVarName<"<file>">;

llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp

int main(int argc, char **argv) {
   Opts.DWPName = Args.getLastArgValue(OPT_dwp_EQ).str();
   Opts.FallbackDebugPath =
       Args.getLastArgValue(OPT_fallback_debug_path_EQ).str();
   Opts.PrintFunctions = decideHowToPrintFunctions(Args, IsAddr2Line);
   parseIntArg(Args, OPT_print_source_context_lines_EQ, SourceContextLines);
   Opts.RelativeAddresses = Args.hasArg(OPT_relative_address);
   Opts.UntagAddresses =
       Args.hasFlag(OPT_untag_addresses, OPT_no_untag_addresses, !IsAddr2Line);
-  Opts.UseNativePDBReader = Args.hasArg(OPT_use_native_pdb_reader);
+  Opts.UseDIA = Args.hasArg(OPT_use_dia);
+#if !defined(LLVM_ENABLE_DIA_SDK)
+  if (Opts.UseDIA) {
+    errs() << "Warning: DIA not available; using native PDB reader"
+                  .\n ";
+              Opts.UseDIA = false;
+  }
+#endif
   Opts.UseSymbolTable = true;
 
   for (const opt::Arg *A : Args.filtered(OPT_dsym_hint_EQ)) {
     StringRef Hint(A->getValue());
     if (sys::path::extension(Hint) == ".dSYM") {
       Opts.DsymHints.emplace_back(Hint);
     } else {
       errs() << "Warning: invalid dSYM hint: \"" << Hint