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

10 ↗

(On Diff #24335)

Header guard looks wrong.

30 ↗

(On Diff #24335)

Will these be implicitly deleted anyway?

lib/DebugInfo/PDB/PDBContext.cpp

37 ↗

(On Diff #24335)

Note that ImageBase may be uninitialized at this point.

60 ↗

(On Diff #24335)

Can LineInfo be nullptr at this point?

tools/llvm-symbolizer/LLVMSymbolize.cpp

169 ↗

(On Diff #24335)

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 ↗

(On Diff #24335)

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

23 ↗

(On Diff #24335)

nit: s/pdb/PDB/

lib/DebugInfo/PDB/PDBContext.cpp

78 ↗

(On Diff #24335)

can this return nullptr?

tools/llvm-symbolizer/LLVMSymbolize.cpp

472 ↗

(On Diff #24335)

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 ↗

(On Diff #24406)

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 ↗

(On Diff #24406)

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

57 ↗

(On Diff #24406)

You can reduce indentation with

if (Length == 0)
  return Result;

61 ↗

(On Diff #24406)

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

87 ↗

(On Diff #24406)

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

94 ↗

(On Diff #24406)

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 ↗

(On Diff #24415)

config.h is probably not needed now?

llvm/trunk/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_DEBUGINFO_PDB_PDBCONTEXT_H
+#define LLVM_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

llvm/trunk/lib/DebugInfo/PDB/CMakeLists.txt

 
 endif()
 
 list(APPEND LIBPDB_ADDITIONAL_HEADER_DIRS "${LLVM_MAIN_INCLUDE_DIR}/llvm/DebugInfo/PDB")
 
 add_llvm_library(LLVMDebugInfoPDB
   IPDBSourceFile.cpp
   PDB.cpp
+  PDBContext.cpp
   PDBExtras.cpp
   PDBInterfaceAnchors.cpp
   PDBSymbol.cpp
   PDBSymbolAnnotation.cpp
   PDBSymbolBlock.cpp
   PDBSymbolCompiland.cpp
   PDBSymbolCompilandDetails.cpp
   PDBSymbolCompilandEnv.cpp

llvm/trunk/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 = 1;
+  DILineInfo Result;
+  if (auto Func = dyn_cast_or_null<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_or_null<PDBSymbolData>(Symbol.get())) {
+    Length = Data->getLength();
+  }
+
+  // If we couldn't find a symbol, then just assume 1 byte, so that we get
+  // only the line number of the first instruction.
+  auto LineNumbers = Session->findLineNumbersByAddress(Address, Length);
+  if (!LineNumbers || LineNumbers->getChildCount() == 0)
+    return Result;
+
+  auto LineInfo = LineNumbers->getNext();
+  assert(LineInfo);
+  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 || LineNumbers->getChildCount() == 0)
+    return Table;
+
+  while (auto LineInfo = LineNumbers->getNext()) {
+    DILineInfo LineEntry =
+        getLineInfoForAddress(LineInfo->getVirtualAddress(), Specifier);
+    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;
+}

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

+// To generate the corresponding EXE/PDB, run:
+// cl /Zi test.c
+void foo() {
+}
+
+int main() {
+  foo();
+}

llvm/trunk/test/tools/llvm-symbolizer/pdb/Inputs/test.exe.input

+0x401020
+0x401030
+0x500000

llvm/trunk/test/tools/llvm-symbolizer/pdb/lit.local.cfg

+config.unsupported = not config.have_dia_sdk

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

+RUN: llvm-symbolizer -obj="%p/Inputs/test.exe" < "%p/Inputs/test.exe.input" | FileCheck %s --check-prefix=CHECK
+
+CHECK: _foo
+CHECK-NEXT: test.c:3:0
+CHECK: _main
+CHECK-NEXT: test.c:6:0
+CHECK: ??
+CHECK-NEXT: ??:0:0

llvm/trunk/tools/llvm-symbolizer/CMakeLists.txt

 # FIXME: As we plan to execute llvm-symbolizer binary from compiler-rt
 # libraries, it has to be compiled for all supported targets (x86_64, i386 etc).
 # This means that we need LLVM libraries to be compiled for these
 # targets as well. Currently, there is no support for such a build strategy.
 
 set(LLVM_LINK_COMPONENTS
   DebugInfoDWARF
+  DebugInfoPDB
   Object
   Support
   )
 
 add_llvm_tool(llvm-symbolizer
   LLVMSymbolize.cpp
   llvm-symbolizer.cpp
   )

llvm/trunk/tools/llvm-symbolizer/LLVMSymbolize.cpp

 // Implementation for LLVM symbolization library.
 //
 //===----------------------------------------------------------------------===//
 
 #include "LLVMSymbolize.h"
 #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"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/DataExtractor.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
if (Opts.PrintFunctions != FunctionNameKind::None && Opts.UseSymbolTable) {
     }
   }
   return LineInfo;
 }
 
 DIInliningInfo ModuleInfo::symbolizeInlinedCode(
     uint64_t ModuleOffset, const LLVMSymbolizer::Options &Opts) const {
   DIInliningInfo InlinedContext;
+
   if (DebugInfoContext) {
     InlinedContext = DebugInfoContext->getInliningInfoForAddress(
         ModuleOffset, getDILineInfoSpecifier(Opts));
   }
   // Make sure there is at least one frame in context.
   if (InlinedContext.getNumberOfFrames() == 0) {
     InlinedContext.addFrame(DILineInfo());
   }
LLVMSymbolizer::getOrCreateModuleInfo(const std::string &ModuleName) {
   }
   ObjectPair Objects = getOrCreateObjects(BinaryName, ArchName);
 
   if (!Objects.first) {
     // Failed to find valid object file.
     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));
   return Info;
 }
 
 std::string LLVMSymbolizer::printDILineInfo(DILineInfo LineInfo) const {
   // By default, DILineInfo contains "<invalid>" for function/filename it

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

 // file, line number, column number). It is targeted for compiler-rt tools
 // (especially AddressSanitizer and ThreadSanitizer) that can use it
 // to symbolize stack traces in their error reports.
 //
 //===----------------------------------------------------------------------===//
 
 #include "LLVMSymbolize.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/COM.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"

 }
 
 int main(int argc, char **argv) {
   // Print stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal();
   PrettyStackTraceProgram X(argc, argv);
   llvm_shutdown_obj Y; // Call llvm_shutdown() on exit.
 
+  llvm::sys::InitializeCOMRAII COM(llvm::sys::COMThreadingMode::MultiThreaded);
+
   cl::ParseCommandLineOptions(argc, argv, "llvm-symbolizer\n");
   LLVMSymbolizer::Options Opts(ClUseSymbolTable, ClPrintFunctions,
                                ClPrintInlining, ClDemangle, ClDefaultArch);
   for (const auto &hint : ClDsymHint) {
     if (sys::path::extension(hint) == ".dSYM") {
       Opts.DsymHints.push_back(hint);
     } else {
       errs() << "Warning: invalid dSYM hint: \"" << hint <<
                 "\" (must have the '.dSYM' extension).\n";
     }
   }
   LLVMSymbolizer Symbolizer(Opts);
 
   bool IsData = false;
   std::string ModuleName;
   uint64_t ModuleOffset;
   while (parseCommand(IsData, ModuleName, ModuleOffset)) {
     std::string Result =
         IsData ? Symbolizer.symbolizeData(ModuleName, ModuleOffset)
                : Symbolizer.symbolizeCode(ModuleName, ModuleOffset);
     outs() << Result << "\n";
     outs().flush();
   }
+
   return 0;
 }