[PATCH] Make llvm-symbolizer work on Windows.

Alexey Samsonov vonosmas at gmail.com
Fri Apr 24 12:05:25 PDT 2015


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).


================
Comment at: include/llvm/DebugInfo/PDB/PDBContext.h:10
@@ +9,3 @@
+
+#ifndef LLVM_LIB_DEBUGINFO_PDB_PDBCONTEXT_H
+#define LLVM_LIB_DEBUGINFO_PDB_PDBCONTEXT_H
----------------
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.

================
Comment at: lib/DebugInfo/PDB/PDBContext.cpp:44
@@ +43,3 @@
+                                             DILineInfoSpecifier Specifier) {
+  auto Symbol = Session->findSymbolByAddress(Address);
+  uint32_t Length = 0;
----------------
What if Symbol is nullptr? Probably you should bail out early in this case.

================
Comment at: lib/DebugInfo/PDB/PDBContext.cpp:57
@@ +56,3 @@
+  }
+  if (Length > 0) {
+    auto LineNumbers = Session->findLineNumbersByAddress(Address, Length);
----------------
You can reduce indentation with
  if (Length == 0)
    return Result;


================
Comment at: lib/DebugInfo/PDB/PDBContext.cpp:61
@@ +60,3 @@
+      auto LineInfo = LineNumbers->getNext();
+      auto SourceFile = Session->getSourceFileById(LineInfo->getSourceFileId());
+
----------------
if LineInfo can't be nullptr, can we assert on this?

================
Comment at: lib/DebugInfo/PDB/PDBContext.cpp:87
@@ +86,3 @@
+    auto SourceFile = Session->getSourceFileById(LineInfo->getSourceFileId());
+    LineEntry.Column = LineInfo->getColumnNumber();
+    LineEntry.Line = LineInfo->getLineNumber();
----------------
Looks like you can move copying the contents of LineInfo (from IPDBEnumLineNumbers) to DILineInfo into a  helper function.

================
Comment at: lib/DebugInfo/PDB/PDBContext.cpp:94
@@ +93,3 @@
+      auto Symbol = Session->findSymbolByAddress(LineInfo->getVirtualAddress());
+      if (auto Func = dyn_cast<PDBSymbolFunc>(Symbol.get())) {
+        if (Specifier.FNKind == DINameKind::LinkageName)
----------------
Should this be dyn_cast_or_null ?

http://reviews.llvm.org/D9234

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list