[PATCH] D125783: [llvm-dva] 08 - ELF Reader

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp:1269-1271
+  if (Iter != Map->begin())
+    Iter = std::prev(Iter);
+  return (Iter != Map->end()) ? Iter->second : nullptr;
----------------
psamolysov wrote:
> Here could be a bug: if `Map->upper_bound(Address)` returns `Map->end()`, `Map->end()` is definitely not equal to `Map->begin()`, `Iter` changes it's value to an element previous to the `end` and the check `Iter != Map->end()` is forever be `true`.
Using similar approach from previous patches, changed to:

```
LVLine *LVScopeCompileUnit::lineUpperBound(LVAddress Address,
                                           LVScope *Scope) const {
  LVSectionIndex SectionIndex = getReader().getSectionIndex(Scope);
  LVAddressToLine *Map = SectionMappings.findMap(SectionIndex);
  if (!Map || Map->empty())
    return nullptr;
  LVAddressToLine::const_iterator Iter = Map->upper_bound(Address);
  if (Iter != Map->begin())
    Iter = std::prev(Iter);
  return Iter->second;
}
```


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVELFReader.cpp:724
+    for (const DWARFDebugLine::Row &Row : Lines->Rows) {
+      LVLineDebug *Line = new LVLineDebug();
+      CULines.push_back(Line);
----------------
psamolysov wrote:
> An allocated in the heap `LVLineDebug` is appended into the `CULines` vector. The vector then goes into the `processLines` function and is cleared in the `LVELFReader::createScopes()` member function. I cannot find any place where the allocated `Line` is deleted.
The best way to explain it is by looking at the following code:

```
// During the traversal of the debug information sections, we created the
// logical lines representing the disassembled instructions from the text
// section and the logical lines representing the line records from the
// debug line section. Using the ranges associated with the logical scopes,
// we will allocate those logical lines to their logical scopes.
void LVBinaryReader::processLines(LVLines *DebugLines,
                                  LVSectionIndex SectionIndex,
                                  LVScope *Function) {
{
  ..
  // Process collected lines.
  LVScope *Scope;
  for (LVLine *Line : *DebugLines) {
    ..
    // Add line object to scope.
    Scope->addElement(Line);     <---- LineDebug/LineAssembler added to Scope
   ..
  }
}
```
For each `LVLine`, the function finds the `LVScope` whose address range will contain its address. Then it add that `LVLine` to that scope. The `LVLine` can be a `LVLineDebug` or `LVLineAssembler`.
When the scope is destroyed, all its lines (`LVLineDebug` and/or `LVLineAssembler`) are deleted.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVELFReader.cpp:856
+    SymbolsWithLocations.clear();
+    CULines.clear();
+  }
----------------
psamolysov wrote:
> It this correct that `CULines` (a class member) is cleared after each iteration over `CompileUnits`? If there are numerous `CompileUnit`s, `CULines` will just be cleared after the first iteration.
We start with a cleared `CULines`.
At the end of each iteration over `CompileUnits`, `CULines` will contain all its `LVLines` for the current Compile unit `CU`.

The `LVLineDebug`s are created by `createLineAndFileRecords`
The `LVLineAssembler`s are created by `createInstructions`
The `CULines` are processed by `processLines`



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125783/new/

https://reviews.llvm.org/D125783