[Lldb-commits] [PATCH] D17363: Add SymbolFilePDB with basic support for line tables.
Zachary Turner via lldb-commits
lldb-commits at lists.llvm.org
Wed Feb 17 20:29:20 PST 2016
zturner added a comment.
Added a few notes to clarify things that might not be obvious upon a first look at the patch.
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:81
@@ +80,3 @@
+ std::string exePath = m_obj_file->GetFileSpec().GetPath();
+ auto error = llvm::loadDataForEXE(llvm::PDB_ReaderType::DIA, llvm::StringRef(exePath), m_session);
+ if (error != llvm::PDB_ErrorCode::Success)
Note that this entire plugin is supposed to compile and be registered on every platform, including platforms that don't support PDB (i.e. which, for now, is all non-Windows platforms). But this function will simply return an error in those case, and return `0` for the abilities.
Later, if and when PDB reading support is implemented in such a way as to not require Windows, we need only change the first argument and to `loadDataForExe` and we will have PDB support on non windows platforms with no other changes required.
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:118-119
@@ +117,4 @@
+ // Default to C++, currently DebugInfoPDB does not return the language.
+ return lldb::eLanguageTypeC_plus_plus;
This is a mistake, it does contain the language. I will need to implement this, although for all intents and purposes it doesn't matter because it's C++ in all cases we care about anyway.
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:374-375
@@ +373,4 @@
+ // This frequently (i.e. in all cases I've seen) returns null. Hopefully
+ // LLDB can handle null values for this.
+ FileSpec path(cu->getSourceFileName(), false);
This comment is no longer valid. It returns the source file name. Pretend you didn't see this.
Comment at: unittests/SymbolFile/PDB/Inputs/test-dwarf.cpp:1-2
@@ +1,3 @@
+// Compile with "cl /c /Zi /GR- test.cpp"
+// Link with "link test.obj /debug /nodefaultlib /entry:main /out:test.exe"
Need to fix this comment to include the clang++ command line instead of the cl command line.
Comment at: unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp:94
@@ +93,3 @@
+#define REQUIRES_DIA_SDK(TestName) DISABLED_##TestName
This will cause the 2 PDB-specific tests to run only on Windows.
More information about the lldb-commits