zturner added inline comments.
================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:108-125
+ if (auto module_sp = m_obj_file->GetModule()) {
+ // See if a symbol file was specified through the `--symfile` option.
+ FileSpec symfile = module_sp->GetSymbolFileFileSpec();
+ if (symfile) {
+ error = loadDataForPDB(PDB_ReaderType::DIA,
+ llvm::StringRef(symfile.GetPath()),
+ m_session_up);
----------------
LLVM coding style dictates the use of early returns whenever possible. So this
block should be changed to something like.
```
auto module_sp = m_obj_file->GetModule();
if (!module_sp)
return 0;
FileSpec symfile = module_sp->GetSymbolFileSpec();
if (!symfile)
return 0;
error = loadDataForPDB(...);
if (error) {
llvm::consumeError(error);
return 0;
}
// etc
```
================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:141
+ // in the `SECTIONHEADERS` data stream.
+ if (auto enum_dbg_streams_up = m_session_up->getDebugStreams()) {
+ while (auto dbg_stream_up = enum_dbg_streams_up->getNext()) {
----------------
Similar thing here. Assign first, and then add:
```
if (!enum_dbg_streams_up)
break;
```
================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:144
+ auto stream_name = dbg_stream_up->getName();
+ if (stream_name == "SECTIONHEADERS") {
+ for (int rec_idx = 0;
----------------
```
if (stream_name != "SECTIONHEADERS")
continue;
```
================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:151-157
+ if (strncmp(record_pointer, ".data", 5) == 0 ||
+ strncmp(record_pointer, ".bss", 4) == 0 ||
+ strncmp(record_pointer, ".rdata", 6) == 0) {
+ abilities |= (GlobalVariables | LocalVariables |
+ VariableTypes);
+ break;
+ }
----------------
Is this correct? If *any* of these 3 sections of present, then *all* of these
capabilities are present?
Shouldn't we be actually trying to query DIA for a global, local, or variable
and then seeing what it returns?
(Incidentally, it's easy to figure this out with the native PDB reader, since
you can just look for the presence of a module symbol stream, globals stream,
and/or TPI stream).
Repository:
rL LLVM
https://reviews.llvm.org/D41092
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
