[PATCH] D125784: [llvm-debuginfo-analyzer] 09 - CodeView Reader

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Readers/LVCodeViewReader.h:63
+
+// The ELF reader use the DWARF constants to create the logical elements.
+// The DW_TAG_* and DW_AT_* are used to select the logical object and to
----------------
probinson wrote:
> 
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVReader.cpp:183
+  // The current CodeView Reader implementation does not have support for
+  // multiple compile units. Until a proper offset calculation, check only
+  // in the current compile unit.
----------------
probinson wrote:
> 
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp:1795
   //   DW_AT_specification DW_FORM_ref4 0x00000048
+  // The CodeView does not include any information at the class level to
+  // mark the member function as external.
----------------
probinson wrote:
> 
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVType.cpp:333
+  // that refer to internal runtime structures, that we do not process. Those
+  // typedes are marked as 'system'. They have an associated logical type,
+  // but the underlying type always is null.
----------------
probinson wrote:
> 
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/LVReaderHandler.cpp:152
+
+    // Search in the directory derivated from the given 'Filename' for a
+    // matching object file (.o, .obj, .lib) or a matching executable file
----------------
probinson wrote:
> 
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVBinaryReader.cpp:839
+// 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,
----------------
probinson wrote:
> I think `recorded` makes it sound like this function puts them into CUInlineeLines.
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewReader.cpp:392
+  if (BuffOrErr.getError()) {
+    // The server name does not exist. Try in the directory as the input file.
+    ServerName = createAlternativePath(InputFilename, ServerName);
----------------
probinson wrote:
> ?
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewReader.cpp:427
+  // the server. We need to keep the original input source, as reading other
+  // sections wil require the input associated with the loaded object file.
+  TypeServer = std::make_shared<InputFile>(&Pdb);
----------------
probinson wrote:
> 
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewReader.cpp:777
+
+    // Find the associate symbol table information.
+    LVSymbolTableEntry SymbolTableEntry = getSymbolTableEntry(SymbolName);
----------------
probinson wrote:
> 
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewReader.cpp:1170
+
+// In order to create the scopes, the CodeView Reader:
+// = Traverse the TPI/IPI stream (Type visitor):
----------------
probinson wrote:
> 
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewReader.cpp:1175
+// = Traverse the symbols section (Symbol visitor):
+// It creates the scopes tree and creates the required logical elements, by
+// using the collected indexes from the type visitor.
----------------
probinson wrote:
> 
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp:403-406
+  // - deducted scopes (class, structure, union and enum) and
+  // - unresolved scopes, that can represent namespaces or any deducted.
+  // Before creating the namespaces, we have to traverse the unresolved
+  // and remove any references to already deducted scopes.
----------------
probinson wrote:
> 
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp:1068
+    DefRangeFramePointerRelFullScopeSym &DefRangeFramePointerRelFullScope) {
+  // Defranges don't have types, just registers and code offsets.
+  LLVM_DEBUG({
----------------
probinson wrote:
> Otherwise it reads as "de-franges" to me :)
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp:1095
+    CVSymbol &Record, DefRangeFramePointerRelSym &DefRangeFramePointerRel) {
+  // Defranges don't have types, just registers and code offsets.
+  LLVM_DEBUG({
----------------
probinson wrote:
> etc. I'll stop marking each one.
Changed this one and other ocurrences.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp:2257
+
+  // The incomming element, does not have a defined kind. Use the given
+  // modifiers to complete its type. A type can have more than one modifier;
----------------
probinson wrote:
> 
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp:2323
+
+  // Type pointer is point to.
+  LVType *Pointer = static_cast<LVType *>(Element);
----------------
probinson wrote:
> 
Changed.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp:3407
+
+  // For the given inlinesite, get the initial line number and it's
+  // source filename. Update the logical scope representing it.
----------------
probinson wrote:
> 
Changed.


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

https://reviews.llvm.org/D125784