[PATCH] D124317: [llvm-pdbutil] Add options to only dump symbol record at specified offset and its parents or children with spcified depth.
Zequan Wu via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 26 13:45:54 PDT 2022
zequanwu added inline comments.
================
Comment at: llvm/lib/DebugInfo/CodeView/CVSymbolVisitor.cpp:108
+ }
+ }
+ }
----------------
rnk wrote:
> I think you need to "pop off" a parent record every time you see a closing scope. Consider this sequence of records, where `(` is open, `)` is close, `*` is the desired record: `(()(*))`
>
> In your existing test case, this would correspond with the output of a second inline call site that isn't part of the first.
An opening scope is only added in to `ParentSymbols` when the scope range [begin, end] encompass the desired offset.
In the case of `(()(*))`, the first inner scope`()` will not be added into `ParentSymbols`.
Added a test case for `(()(*))`.
================
Comment at: llvm/lib/DebugInfo/CodeView/CVSymbolVisitor.cpp:149
+ uint32_t ParentEndOffset =
+ getScopeEndOffset(*Symbols.at(ParentSymbols[StartIndex]));
+ CVSymbol ParentEnd = *Symbols.at(ParentEndOffset);
----------------
rnk wrote:
> I don't think these scope end offsets are available in the typical object file. Try running llvm-pdbutil dump with these filtration options on a single object file instead of a PDB, I think you'll find that the end offset is zero.
Yes, all offsets and parent/end are 0. So, the filter options are only used in `DumpOutputStyle::dumpModuleSymsForPdb`, and silently ignored when dumping obj file.
================
Comment at: llvm/lib/DebugInfo/CodeView/CVSymbolVisitor.cpp:150
+ getScopeEndOffset(*Symbols.at(ParentSymbols[StartIndex]));
+ CVSymbol ParentEnd = *Symbols.at(ParentEndOffset);
+ if (auto EC = visitSymbolRecord(ParentEnd, ParentEndOffset))
----------------
rnk wrote:
> This would crash if the PDB file is corrupt, please make it handle this case gracefully. I think in dumping code, we should be as defensive as possible because dumpers are often used on new or non-conforming inputs that trigger linker bugs.
Add `VarStreamArray::isOffsetValid` to check if an offset is valid, and use it to check offset before dereference.
================
Comment at: llvm/test/tools/llvm-pdbutil/symbol-offset.test:36
+; RUN: | FileCheck --check-prefix=SHOW-CHILDREN-DEPTH2 %s
+
+OFFSET: 80 | S_LOCAL [size = 16] `argc`
----------------
rnk wrote:
> Please include the C++ source used to produce the input YAML file to help illustrate the expected structure.
I deleted the cpp file, so I rewrote new one.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124317/new/
https://reviews.llvm.org/D124317
More information about the llvm-commits
mailing list