[PATCH] D125784: [llvm-dva] 09 - CodeView Reader
Carlos Alberto Enciso via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 15 05:55:43 PDT 2022
CarlosAlbertoEnciso added inline comments.
================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Readers/LVBinaryReader.h:171
+ void addInlineeLines(LVScope *Scope, LVLines &Lines) {
+ LVLines *InlineeLines = new LVLines();
+ *InlineeLines = std::move(Lines);
----------------
psamolysov wrote:
> An `InlineeLines` is a pointer to an in-heap allocated object. I've found that some lines are deleted in the `LVBinaryReader::includeInlineeLines` function but I'm not sure that all the allocated ones. Although, what if the function is never called after a call to the `addInlineeLines` one? Could you check that all the in-heap allocated and not deleted during the class' instance lifetime objects are deleted in the destructor?
For the CodeView Reader, the lines for inlined functions are processed in a separated step. Those lines are collected in `CUInlineeLines` which are processed by the `includeInlineeLines` funtion.
```
// Traverse the scopes for the given 'Function' looking for any inlined
// scopes with inlined lines, which are recorded in 'CUInlineeLines'.
void LVBinaryReader::includeInlineeLines(LVSectionIndex SectionIndex,
LVScope *Function) {
..
CULines.insert(Iter, InlineeLines->begin() + 1, InlineeLines->end());
..
```
Then those `CULines` are processed by the `void LVBinaryReader::processLines` function which move them to its enclosing logical `LVScope` and deleted at the end of the scope lifetime.
The same case as discussed in https://reviews.llvm.org/D125783 (ELF Reader).
================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.h:350
+ std::string getCompileUnitName() { return CompileUnitName; }
+ void setCompileUnitName(std::string Name) { CompileUnitName = Name; }
+
----------------
psamolysov wrote:
>
Changed to `CompileUnitName = std::move(Name);`.
================
Comment at: llvm/lib/DebugInfo/LogicalView/LVReaderHandler.cpp:157
+ std::vector<StringRef> ExecutableExtensions = {"exe", "dll"};
+ for (const StringRef &Extension : ExecutableExtensions) {
+ std::string ExecutableImage = searchForExe(Filename, Extension);
----------------
psamolysov wrote:
> It makes no sense to pass `StringRef` by reference.
Good point.
================
Comment at: llvm/lib/DebugInfo/LogicalView/LVReaderHandler.cpp:170
+ std::vector<StringRef> ObjectExtensions = {"o", "obj", "lib"};
+ for (const StringRef &Extension : ObjectExtensions) {
+ std::string ObjectImage = searchForObj(Filename, Extension);
----------------
psamolysov wrote:
>
Good point.
================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewReader.cpp:777
+ // know all the required information.
+ for (StringRef &SymbolName : SymbolNames) {
+ LLVM_DEBUG({
----------------
psamolysov wrote:
> `StringRef` is already a "reference".
Good point.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125784/new/
https://reviews.llvm.org/D125784
More information about the llvm-commits
mailing list