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

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 19 00:51:19 PDT 2016


grimar added inline comments.


================
Comment at: ELF/GdbIndex.cpp:76
+
+  std::unique_ptr<llvm::DWARFContext> Dwarf;
+  if (Expected<std::unique_ptr<object::ObjectFile>> Obj =
----------------
ruiu wrote:
> ruiu wrote:
> > Remove `llvm::`.
>  
Done.


================
Comment at: ELF/GdbIndex.cpp:78
+  if (Expected<std::unique_ptr<object::ObjectFile>> Obj =
+          object::ObjectFile::createObjectFile(DebugInfoSec->getFile()->MB))
+    Dwarf.reset(new DWARFContextInMemory(*Obj.get()));
----------------
ruiu wrote:
> It seems that `createObjectFile` parses the file again. Can you avoid that?
I afraid current parsers API does not allow that yet.

Starting from next patch for this feature, I am planing to suggest changes for DWARF parsers API as it seems not possible to do things I need for next steps without some changes.
An parallel task might be optimization of parsers entrypoints for linker needs then, there is a room for that.

I suggest to land this one first because then it will be easy to justify the need of parsers changes and adding linker specific functionality.


================
Comment at: ELF/GdbIndex.cpp:88
+  std::vector<std::pair<uintX_t, uintX_t>> Ret;
+  for (const auto &CU : Dwarf->compile_units())
+    Ret.push_back(
----------------
ruiu wrote:
> Replace `auto` with its concrete type.
Done.


================
Comment at: ELF/GdbIndex.cpp:94-101
+template std::vector<std::pair<ELF32LE::uint, ELF32LE::uint>>
+lld::elf::readCuList<ELF32LE>(InputSection<ELF32LE> *Sec);
+template std::vector<std::pair<ELF32BE::uint, ELF32BE::uint>>
+lld::elf::readCuList<ELF32BE>(InputSection<ELF32BE> *Sec);
+template std::vector<std::pair<ELF64LE::uint, ELF64LE::uint>>
+lld::elf::readCuList<ELF64LE>(InputSection<ELF64LE> *Sec);
+template std::vector<std::pair<ELF64BE::uint, ELF64BE::uint>>
----------------
ruiu wrote:
> Remove `Sec`. Use uint32_t and uint64_t instead of ELF????::uint.
Done.


https://reviews.llvm.org/D24706





More information about the llvm-commits mailing list