|
| 1 | +# `llvm-debuginfo-analyzer` |
| 2 | + |
| 3 | +These are the notes collected during the development, review and test. |
| 4 | +They describe limitations, known issues and future work. |
| 5 | + |
| 6 | +### Remove the use of macros in ``LVReader.h`` that describe the ``bumpallocators``. |
| 7 | +**[D137933](https://reviews.llvm.org/D137933#inline-1389904)** |
| 8 | + |
| 9 | +Use a standard (or LLVM) ``map`` with ``typeinfo`` (would need a specialization |
| 10 | +to expose equality and hasher) for the allocators and the creation |
| 11 | +functions could be a function template. |
| 12 | + |
| 13 | +### Use a **lit test** instead of a **unit test** for the **logical readers**. |
| 14 | +**[D125783](https://reviews.llvm.org/D125783#inline-1324376)** |
| 15 | + |
| 16 | +As the ``DebugInfoLogicalView`` library is sufficiently exposed via the |
| 17 | +``llvm-debuginfo-analyzer`` tool, follow the LLVM general approach and |
| 18 | +use ``lit`` tests to validate the **logical readers**. |
| 19 | + |
| 20 | +Convert the ``unitests``: |
| 21 | +``` |
| 22 | +llvm-project/llvm/unittests/DebugInfo/LogicalView/CodeViewReaderTest.cpp |
| 23 | +llvm-project/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp |
| 24 | +``` |
| 25 | +into ``lit`` tests: |
| 26 | +``` |
| 27 | +llvm-project/llvm/test/DebugInfo/LogicalView/CodeViewReader.test |
| 28 | +llvm-project/llvm/test/DebugInfo/LogicalView/DWARFReader.test |
| 29 | +``` |
| 30 | + |
| 31 | +### Eliminate calls to ``getInputFileDirectory()`` in the ``unittests``. |
| 32 | +**[D125783](https://reviews.llvm.org/D125783#inline-1324359)** |
| 33 | + |
| 34 | +Rewrite the unittests ``ReaderTest`` and ``CodeViewReaderTest`` to eliminate |
| 35 | +the call: |
| 36 | +``` |
| 37 | + getInputFileDirectory() |
| 38 | +``` |
| 39 | +as use of that call is discouraged. |
| 40 | + |
| 41 | +### Fix mismatch between ``%d/%x`` format strings and ``uint64_t`` type. |
| 42 | +**[D137400](https://reviews.llvm.org/D137400) / [58758](https://github.com/llvm/llvm-project/issues/58758)** |
| 43 | + |
| 44 | +Incorrect printing of ``uint64_t`` on ``32-bit`` platforms. |
| 45 | +Add the ``PRIx64`` specifier to the printing code (``format()``). |
| 46 | + |
| 47 | +### Remove ``LVScope::Children`` container. |
| 48 | +**[D137933](https://reviews.llvm.org/D137933#inline-1373902)** |
| 49 | + |
| 50 | +Use a **chaining iterator** over the other containers rather than keep a |
| 51 | +separate container ``Children`` that mirrors their contents. |
| 52 | + |
| 53 | +### Use ``TableGen`` for command line options. |
| 54 | +**[D125777](https://reviews.llvm.org/D125777#inline-1291801)** |
| 55 | + |
| 56 | +The current trend is to use ``TableGen`` for command-line options in tools. |
| 57 | +Change command line options to use ``tablegen`` as many other LLVM tools. |
| 58 | + |
| 59 | +### ``LVDoubleMap`` to return ``optional<ValueType>`` instead of ``null pointer``. |
| 60 | +**[D125783](https://reviews.llvm.org/D125783#inline-1294164)** |
| 61 | + |
| 62 | +The more idiomatic LLVM way to handle this would be to have ``find`` |
| 63 | +return ``Optional<ValueType>``. |
| 64 | + |
| 65 | +### Pass references instead of pointers (**Comparison functions**). |
| 66 | +**[D125782](https://reviews.llvm.org/D125782#inline-1293920)** |
| 67 | + |
| 68 | +In the **comparison functions**, pass references instead of pointers (when |
| 69 | +pointers cannot be null). |
| 70 | + |
| 71 | +### Use ``StringMap`` where possible. |
| 72 | +**[D125783](https://reviews.llvm.org/D125783#inline-1294211)** |
| 73 | + |
| 74 | +LLVM has a ``StringMap`` class that is advertised as more efficient than |
| 75 | +``std::map<std::string, ValueType>``. Mainly it does fewer allocations |
| 76 | +because the key is not a ``std::string``. |
| 77 | + |
| 78 | +Replace the use of ``std::map<std::string, ValueType>`` with ``StringMap``. |
| 79 | +One specific case is the ``LVSymbolNames`` definitions. |
| 80 | + |
| 81 | +### Calculate unique offset for CodeView elements. |
| 82 | +In order to have the same logical functionality as the DWARF reader, such |
| 83 | +as: |
| 84 | + |
| 85 | +* find scopes contribution to debug info |
| 86 | +* sort by its physical location |
| 87 | + |
| 88 | +The logical elements must have an unique offset (similar like the DWARF |
| 89 | +``DIE`` offset). |
| 90 | + |
| 91 | +### Move ``initializeFileAndStringTables`` to the CodeView Library. |
| 92 | +There is some code in the CodeView reader that was extracted/adapted |
| 93 | +from ``tools/llvm-readobj/COFFDumper.cpp`` that can be moved to the CodeView |
| 94 | +library. |
| 95 | + |
| 96 | +We had a similar case with code shared with ``llvm-pdbutil`` that was moved |
| 97 | +to the PDB library: **[D122226](https://reviews.llvm.org/D122226)** |
| 98 | + |
| 99 | +### Move ``getSymbolKindName`` and ``formatRegisterId`` to the CodeView Library. |
| 100 | +There is some code in the CodeView reader that was extracted/adapted |
| 101 | +from ``lib/DebugInfo/CodeView/SymbolDumper.cpp`` that can be used. |
| 102 | + |
| 103 | +### Use of ``std::unordered_set`` instead of ``std::set``. |
| 104 | +**[D125784](https://reviews.llvm.org/D125784#inline-1221421)** |
| 105 | + |
| 106 | +Replace the ``std::set`` usage for ``DeducedScopes``, ``UnresolvedScopes`` and |
| 107 | +``IdentifiedNamespaces`` with ``std::unordered_set`` and get the benefit |
| 108 | +of the O(1) while inserting/searching, as the order is not important. |
| 109 | + |
| 110 | +### Optimize ``LVNamespaceDeduction::find`` funtion. |
| 111 | +**[D125784](https://reviews.llvm.org/D125784#inline-1296195)** |
| 112 | + |
| 113 | +Optimize the ``find`` method to use the proposed code: |
| 114 | + |
| 115 | +``` |
| 116 | + LVStringRefs::iterator Iter = std::find_if(Components.begin(), Components.end(), |
| 117 | + [](StringRef Name) { |
| 118 | + return IdentifiedNamespaces.find(Name) == IdentifiedNamespaces.end(); |
| 119 | + }); |
| 120 | + LVStringRefs::size_type FirstNonNamespace = std::distance(Components.begin(), Iter); |
| 121 | +``` |
| 122 | + |
| 123 | +### Move all the printing support to a common module. |
| 124 | +Factor out printing functionality from the logical elements into a |
| 125 | +common module. |
| 126 | + |
| 127 | +### Refactor ``LVBinaryReader::processLines``. |
| 128 | +**[D125783](https://reviews.llvm.org/D125783#inline-1246155) / |
| 129 | +[D137156](https://reviews.llvm.org/D137156)** |
| 130 | + |
| 131 | +During the traversal of the debug information sections, we created the |
| 132 | +logical lines representing the **disassembled instructions** from the **text |
| 133 | +section** and the logical lines representing the **line records** from the |
| 134 | +**debug line** section. Using the ranges associated with the logical scopes, |
| 135 | +we will allocate those logical lines to their logical scopes. |
| 136 | + |
| 137 | +Consider the case when any of those lines become orphans, causing |
| 138 | +incorrect scope parent for disassembly or line records. |
| 139 | + |
| 140 | +### Add support for ``-ffunction-sections``. |
| 141 | +**[D125783](https://reviews.llvm.org/D125783#inline-1295012)** |
| 142 | + |
| 143 | +Only linked executables are handled. It does not support relocatable |
| 144 | +files compiled with ``-ffunction-sections``. |
| 145 | + |
| 146 | +### Add support for DWARF v5 `.debug_names` section / CodeView public symbols stream. |
| 147 | +**[D125783](https://reviews.llvm.org/D125783#inline-1294142)** |
| 148 | + |
| 149 | +The DWARF and CodeView readers use the public names information to create |
| 150 | +the instructions (``LVLineAssembler``). Instead of relying on DWARF section |
| 151 | +names (``.debug_pubnames``, ``.debug_names``) and CodeView public symbol stream |
| 152 | +(``S_PUB32``), the readers should collect the needed information while processing |
| 153 | +the debug information. |
| 154 | + |
| 155 | +If the object file supports the above section names and stream, use them |
| 156 | +to create the public names. |
| 157 | + |
| 158 | +### Add support for some extra DWARF locations. |
| 159 | +The following DWARF debug location operands are not supported: |
| 160 | + |
| 161 | +* `DW_OP_const_type` |
| 162 | +* `DW_OP_entry_value` |
| 163 | +* `DW_OP_implicit_value` |
| 164 | + |
| 165 | +### Add support for additional binary formats. |
| 166 | +* Extended COFF (`XCOFF`) |
| 167 | + |
| 168 | +### Add support for ``JSON`` or ``YAML`` |
| 169 | +The logical view uses its own and non-standard free form text when |
| 170 | +displaying information on logical elements. |
0 commit comments