Make llvm-symbolizer work on Windows.

[zturner]

Summary

This is a rework of my original patch which introduced a new class hierarchy into llvm-symbolizer. Now that DIContext has moved to a common layer, this patch simply uses DIContext, resulting in much nicer code in llvm-symbolizer.

Inline frames are not yet supported, so we fall back to just pretending like the option wasn't specified whenever a PDBContext is in use. Test program:

Here is some program output showing that it works:

d:\testexe>cat test.c
  void foo() {
}

int main() {
  foo();
}

d:\testexe>dumpbin /DISASM test.exe | grep "_foo:\|_main:" --after-context=1
_foo:
  00401020: 55                 push        ebp
--
_main:
  00401030: 55                 push        ebp

d:\testexe>d:\src\llvmbuild\ninja\bin\llvm-symbolizer.exe --obj test.exe
0x401020
_foo
d:\testexe\test.c:1:0

0x401030
_main
d:\testexe\test.c:4:0

[samsonov]

include/llvm/DebugInfo/PDB/PDBContext.h

11

Header guard looks wrong.

31

Will these be implicitly deleted anyway?

lib/DebugInfo/PDB/PDBContext.cpp

38

Note that ImageBase may be uninitialized at this point.

61

Can LineInfo be nullptr at this point?

tools/llvm-symbolizer/LLVMSymbolize.cpp

169

The code has assumption that ModuleInfo::symbolizeInlinedCode() always returns *some* frames (at least one empty) - see the special case for InlinedContext.getNumberOfFrames() == 0. Hence, the assert in LLVMSymbolizer::symbolizeCode().

I think, that's a wrong place to add a fallback - instead you can make PDBContext::getInliningInfoForAddress() return a DIInliningInfo with a single frame, obtained from PDBContext::getLineInfoForAddress().

tools/llvm-symbolizer/llvm-symbolizer.cpp

131

Can you create RAII object that would call CoInitializeEx in ctor and CoUninitialize in dtor (and use it in applicable tools - for now llvm-symbolizer / llvm-dwarfdump)? It can have empty implementation on non-Windows platforms.

[timurrrr]

It's great to see how small the changes to the symbolizer code are!

I'll defer to Alexey for the review as I'm not at all familiar with the code.

include/llvm/DebugInfo/PDB/PDBContext.h

24

nit: s/pdb/PDB/

lib/DebugInfo/PDB/PDBContext.cpp

79

can this return nullptr?

tools/llvm-symbolizer/LLVMSymbolize.cpp

472

nit:
if (X *x = dyn_cast<X>(...)) { (1)
is not consistent with the other parts of the change where you use
if (auto x = dyn_cast<X>(...)) {

Personally I like the former (1) more.

[zturner]

I've addressed all of the comments except for the RAII com initialization comment. I think that can happen as a separate CL, since it will need to undergo review from others to make sure they're comfortable with the way we're introducing it (since COM is kind of a platform-specific thing, it has to be done carefully).

[samsonov]

Can you add tests for this functionality (proper handling of invalid addresses etc.)? For DWARF, we just check in small binaries with debug information (see test/DebugInfo/llvm-symbolizer.test).

include/llvm/DebugInfo/PDB/PDBContext.h

10

LLVM_DEBUGINFO_PDB_PDBCONTEXT_H ?

(yep, I've just noticed that header guards in include/llvm/DebugInfo/DWARF/* files are wrong, I will probably fix that.

lib/DebugInfo/PDB/PDBContext.cpp

44

What if Symbol is nullptr? Probably you should bail out early in this case.

57

You can reduce indentation with

if (Length == 0)
  return Result;

61

if LineInfo can't be nullptr, can we assert on this?

87

Looks like you can move copying the contents of LineInfo (from IPDBEnumLineNumbers) to DILineInfo into a helper function.

94

Should this be dyn_cast_or_null ?

[zturner]

Fix remaining issues and added a test.

[samsonov]

LGTM. There are a few minor comments below, feel free to land after addressing them. Thank you!

test/tools/llvm-symbolizer/pdb/Inputs/addresses.txt

1 ↗

(On Diff #24415)

Probably filename should reflect that these are addresses in test.exe

4 ↗

(On Diff #24415)

missing newline?

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

1 ↗

(On Diff #24415)

Could you provide comment with brief instruction on how to build test.exe / test.pdf from it?

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

9 ↗

(On Diff #24415)

missing newline

tools/llvm-symbolizer/llvm-symbolizer.cpp

20

config.h is probably not needed now?

include/llvm/DebugInfo/PDB/PDBContext.h

+//===-- PDBContext.h --------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===/
+
+#ifndef LLVM_LIB_DEBUGINFO_PDB_PDBCONTEXT_H
+#define LLVM_LIB_DEBUGINFO_PDB_PDBCONTEXT_H
+
+#include "llvm/DebugInfo/DIContext.h"
+#include "llvm/DebugInfo/PDB/IPDBSession.h"
+
+namespace llvm {
+
+namespace object {
+class COFFObjectFile;
+}
+
+/// PDBContext
+/// This data structure is the top level entity that deals with PDB debug
+/// information parsing.  This data structure exists only when there is a
+/// need for a transparent interface to different debug information formats
+/// (e.g. PDB and DWARF).  More control and power over the debug information
+/// access can be had by using the PDB interfaces directly.
+class PDBContext : public DIContext {
+
+  PDBContext(PDBContext &) = delete;
+  PDBContext &operator=(PDBContext &) = delete;
+
+public:
+  PDBContext(const object::COFFObjectFile &Object,
+             std::unique_ptr<IPDBSession> PDBSession);
+
+  static bool classof(const DIContext *DICtx) {
+    return DICtx->getKind() == CK_PDB;
+  }
+
+  void dump(raw_ostream &OS, DIDumpType DumpType = DIDT_All) override;
+
+  DILineInfo getLineInfoForAddress(
+      uint64_t Address,
+      DILineInfoSpecifier Specifier = DILineInfoSpecifier()) override;
+  DILineInfoTable getLineInfoForAddressRange(
+      uint64_t Address, uint64_t Size,
+      DILineInfoSpecifier Specifier = DILineInfoSpecifier()) override;
+  DIInliningInfo getInliningInfoForAddress(
+      uint64_t Address,
+      DILineInfoSpecifier Specifier = DILineInfoSpecifier()) override;
+
+private:
+  std::unique_ptr<IPDBSession> Session;
+};
+}
+
+#endif

lib/DebugInfo/PDB/CMakeLists.txt

 add_llvm_library(LLVMDebugInfoPDB
   IPDBSourceFile.cpp
   PDB.cpp
+  PDBContext.cpp
   PDBExtras.cpp
   PDBInterfaceAnchors.cpp
   PDBSymbol.cpp

lib/DebugInfo/PDB/PDBContext.cpp

+//===-- PDBContext.cpp ------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===/
+
+#include "llvm/DebugInfo/PDB/PDBContext.h"
+#include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
+#include "llvm/DebugInfo/PDB/IPDBLineNumber.h"
+#include "llvm/DebugInfo/PDB/IPDBSourceFile.h"
+#include "llvm/DebugInfo/PDB/PDBSymbol.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolFunc.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolData.h"
+#include "llvm/Object/COFF.h"
+
+using namespace llvm;
+using namespace llvm::object;
+
+PDBContext::PDBContext(const COFFObjectFile &Object,
+                       std::unique_ptr<IPDBSession> PDBSession)
+    : DIContext(CK_PDB), Session(std::move(PDBSession)) {
+  uint64_t ImageBase = 0;
+  if (Object.is64()) {
+    const pe32plus_header *Header = nullptr;
+    Object.getPE32PlusHeader(Header);
+    if (Header)
+      ImageBase = Header->ImageBase;
+  } else {
+    const pe32_header *Header = nullptr;
+    Object.getPE32Header(Header);
+    if (Header)
+      ImageBase = static_cast<uint64_t>(Header->ImageBase);
+  }
+  Session->setLoadAddress(ImageBase);
+}
+
+void PDBContext::dump(raw_ostream &OS, DIDumpType DumpType) {}
+
+DILineInfo PDBContext::getLineInfoForAddress(uint64_t Address,
+                                             DILineInfoSpecifier Specifier) {
+  auto Symbol = Session->findSymbolByAddress(Address);
+  uint32_t Length = 0;
+  DILineInfo Result;
+  if (auto Func = dyn_cast<PDBSymbolFunc>(Symbol.get())) {
+    if (Specifier.FNKind == DINameKind::LinkageName)
+      Result.FunctionName = Func->getUndecoratedName();
+    else if (Specifier.FNKind == DINameKind::ShortName)
+      Result.FunctionName = Func->getName();
+
+    Length = Func->getLength();
+  } else if (auto Data = dyn_cast<PDBSymbolData>(Symbol.get())) {
+    Length = Data->getLength();
+  }
+  if (Length > 0) {
+    auto LineNumbers = Session->findLineNumbersByAddress(Address, Length);
+    if (LineNumbers && LineNumbers->getChildCount() > 0) {
+      auto LineInfo = LineNumbers->getNext();
+      auto SourceFile = Session->getSourceFileById(LineInfo->getSourceFileId());
+
+      if (SourceFile &&
+          Specifier.FLIKind != DILineInfoSpecifier::FileLineInfoKind::None)
+        Result.FileName = SourceFile->getFileName();
+      Result.Column = LineInfo->getColumnNumber();
+      Result.Line = LineInfo->getLineNumber();
+    }
+  }
+  return Result;
+}
+
+DILineInfoTable
+PDBContext::getLineInfoForAddressRange(uint64_t Address, uint64_t Size,
+                                       DILineInfoSpecifier Specifier) {
+  if (Size == 0)
+    return DILineInfoTable();
+
+  DILineInfoTable Table;
+  auto LineNumbers = Session->findLineNumbersByAddress(Address, Size);
+  if (!LineNumbers)
+    return Table;
+
+  while (auto LineInfo = LineNumbers->getNext()) {
+    DILineInfo LineEntry;
+    auto SourceFile = Session->getSourceFileById(LineInfo->getSourceFileId());
+    LineEntry.Column = LineInfo->getColumnNumber();
+    LineEntry.Line = LineInfo->getLineNumber();
+    if (SourceFile &&
+        Specifier.FLIKind != DILineInfoSpecifier::FileLineInfoKind::None)
+      LineEntry.FileName = SourceFile->getFileName();
+    if (Specifier.FNKind != DINameKind::None) {
+      auto Symbol = Session->findSymbolByAddress(LineInfo->getVirtualAddress());
+      if (auto Func = dyn_cast<PDBSymbolFunc>(Symbol.get())) {
+        if (Specifier.FNKind == DINameKind::LinkageName)
+          LineEntry.FunctionName = Func->getUndecoratedName();
+        else
+          LineEntry.FunctionName = Func->getName();
+      }
+    }
+
+    Table.push_back(std::make_pair(LineInfo->getVirtualAddress(), LineEntry));
+  }
+  return Table;
+}
+
+DIInliningInfo
+PDBContext::getInliningInfoForAddress(uint64_t Address,
+                                      DILineInfoSpecifier Specifier) {
+  DIInliningInfo InlineInfo;
+  DILineInfo Frame = getLineInfoForAddress(Address, Specifier);
+  InlineInfo.addFrame(Frame);
+  return InlineInfo;
+}

tools/llvm-symbolizer/CMakeLists.txt

 
 set(LLVM_LINK_COMPONENTS
   DebugInfoDWARF
+  DebugInfoPDB
   Object
   Support
   )

tools/llvm-symbolizer/LLVMSymbolize.cpp

 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Config/config.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
+#include "llvm/DebugInfo/PDB/PDB.h"
+#include "llvm/DebugInfo/PDB/PDBContext.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Support/Casting.h"

 DIInliningInfo ModuleInfo::symbolizeInlinedCode(
     uint64_t ModuleOffset, const LLVMSymbolizer::Options &Opts) const {
   DIInliningInfo InlinedContext;
+
   if (DebugInfoContext) {
     InlinedContext = DebugInfoContext->getInliningInfoForAddress(
         ModuleOffset, getDILineInfoSpecifier(Opts));

     Modules.insert(make_pair(ModuleName, (ModuleInfo *)nullptr));
     return nullptr;
   }
-  DIContext *Context = new DWARFContextInMemory(*Objects.second);
+  DIContext *Context = nullptr;
+  if (auto CoffObject = dyn_cast<COFFObjectFile>(Objects.first)) {
+    // If this is a COFF object, assume it contains PDB debug information.  If
+    // we don't find any we will fall back to the DWARF case.
+    std::unique_ptr<IPDBSession> Session;
+    PDB_ErrorCode Error = loadDataForEXE(PDB_ReaderType::DIA,
+                                         Objects.first->getFileName(), Session);
+    if (Error == PDB_ErrorCode::Success)
+      Context = new PDBContext(*CoffObject, std::move(Session));
+  }
+  if (!Context)
+    Context = new DWARFContextInMemory(*Objects.second);
   assert(Context);
   ModuleInfo *Info = new ModuleInfo(Objects.first, Context);
   Modules.insert(make_pair(ModuleName, Info));

tools/llvm-symbolizer/llvm-symbolizer.cpp

 
 #include "LLVMSymbolize.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Config/config.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FileSystem.h"

 #include <cstring>
 #include <string>
 
+#if defined(_WIN32) && defined(HAVE_DIA_SDK)
+#include <Windows.h>
+#endif
+
 using namespace llvm;
 using namespace symbolize;
 

   PrettyStackTraceProgram X(argc, argv);
   llvm_shutdown_obj Y; // Call llvm_shutdown() on exit.
 
+#if defined(_WIN32) && defined(HAVE_DIA_SDK)
+  ::CoInitializeEx(nullptr, COINIT_MULTITHREADED);
+#endif
+
   cl::ParseCommandLineOptions(argc, argv, "llvm-symbolizer\n");
   LLVMSymbolizer::Options Opts(ClUseSymbolTable, ClPrintFunctions,
                                ClPrintInlining, ClDemangle, ClDefaultArch);

     outs() << Result << "\n";
     outs().flush();
   }
+
+#if defined(_WIN32) && defined(HAVE_DIA_SDK)
+  ::CoUninitialize();
+#endif
+
   return 0;
 }