Skip to content

Commit 4af8ff6

Browse files
committed
DBG: (performance) improvements to SymbolSourceDIA
1 parent 9b602ee commit 4af8ff6

File tree

3 files changed

+67
-36
lines changed

3 files changed

+67
-36
lines changed

src/dbg/module.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1190,6 +1190,8 @@ bool MODINFO::loadSymbols()
11901190
if(symbols == &EmptySymbolSource && SymbolSourceDIA::isLibraryAvailable())
11911191
{
11921192
// TODO: do something with searchPaths
1193+
std::string modname = name;
1194+
modname += extension;
11931195
DiaValidationData_t validationData;
11941196
memcpy(&validationData.guid, &pdbValidation.guid, sizeof(GUID));
11951197
validationData.signature = pdbValidation.signature;
@@ -1201,7 +1203,7 @@ bool MODINFO::loadSymbols()
12011203
{
12021204
GuiSymbolLogAdd(StringUtils::sprintf("[DIA] Skipping non-existent PDB: %s\n", pdbPath.c_str()).c_str());
12031205
}
1204-
else if(symSource->loadPDB(pdbPath, base, size, bForceLoadSymbols ? nullptr : &validationData))
1206+
else if(symSource->loadPDB(pdbPath, modname, base, size, bForceLoadSymbols ? nullptr : &validationData))
12051207
{
12061208
symSource->resizeSymbolBitmap(size);
12071209

src/dbg/symbolsourcedia.cpp

Lines changed: 62 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ DWORD WINAPI SymbolSourceDIA::SourceLinesThread(void* parameter)
4242
return 0;
4343
}
4444

45-
bool SymbolSourceDIA::loadPDB(const std::string & path, duint imageBase, duint imageSize, DiaValidationData_t* validationData)
45+
bool SymbolSourceDIA::loadPDB(const std::string & path, const std::string & modname, duint imageBase, duint imageSize, DiaValidationData_t* validationData)
4646
{
4747
PDBDiaFile pdb; // Instance used for validation only.
4848
_isOpen = pdb.open(path.c_str(), 0, validationData);
@@ -119,7 +119,7 @@ bool SymbolSourceDIA::loadSymbolsAsync()
119119
DWORD loadStart = GetTickCount();
120120

121121
PDBDiaFile::Query_t query;
122-
query.collectSize = true;
122+
query.collectSize = false;
123123
query.collectUndecoratedNames = true;
124124
query.callback = [&](DiaSymbol_t & sym) -> bool
125125
{
@@ -221,7 +221,7 @@ bool SymbolSourceDIA::loadSymbolsAsync()
221221
DWORD ms = GetTickCount() - loadStart;
222222
double secs = (double)ms / 1000.0;
223223

224-
GuiSymbolLogAdd(StringUtils::sprintf("[%p] Loaded %d symbols in %.03fs\n", _imageBase, _symAddrs.size(), secs).c_str());
224+
GuiSymbolLogAdd(StringUtils::sprintf("[%p, %s] Loaded %d symbols in %.03fs\n", _imageBase, _modname.c_str(), _symAddrs.size(), secs).c_str());
225225

226226
GuiInvalidateSymbolSource(_imageBase);
227227

@@ -245,48 +245,76 @@ bool SymbolSourceDIA::loadSourceLinesAsync()
245245

246246
const uint32_t rangeSize = 1024 * 1024 * 10;
247247

248+
std::vector<DiaLineInfo_t> lines;
248249
std::map<DWORD, String> files;
249250

250-
for(duint rva = 0; rva < _imageSize; rva += rangeSize)
251-
{
252-
if(_requiresShutdown)
253-
return false;
254-
255-
std::vector<DiaLineInfo_t> lines;
251+
if(!pdb.enumerateLineNumbers(0, uint32_t(_imageSize), lines, files))
252+
return false;
256253

257-
bool res = pdb.enumerateLineNumbers(uint32_t(rva), rangeSize, lines, files);
258-
for(const auto & line : lines)
254+
if(files.size() == 1)
255+
{
256+
GuiSymbolLogAdd(StringUtils::sprintf("[%p, %s] Since there is only one file, attempting line overflow detection..\n", _imageBase, _modname.c_str()).c_str());
257+
258+
// This is a super hack to adjust for the (undocumented) limit of 16777215 lines (unsigned 24 bits maximum).
259+
// It is unclear at this point if yasm/coff/link/pdb is causing this issue.
260+
// We can fix this because there is only a single source file and the returned result is sorted by *both* rva/line (IMPORTANT!).
261+
// For supporting multiple source files in the future we could need multiple 'lineOverflow' variables.
262+
uint32_t maxLine = 0, maxRva = 0, lineOverflows = 0;
263+
for(auto & line : lines)
259264
{
260-
if(_requiresShutdown)
261-
return false;
262-
263-
const auto & info = line;
264-
auto it = _lines.find(info.rva);
265-
if(it != _lines.end())
266-
continue;
267-
268-
CachedLineInfo lineInfo;
269-
lineInfo.rva = info.rva;
270-
lineInfo.lineNumber = info.lineNumber;
265+
uint32_t overflowValue = 0x1000000 * (lineOverflows + 1) - 1; //0xffffff, 0x1ffffff, 0x2ffffff, etc
266+
if((line.lineNumber & 0xfffff0) == 0 && (maxLine & 0xfffffff0) == (overflowValue & 0xfffffff0)) // allow 16 lines of play, perhaps there is a label/comment on line 0xffffff+1
267+
{
268+
GuiSymbolLogAdd(StringUtils::sprintf("[%p, %s] Line number overflow detected (%u -> %u), adjusting with hacks...\n", _imageBase, _modname.c_str(), maxLine, line.lineNumber).c_str());
269+
lineOverflows++;
270+
}
271271

272-
auto sourceFileId = info.sourceFileId;
273-
auto found = _sourceIdMap.find(sourceFileId);
274-
if(found == _sourceIdMap.end())
272+
line.lineNumber += lineOverflows * 0xffffff + lineOverflows;
273+
if(!(line.lineNumber > maxLine))
275274
{
276-
auto idx = _sourceFiles.size();
277-
_sourceFiles.push_back(files[sourceFileId]);
278-
found = _sourceIdMap.insert({ sourceFileId, uint32_t(idx) }).first;
275+
GuiSymbolLogAdd(StringUtils::sprintf("[%p, %s] The line information is not sorted by line (violated assumption)! lineNumber: %u, maxLine: %u\n", _imageBase, _modname.c_str(), line.lineNumber, maxLine).c_str());
279276
}
280-
lineInfo.sourceFileIdx = found->second;
277+
maxLine = line.lineNumber;
278+
if(!(line.rva > maxRva))
279+
{
280+
GuiSymbolLogAdd(StringUtils::sprintf("[%p, %s] The line information is not sorted by rva (violated assumption)! rva: 0x%x, maxRva: 0x%x\n", _imageBase, _modname.c_str(), line.rva, maxRva).c_str());
281+
}
282+
maxRva = line.rva;
283+
}
284+
}
285+
286+
_linesData.reserve(lines.size());
287+
_sourceFiles.reserve(files.size());
288+
for(const auto & line : lines)
289+
{
290+
if(_requiresShutdown)
291+
return false;
281292

282-
_lockLines.lock();
293+
const auto & info = line;
294+
auto it = _lines.find(info.rva);
295+
if(it != _lines.end())
296+
continue;
283297

284-
_linesData.push_back(lineInfo);
285-
_lines.insert({ lineInfo.rva, _linesData.size() - 1 });
298+
CachedLineInfo lineInfo;
299+
lineInfo.rva = info.rva;
300+
lineInfo.lineNumber = info.lineNumber;
286301

287-
_lockLines.unlock();
302+
auto sourceFileId = info.sourceFileId;
303+
auto found = _sourceIdMap.find(sourceFileId);
304+
if(found == _sourceIdMap.end())
305+
{
306+
auto idx = _sourceFiles.size();
307+
_sourceFiles.push_back(files[sourceFileId]);
308+
found = _sourceIdMap.insert({ sourceFileId, uint32_t(idx) }).first;
288309
}
310+
lineInfo.sourceFileIdx = found->second;
311+
312+
_lockLines.lock();
313+
314+
_linesData.push_back(lineInfo);
315+
_lines.insert({ lineInfo.rva, _linesData.size() - 1 });
289316

317+
_lockLines.unlock();
290318
}
291319

292320
_sourceLines.resize(_sourceFiles.size());
@@ -302,7 +330,7 @@ bool SymbolSourceDIA::loadSourceLinesAsync()
302330
DWORD ms = GetTickCount() - lineLoadStart;
303331
double secs = (double)ms / 1000.0;
304332

305-
GuiSymbolLogAdd(StringUtils::sprintf("[%p] Loaded %d line infos in %.03fs\n", _imageBase, _lines.size(), secs).c_str());
333+
GuiSymbolLogAdd(StringUtils::sprintf("[%p, %s] Loaded %d line infos in %.03fs\n", _imageBase, _modname.c_str(), _lines.size(), secs).c_str());
306334

307335
GuiUpdateAllViews();
308336

src/dbg/symbolsourcedia.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ class SymbolSourceDIA : public SymbolSourceBase
106106

107107
bool _isOpen;
108108
std::string _path;
109+
std::string _modname;
109110
duint _imageBase;
110111
duint _imageSize;
111112
SpinLock _lockSymbols;
@@ -158,7 +159,7 @@ class SymbolSourceDIA : public SymbolSourceBase
158159
virtual bool findSymbolsByPrefix(const std::string & prefix, const std::function<bool(const SymbolInfo &)> & cbSymbol, bool caseSensitive) override;
159160

160161
public:
161-
bool loadPDB(const std::string & path, duint imageBase, duint imageSize, DiaValidationData_t* validationData);
162+
bool loadPDB(const std::string & path, const std::string & modname, duint imageBase, duint imageSize, DiaValidationData_t* validationData);
162163

163164
private:
164165
void loadPDBAsync();

0 commit comments

Comments
 (0)