[PATCH] D85579: [ELF] --gdb-index: skip SHF_GROUP .debug_info

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

I can't say about the heuristic used and how well it should work,
but have a few comments about the code.



================
Comment at: lld/ELF/DWARF.cpp:33
+    InputSectionBase *sec = it.value();
     if (!sec)
       continue;
----------------
So, you need this to read the flags from the `sh_flags` field of a section header in the initial object.
And you can't just use `sec->flags`, because we drop the `SHF_GROUP` flag much earlier.

I do not know a simpler way to access initial flags. Though probably it worth adding an assert to check that the number
of sections in `obj->getObj().sections()` == the number of them in `obj->getSections()` and a comment
about `SHT_GROUP` flag? That way we will not forget to simplify this loop back later when will stop relying on `SHF_GROUP` flag.


================
Comment at: lld/ELF/SyntheticSections.cpp:2868
 template <class ELFT> GdbIndexSection *GdbIndexSection::create() {
   std::vector<InputSection *> sections = getDebugInfoSections();
 
----------------
It doesn't seem the `getDebugInfoSections` helper and its name makes much sence now with this change?

1) It is not clear what it returns anymore: it might return either CU or TU `.debug_info`s.
2) You are not really using the result, since `dobj.getInfoSection()` is called instead to get debug info sections.
3) It is just needed now to find the number of files with `.debug_info` sections.

So I think it should probably just be inlined and/or rewritten to collect a file set mentioned probably?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85579