[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