[PATCH] D26283: [ELF] - Partial support of --gdb-index command line option (Part 3).

[Greg Clayton via Phabricator via llvm-commits]

clayborg added a comment.

Seems like the GdbIndex isn't specific to ELF. I know the file started off here and that some of the template classes you use are using ELF defines, but they don't really need to be in the ELF plug-in.

Would we want to move this functionality to the lib/DebugInfo/Dwarf/DWARFGdbIndex.cpp file so that other file formats like mach-o and COFF could create the gdb index section and be able to emit it? This code accesses existing DWARF sections and then creates the .gdb-index section contents.



================
Comment at: ELF/GdbIndex.cpp:63
+// symbol table and constant pool.
+// Since we so not plan to support types CU list area, it is also empty and
+// so far is "implemented". Address area is the only not implemented part.
----------------
Since we "do" not plan


================
Comment at: ELF/SyntheticSections.cpp:1441
+      Builder.readPubNamesAndTypes();
+  for (std::pair<StringRef, uint8_t> &Pair : NamesAndTypes) {
+    uint32_t Hash = hash(Pair.first.data());
----------------
use std::tie here with appropriate var names since you did it below?


================
Comment at: ELF/SyntheticSections.cpp:1442
+  for (std::pair<StringRef, uint8_t> &Pair : NamesAndTypes) {
+    uint32_t Hash = hash(Pair.first.data());
+    size_t Offset = StringPool.add(Pair.first);
----------------
I am guessing we can guarantee that the StringRef is NULL terminated here since the string probably originated from a string table.


================
Comment at: ELF/SyntheticSections.cpp:1522
+    Buf += 4;
+    for (std::pair<uint32_t, uint8_t> &P : CuVec) {
+      uint32_t Index = P.first;
----------------
use std::tie?


https://reviews.llvm.org/D26283